Re: Characters vanishing in the tty layer? (maybe related to [Bug#14388] keyboard under X with 2.6.31)

[Michael Ellerman]

On Wed, 2009-10-14 at 07:53 -0700, Linus Torvalds wrote:
> 
> On Wed, 14 Oct 2009, Michael Ellerman wrote:
> > 
> > I'm using 80f5069 or thereabouts, plus some powerpc patches. We also see
> > the bug in 31, but not in 27.
> 
> Could you bisect it? Even if you don't bisect it _all_ the way, 27..31 is 
> a big range. Doing just a handful of bisections (say six or seven) would 
> cut down the number of commits from 47 thousand to somewhat more 
> manageable "hundreds of commits"... But obviously going all the way would 
> be better.

As Benh said it's not really bisectable on our kernel. But I got Mikey
to bisect it on upstream using a different simulator model, and he
couldn't tie it down. It becomes easier to hit in more recent kernels
(since 27), but he could hit in 25 too.

Looking at the code, which I should have done first, it looks like this
is expected behaviour. The sequence is roughly:

      * hvc_console reads all our input and passes it to the tty code
        via tty_insert_flip_char()
      * flush_to_ldisc() runs calling n_tty_receive_buf(), which fills
        4K of tty->read_buf
      * Once read_buf is full, tty->receive_room becomes 0 and
        flush_to_ldisc() reschedules itself to run again in 1 jiffy.
      * Bash reads 1 character, causing receive_room to become 1.
      * flush_to_ldisc() runs again and inserts 1 more char because
        receive_room is now 1.
      * (repeat the last two steps a few times)
      * Bash sets tty->icanon = 1 via n_tty_set_termios(), which calls
        n_tty_set_room(). Because icanon is enabled, n_tty_set_room()
        lies and says we have space for 1 character even though we
        don't.
      * flush_to_ldisc() runs, sees that receive_room is 1 and calls
        n_tty_receive_buf()
      * n_tty_receive_buf() calls n_tty_receive_char() which drops the
        character because there's no room (~ line 1132).
      * We keep dropping characters until we see a newline, which
        increments tty->canon_data, causing n_tty_set_room() to report 0
        space left, and so flush_to_ldisc() reschedules again.


It's a bit unfortunate that n_tty_set_room() lies about the available
space when icanon = 1, but it makes sense in order to handle erase. It
would be nice if n_tty_receive_buf() returned a status to
flush_to_disc() to say "actually I could only fit X chars after all,
please take them back" - but I don't grok how that would play with all
the other logic in there.

cheers

Attachment: signature.asc
Description: This is a digitally signed message part