IO-Uring Archive on lore.kernel.org
 help / color / Atom feed
* TODO list item - multiple poll waitqueues
@ 2020-02-10 15:12 Jens Axboe
  2020-02-10 20:53 ` Jens Axboe
  0 siblings, 1 reply; 2+ messages in thread
From: Jens Axboe @ 2020-02-10 15:12 UTC (permalink / raw)
  To: io-uring

Hi,

This has been on my TODO list for a while, just haven't gotten around to
it.

The issue is that some drivers use multiple waitqueues for poll, which
doesn't work with POLL_ADD. io_poll_queue_proc() checks for this and
fails it:

static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head,
			       struct poll_table_struct *p)
{
	struct io_poll_table *pt = container_of(p, struct io_poll_table, pt);

	if (unlikely(pt->req->poll.head)) {
		pt->error = -EINVAL;
		return;
	}

	pt->error = 0;
	pt->req->poll.head = head;
	add_wait_queue(head, &pt->req->poll.wait);
}

since we just have the one waitqueue on the io_uring side. Most notably
affected are TTYs, I've also noticed that /dev/random does the same
thing, and recently pipes as well.

This is a problem for event handlers, in that not all file types work
reliably with POLL_ADD. Note that this also affects the aio poll
implementation, unsurprisingly.

If anyone has the inclination to look into this, that'd be great.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: TODO list item - multiple poll waitqueues
  2020-02-10 15:12 TODO list item - multiple poll waitqueues Jens Axboe
@ 2020-02-10 20:53 ` Jens Axboe
  0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2020-02-10 20:53 UTC (permalink / raw)
  To: io-uring

On 2/10/20 8:12 AM, Jens Axboe wrote:
> Hi,
> 
> This has been on my TODO list for a while, just haven't gotten around to
> it.
> 
> The issue is that some drivers use multiple waitqueues for poll, which
> doesn't work with POLL_ADD. io_poll_queue_proc() checks for this and
> fails it:
> 
> static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head,
> 			       struct poll_table_struct *p)
> {
> 	struct io_poll_table *pt = container_of(p, struct io_poll_table, pt);
> 
> 	if (unlikely(pt->req->poll.head)) {
> 		pt->error = -EINVAL;
> 		return;
> 	}
> 
> 	pt->error = 0;
> 	pt->req->poll.head = head;
> 	add_wait_queue(head, &pt->req->poll.wait);
> }
> 
> since we just have the one waitqueue on the io_uring side. Most notably
> affected are TTYs, I've also noticed that /dev/random does the same
> thing, and recently pipes as well.
> 
> This is a problem for event handlers, in that not all file types work
> reliably with POLL_ADD. Note that this also affects the aio poll
> implementation, unsurprisingly.
> 
> If anyone has the inclination to look into this, that'd be great.

Of course after sending this one, I did take a look and hacked up
some support for it. Not super happy with it, but I'll send it out
for proper review and hopefully we can turn it into something that's
5.6 material.

Forgot to mention that this is particularly sucky since pipes were
recently switched to using multiple waitqueues.

https://git.kernel.dk/cgit/linux-block/log/?h=io_uring-5.6-poll

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 15:12 TODO list item - multiple poll waitqueues Jens Axboe
2020-02-10 20:53 ` Jens Axboe

IO-Uring Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/io-uring/0 io-uring/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 io-uring io-uring/ https://lore.kernel.org/io-uring \
		io-uring@vger.kernel.org
	public-inbox-index io-uring

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.io-uring


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git