Re: [PATCH] select fix

[Manfred Spraul <manfred@colorfullife.com>]

Valdis.Kletnieks@vt.edu wrote:

>On Tue, 29 Jul 2003 10:36:30 PDT, Andrew Morton said:
>
>  
>
>>>-	if (tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS)
>>>+	if (!tty->stopped && tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS)
>>> 		mask |= POLLOUT | POLLWRNORM;
>>>      
>>>
>>Manfred sent a patch through esterday which addresses it this way:
>>
>>-	if (tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS)
>>+	if (tty->driver->chars_in_buffer(tty) < WAKEUP_CHARS &&
>>+			tty->driver->write_room(tty) > 0)
>>
>>Any preferences?
>>    
>>
>
>Would including all 3 conditions make sense?  Not sure if it should be A&B&C, or
>A&(B|C) though, but it certainly smells like the write_room() and tty->stopped
>checks are covering 2 different corner cases....
>  
>
No. select() and write() must agree when -EAGAIN happens.
write() will fail if write_room() returns 0. Additionally, we want to 
delay wakeups a bit, to reduce context switches.
The problem is that the console driver implements stopping by returning 
0 from ->write_room() - therefore "less than WAKEUP_CHARS in buffer" is 
not equivalent to "write will not return -EAGAIN", and thus user space 
loops. My patch fixes that by checking ->write_room() in normal_poll.

Perhaps the Right Thing (tm) is
 >    if (tty->driver->write_room(tty) > WAKEUP_CHARS)
 >        mask |= POLLOUT | POLLWRNORM;

but I simply to not understand the tty layer at all, thus I proposed the 
minimal patch.

--
    Manfred