Re: PATCH: virtio_console: Fix poll blocking even though there is data to read

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 09/15/2010 03:25 PM, Amit Shah wrote:
> On (Wed) Sep 15 2010 [15:04:53], Hans de Goede wrote:
>> Hi All,
>>
>> I found this while working on a Linux agent for spice, the symptom I was
>> seeing was select blocking on the spice vdagent virtio serial port even
>> though there were messages queued up there.
>>
>> I found this while working on a Linux agent for spice, the symptom I was
>> seeing was select blocking on the spice vdagent virtio serial port even
>> though there were messages queued up there.
>>
>> virtio_console's port_fops_poll checks port->inbuf != NULL to determine if
>> read won't block. However if an application reads enough bytes from inbuf
>> through port_fops_read, to empty the current port->inbuf, port->inbuf
>> will be NULL even though there may be buffers left in the virtqueue.
>>
>> This causes poll() to block even though there is data to be read, this patch
>> fixes this by using the alredy defined will_read_block utility function
>> instead of the port->inbuf != NULL check.
>>
>> Signed-off-By: Hans de Goede<hdegoede@redhat.com>
>>
>> Regards,
>>
>> Hans
>
>> diff -up linux-2.6.35.x86_64/drivers/char/virtio_console.c~ linux-2.6.35.x86_64/drivers/char/virtio_console.c
>> --- 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.

Regards,

Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel