Re: [Bug #14388] keyboard under X with 2.6.31

Linus Torvalds
2009-10-13T22:42:56Z

On Tue, 13 Oct 2009, Paul Fulghum wrote:
>
> If flush_to_ldisc() is reentered with the head set to null, nothing
> is done. New buffers can be added where you say, but they are
> added to the tail. So the order of flushed data is retained.

They are added to the tail only if the tail is non-NULL.

And buf.tail, in turn, is protected by the TTY_FLUSHING bit.

And look what happens to TTY_FLUSHING if flush_to_ldisc() is called by
multiple contexts - it doesn't nest right. The inner "flush_to_ldisc()"
will clear the bit (your "nothing is done" case).

Now, I agree that we can solve things differently. We could, for example,
get rid of TTY_FLUSHING entirely. If you want to keep the crazy "head =
NULL" special case, we could basically replace all tests of TTY_FLUSHING
with "tty->buf.tail && !tty->buf.head" instead, and use _that_ as a "the
TTY is in the middle of a flush" operation. That should be 100% equivalent
to my patch.

I do object to the whole crazy subtle TTY locking. I'm convinced it's
wrong, and I'm convinced it's wrong exactly _because_ it tries to be so
subtle and does non-obvious things.

That's why my patch also changed the whole loop logic: it's not subtle any
more. Not only did I make TTY_FLUSHING nest correctly, I also stopped
playing games with buf.head: it's now purely a list, rather than "a list
and a failed attempt to lock".

And no, I'm not sure my patch helps. I'd have expected
'tty_buffer_flush()' to be something very rare, for example. But I also
didn't really check if we may do it some other way.

But I _am_ sure that it makes the code a whole lot more straightforward.
Bits that say "we're busy flushing" suddenly actually act that way, and
pointers that say "this is the head of the buffers" also act that wy.

Linus