public inbox for [email protected]
 help / color / mirror / Atom feed
* [RFC 0/1] io_uring: process requests completed with -EAGAIN on poll list
@ 2020-04-02 18:54 Bijan Mottahedeh
  2020-04-02 18:54 ` [RFC 1/1] " Bijan Mottahedeh
  0 siblings, 1 reply; 3+ messages in thread
From: Bijan Mottahedeh @ 2020-04-02 18:54 UTC (permalink / raw)
  To: axboe; +Cc: io-uring

Running fio polled tests with a large number of jobs ends up with stuck
polling threads.  I think this because of polling for requests that have
completed with -EAGAIN.

Running with the RFC applied, and with sufficiently large nvme timeout
values, the fio tests complete.

The RFC creates a retry list similar to the done list.  I'm not sure
if that's the best approach and whether there may be ordering issues
processing the two lists but I haven't seen any problems.

Bijan Mottahedeh (1):
  io_uring: process requests completed with -EAGAIN on poll list

 fs/io_uring.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

-- 
1.8.3.1


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

* [RFC 1/1] io_uring: process requests completed with -EAGAIN on poll list
  2020-04-02 18:54 [RFC 0/1] io_uring: process requests completed with -EAGAIN on poll list Bijan Mottahedeh
@ 2020-04-02 18:54 ` Bijan Mottahedeh
  2020-04-03 19:38   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Bijan Mottahedeh @ 2020-04-02 18:54 UTC (permalink / raw)
  To: axboe; +Cc: io-uring

A request that completes with an -EAGAIN result after it has been added
to the poll list, will not be removed from that list in io_do_iopoll()
because the f_op->iopoll() will not succeed for that request.

Maintain a retryable local list similar to the done list, and explicity
reissue requests with an -EAGAIN result.

Signed-off-by: Bijan Mottahedeh <[email protected]>
---
 fs/io_uring.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 62bd410..a3e3a4e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1738,11 +1738,24 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 	io_free_req_many(ctx, &rb);
 }
 
+static void io_iopoll_queue(struct list_head *again)
+{
+	struct io_kiocb *req;
+
+	while (!list_empty(again)) {
+		req = list_first_entry(again, struct io_kiocb, list);
+		list_del(&req->list);
+		refcount_inc(&req->refs);
+		io_queue_async_work(req);
+	}
+}
+
 static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
 			long min)
 {
 	struct io_kiocb *req, *tmp;
 	LIST_HEAD(done);
+	LIST_HEAD(again);
 	bool spin;
 	int ret;
 
@@ -1757,9 +1770,9 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
 		struct kiocb *kiocb = &req->rw.kiocb;
 
 		/*
-		 * Move completed entries to our local list. If we find a
-		 * request that requires polling, break out and complete
-		 * the done list first, if we have entries there.
+		 * Move completed and retryable entries to our local lists.
+		 * If we find a request that requires polling, break out
+		 * and complete those lists first, if we have entries there.
 		 */
 		if (req->flags & REQ_F_IOPOLL_COMPLETED) {
 			list_move_tail(&req->list, &done);
@@ -1768,6 +1781,13 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
 		if (!list_empty(&done))
 			break;
 
+		if (req->result == -EAGAIN) {
+			list_move_tail(&req->list, &again);
+			continue;
+		}
+		if (!list_empty(&again))
+			break;
+
 		ret = kiocb->ki_filp->f_op->iopoll(kiocb, spin);
 		if (ret < 0)
 			break;
@@ -1780,6 +1800,9 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
 	if (!list_empty(&done))
 		io_iopoll_complete(ctx, nr_events, &done);
 
+	if (!list_empty(&again))
+		io_iopoll_queue(&again);
+
 	return ret;
 }
 
-- 
1.8.3.1


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

* Re: [RFC 1/1] io_uring: process requests completed with -EAGAIN on poll list
  2020-04-02 18:54 ` [RFC 1/1] " Bijan Mottahedeh
@ 2020-04-03 19:38   ` Jens Axboe
  0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-04-03 19:38 UTC (permalink / raw)
  To: Bijan Mottahedeh; +Cc: io-uring

On 4/2/20 12:54 PM, Bijan Mottahedeh wrote:
> A request that completes with an -EAGAIN result after it has been added
> to the poll list, will not be removed from that list in io_do_iopoll()
> because the f_op->iopoll() will not succeed for that request.
> 
> Maintain a retryable local list similar to the done list, and explicity
> reissue requests with an -EAGAIN result.

I think this looks reasonable. You could make the loop here:

> +static void io_iopoll_queue(struct list_head *again)
> +{
> +	struct io_kiocb *req;
> +
> +	while (!list_empty(again)) {
> +		req = list_first_entry(again, struct io_kiocb, list);
> +		list_del(&req->list);
> +		refcount_inc(&req->refs);
> +		io_queue_async_work(req);
> +	}
> +}

be:
	do {
		req = list_first_entry(again, struct io_kiocb, list);
		list_del(&req->list);
		refcount_inc(&req->refs);
		io_queue_async_work(req);
	} while (!list_empty(again));

as you always enter with it non-empty, at least that eliminates one
check per list.

We could also issue inline again, instead of punting these async. But
probably not worth the bother.

-- 
Jens Axboe


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

end of thread, other threads:[~2020-04-03 19:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-02 18:54 [RFC 0/1] io_uring: process requests completed with -EAGAIN on poll list Bijan Mottahedeh
2020-04-02 18:54 ` [RFC 1/1] " Bijan Mottahedeh
2020-04-03 19:38   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox