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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

I'm willing to go with the poll manpage and use port_has_data().

		Amit
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux