Am 17.09.2015 um 20:13 schrieb 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? Of course not. That wouldn't make any sense. > So for example, if you manually set N_PPP (as if by user error) User error wouldn't suffice, as the LD would get reset to N_TTY when the serial device is closed. You would have to write a program that deliberately switched the LD first to N_PPP and then to N_GIGASET_M101 without closing the device in between. > and then set this line discipline, tty->receive_room will be 64K, not 4K. That wouldn't affect the operation of ser_gigaset, so even if I had set up such a contrived test scenario it wouldn't have exposed any problem. Only setting tty->receive_room to 0 causes the problem, and N_TTY with commit 79901317ce80 is the only LD which does that. >> - 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. Your word for it. Still I don't understand the curious resistance to documenting it. If it is the requirement, why keep it secret? Regards, Tilman -- Tilman Schmidt E-Mail: tilman@xxxxxxx Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
Attachment:
signature.asc
Description: OpenPGP digital signature