Re: [PATCH 3.12 16/33] isdn/gigaset: reset tty->receive_room when attaching ser_gigaset

[Peter Hurley]

On Wed, Sep 16, 2015 at 7:26 AM, Tilman Schmidt <tilman@xxxxxxx> wrote:
> Am 16.09.2015 um 03:18 schrieb Peter Hurley:
>> On Tue, Sep 15, 2015 at 8:37 PM, Tilman Schmidt <tilman@xxxxxxx> wrote:
>>> Am 16.09.2015 um 01:08 schrieb Peter Hurley:
>>>> On Tue, Sep 15, 2015 at 10:22 AM, Jiri Slaby <jslaby@xxxxxxx> wrote:
>>>>
>>>>     From: Tilman Schmidt <tilman@xxxxxxx>
>>>>
>>>>     3.12-stable review patch.  If anyone has any objections, please let
>>>>     me know.
>>>>
>>>>     ===============
>>>>
>>>>     [ Upstream commit fd98e9419d8d622a4de91f76b306af6aa627aa9c ]
>>>>
>>>>     Commit 79901317ce80 ("n_tty: Don't flush buffer when closing ldisc"),
>>>>     first merged in kernel release 3.10, caused the following regression
>>>>     in the Gigaset M101 driver:
>>>>
>>>>
>>>> Again, I'll just note my objection to this commit log.
>>>>
>>>> This driver was always broken because it never initialized
>>>> tty->receive_room,
>>>> but rather relied on common but not guaranteed circumstances to
>>>> function.
>>>>
>>>> The commit noted simply made the underlying bug more evident, but the
>>>> root cause was from the original merge commit of this driver.
>>>
>>> I must admit I still don't understand that objection. The meaning of the
>>> term "regression" is simply that something which previously worked
>>> stopped working. It doesn't imply any statement about the root cause.
>>>
>>> The ser-gigaset driver worked before the introduction of commit
>>> 79901317ce80. It didn't work anymore after the introduction of that
>>> commit. So it is correct, and does not contradict your statements above
>>> in any way, to state that commit introduced the described regression.
>>
>> By asserting that commit 79901317ce80 caused the regression, you're
>> claiming that this fix is unnecessary for kernel versions prior to 3.10
>
> Correct.
>
>> Are you certain that no other sequence of state leads to the same
>> condition (and thus requiring the same fix) in earlier kernel versions?
>
> Reasonably certain, yes, for three reasons:
> - There where no reports of that problem before 3.10.



> - My own tests did never encounter that condition, and even after being
> made aware of it I was not able to come up with a test that would
> provoke it with a kernel version before 3.10.

Do any of your tests switch to this line discipline from any other than N_TTY?
Because if so, you would realize that whatever _that_ line discipline sets
tty->receive_room to when it initializes, is what your line discipline will use
without this fix.

So for example, if you manually set N_PPP (as if by user error) and then
set this line discipline, tty->receive_room will be 64K, not 4K.

> - The requirement for line disciplines to set receive_room wasn't (and
> btw still isn't) documented anywhere, so it's unlikely anything actively
> relied on it.

Nevertheless, that is the requirement, and what every other in-tree line
discipline does.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/