[Spice-devel] PATCH: virtio_console: Fix poll blocking even though there is data to read

[Hans de Goede]

Hi,

On 09/15/2010 03:46 PM, Amit Shah wrote:
> On (Wed) Sep 15 2010 [15:37:21], Hans de Goede wrote:
>>>> --- linux-2.6.35.x86_64/drivers/char/virtio_console.c~	2010-08-02 00:11:14.000000000 +0200
>>>> +++ linux-2.6.35.x86_64/drivers/char/virtio_console.c	2010-09-15 13:39:29.043505000 +0200
>>>> @@ -642,7 +642,7 @@ static unsigned int port_fops_poll(struc
>>>>   	poll_wait(filp,&port->waitqueue, wait);
>>>>
>>>>   	ret = 0;
>>>> -	if (port->inbuf)
>>>> +	if (!will_read_block(port))
>>>
>>> Looks correct, but this should be
>>>
>>> 	if (port_has_data(port))
>>>
>>> instead.
>>
>> That certainly works for me (as in will still fix the bug I'm hitting), but
>> quoting from "man 2 select":
>>
>>         Three  independent  sets of file descriptors are watched.  Those listed
>>         in readfds will be watched to see if characters  become  available  for
>>         reading  (more  precisely, to see if a read will not block; in particu‐
>>         lar, a file descriptor is also ready on end-of-file)
>>
>> Notice the "a file descriptor is also ready on end-of-file", and
>> port_fops_read treats the host not being connected as eof (it returns 0
>> in that case). So from an API pov I'm not sure what is correct.
>
> poll(2) says:
>
>                POLLIN There is data to read.
>
> That makes it simple.
>
> Also, we also set POLLHUP in case the host is disconnected, so if host
> is not connected while there's data to read, we'll get POLLIN|POLLHUP in
> revents, and further reads will be blocked.
>

Ah, I completely missed the POLLHUP getting set thingie. I guess that will
get used by the kernel to also not block if any fds have it said in select's
readfds argument.

One new patch coming up!

Regards,

Hans