[lkml]   [2009]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [Bug #14388] keyboard under X with 2.6.31

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.


 \ /
  Last update: 2009-10-18 23:28    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean