public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Amit Shah <amit.shah@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	virtualization@lists.linux-foundation.org, kvm@vger.kernel.org,
	spice-devel@lists.freedesktop.org,
	"Michael Kerrisk" <mtk.manpages@googlemail.com>
Subject: Re: PATCH: virtio_console: Fix poll blocking even though there is data to read
Date: Thu, 16 Sep 2010 15:32:54 +0930	[thread overview]
Message-ID: <201009161532.55462.rusty@rustcorp.com.au> (raw)
In-Reply-To: <20100915134624.GB12320@amit-laptop.redhat.com>

On Wed, 15 Sep 2010 11:16:24 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.

That's a documentation bug.  On a pipe, POLLIN is set when the other end
closes.  read() then returns 0 immediately.

poll() sets POLLIN when read() won't block, and people count on it.

Let's do that, please.
Rusty.

  parent reply	other threads:[~2010-09-16  6:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-15 13:04 PATCH: virtio_console: Fix poll blocking even though there is data to read Hans de Goede
2010-09-15 13:25 ` Amit Shah
2010-09-15 13:37   ` Hans de Goede
2010-09-15 13:46     ` Amit Shah
2010-09-15 14:02       ` Hans de Goede
2010-09-16  6:02       ` Rusty Russell [this message]
2010-09-16  6:22         ` Amit Shah
2010-09-16  7:16           ` Amit Shah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201009161532.55462.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=amit.shah@redhat.com \
    --cc=hdegoede@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtk.manpages@googlemail.com \
    --cc=spice-devel@lists.freedesktop.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox