public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 0/5] reissue fix and various cleanups
@ 2025-03-24 15:32 Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 1/5] io_uring: fix retry handling off iowq Pavel Begunkov
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Pavel Begunkov @ 2025-03-24 15:32 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

Patch 1 handle REQ_F_REISSUE off the iowq path, The rest are
random cleanups.

Pavel Begunkov (5):
  io_uring: fix retry handling off iowq
  io_uring: defer iowq cqe overflow via task_work
  io_uring: open code __io_post_aux_cqe()
  io_uring: rename "min" arg in io_iopoll_check()
  io_uring: move min_events sanitisation

 io_uring/io_uring.c | 49 ++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

-- 
2.48.1


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

* [PATCH 1/5] io_uring: fix retry handling off iowq
  2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
@ 2025-03-24 15:32 ` Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 2/5] io_uring: defer iowq cqe overflow via task_work Pavel Begunkov
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2025-03-24 15:32 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

io_req_complete_post() doesn't handle reissue and if called with a
REQ_F_REISSUE request it might post extra unexpected completions. Fix it
by pushing into flush_completion via task work.

Fixes: d803d123948fe ("io_uring/rw: handle -EAGAIN retry at IO completion time")
Signed-off-by: Pavel Begunkov <[email protected]>
---
 io_uring/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index e1128b9551aa..e6c462948273 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -904,7 +904,7 @@ static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
 	 * Handle special CQ sync cases via task_work. DEFER_TASKRUN requires
 	 * the submitter task context, IOPOLL protects with uring_lock.
 	 */
-	if (ctx->lockless_cq) {
+	if (ctx->lockless_cq || (req->flags & REQ_F_REISSUE)) {
 		req->io_task_work.func = io_req_task_complete;
 		io_req_task_work_add(req);
 		return;
-- 
2.48.1


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

* [PATCH 2/5] io_uring: defer iowq cqe overflow via task_work
  2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 1/5] io_uring: fix retry handling off iowq Pavel Begunkov
@ 2025-03-24 15:32 ` Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 3/5] io_uring: open code __io_post_aux_cqe() Pavel Begunkov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2025-03-24 15:32 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

Don't handle CQE overflows in io_req_complete_post() and defer it to
flush_completions. It cuts some duplication, and I also want to limit
the number of places directly overflowing completions.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 io_uring/io_uring.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index e6c462948273..1fcfe62cecd9 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -892,6 +892,7 @@ bool io_req_post_cqe(struct io_kiocb *req, s32 res, u32 cflags)
 static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
 {
 	struct io_ring_ctx *ctx = req->ctx;
+	bool completed = true;
 
 	/*
 	 * All execution paths but io-wq use the deferred completions by
@@ -905,18 +906,20 @@ static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
 	 * the submitter task context, IOPOLL protects with uring_lock.
 	 */
 	if (ctx->lockless_cq || (req->flags & REQ_F_REISSUE)) {
+defer_complete:
 		req->io_task_work.func = io_req_task_complete;
 		io_req_task_work_add(req);
 		return;
 	}
 
 	io_cq_lock(ctx);
-	if (!(req->flags & REQ_F_CQE_SKIP)) {
-		if (!io_fill_cqe_req(ctx, req))
-			io_req_cqe_overflow(req);
-	}
+	if (!(req->flags & REQ_F_CQE_SKIP))
+		completed = io_fill_cqe_req(ctx, req);
 	io_cq_unlock_post(ctx);
 
+	if (!completed)
+		goto defer_complete;
+
 	/*
 	 * We don't free the request here because we know it's called from
 	 * io-wq only, which holds a reference, so it cannot be the last put.
-- 
2.48.1


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

* [PATCH 3/5] io_uring: open code __io_post_aux_cqe()
  2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 1/5] io_uring: fix retry handling off iowq Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 2/5] io_uring: defer iowq cqe overflow via task_work Pavel Begunkov
@ 2025-03-24 15:32 ` Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 4/5] io_uring: rename "min" arg in io_iopoll_check() Pavel Begunkov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2025-03-24 15:32 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

There is no reason to keep __io_post_aux_cqe() separately from
io_post_aux_cqe().

Signed-off-by: Pavel Begunkov <[email protected]>
---
 io_uring/io_uring.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 1fcfe62cecd9..df3685803ef7 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -834,24 +834,14 @@ static bool io_fill_cqe_aux(struct io_ring_ctx *ctx, u64 user_data, s32 res,
 	return false;
 }
 
-static bool __io_post_aux_cqe(struct io_ring_ctx *ctx, u64 user_data, s32 res,
-			      u32 cflags)
+bool io_post_aux_cqe(struct io_ring_ctx *ctx, u64 user_data, s32 res, u32 cflags)
 {
 	bool filled;
 
+	io_cq_lock(ctx);
 	filled = io_fill_cqe_aux(ctx, user_data, res, cflags);
 	if (!filled)
 		filled = io_cqring_event_overflow(ctx, user_data, res, cflags, 0, 0);
-
-	return filled;
-}
-
-bool io_post_aux_cqe(struct io_ring_ctx *ctx, u64 user_data, s32 res, u32 cflags)
-{
-	bool filled;
-
-	io_cq_lock(ctx);
-	filled = __io_post_aux_cqe(ctx, user_data, res, cflags);
 	io_cq_unlock_post(ctx);
 	return filled;
 }
-- 
2.48.1


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

* [PATCH 4/5] io_uring: rename "min" arg in io_iopoll_check()
  2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
                   ` (2 preceding siblings ...)
  2025-03-24 15:32 ` [PATCH 3/5] io_uring: open code __io_post_aux_cqe() Pavel Begunkov
@ 2025-03-24 15:32 ` Pavel Begunkov
  2025-03-24 15:32 ` [PATCH 5/5] io_uring: move min_events sanitisation Pavel Begunkov
  2025-03-25 13:06 ` [PATCH 0/5] reissue fix and various cleanups Jens Axboe
  5 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2025-03-24 15:32 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

Don't name arguments "min", it shadows the namesake function.
min_events is also more consistent.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 io_uring/io_uring.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index df3685803ef7..6022a00de95b 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1505,7 +1505,7 @@ static __cold void io_iopoll_try_reap_events(struct io_ring_ctx *ctx)
 	mutex_unlock(&ctx->uring_lock);
 }
 
-static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
+static int io_iopoll_check(struct io_ring_ctx *ctx, long min_events)
 {
 	unsigned int nr_events = 0;
 	unsigned long check_cq;
@@ -1551,7 +1551,7 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
 		    io_task_work_pending(ctx)) {
 			u32 tail = ctx->cached_cq_tail;
 
-			(void) io_run_local_work_locked(ctx, min);
+			(void) io_run_local_work_locked(ctx, min_events);
 
 			if (task_work_pending(current) ||
 			    wq_list_empty(&ctx->iopoll_list)) {
@@ -1564,7 +1564,7 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
 			    wq_list_empty(&ctx->iopoll_list))
 				break;
 		}
-		ret = io_do_iopoll(ctx, !min);
+		ret = io_do_iopoll(ctx, !min_events);
 		if (unlikely(ret < 0))
 			return ret;
 
@@ -1574,7 +1574,7 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
 			break;
 
 		nr_events += ret;
-	} while (nr_events < min);
+	} while (nr_events < min_events);
 
 	return 0;
 }
-- 
2.48.1


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

* [PATCH 5/5] io_uring: move min_events sanitisation
  2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
                   ` (3 preceding siblings ...)
  2025-03-24 15:32 ` [PATCH 4/5] io_uring: rename "min" arg in io_iopoll_check() Pavel Begunkov
@ 2025-03-24 15:32 ` Pavel Begunkov
  2025-03-25 13:06 ` [PATCH 0/5] reissue fix and various cleanups Jens Axboe
  5 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2025-03-24 15:32 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

iopoll and normal waiting already duplicate min_completion truncation,
so move them inside the corresponding routines.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 io_uring/io_uring.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 6022a00de95b..4ea684a17d01 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1505,11 +1505,13 @@ static __cold void io_iopoll_try_reap_events(struct io_ring_ctx *ctx)
 	mutex_unlock(&ctx->uring_lock);
 }
 
-static int io_iopoll_check(struct io_ring_ctx *ctx, long min_events)
+static int io_iopoll_check(struct io_ring_ctx *ctx, unsigned int min_events)
 {
 	unsigned int nr_events = 0;
 	unsigned long check_cq;
 
+	min_events = min(min_events, ctx->cq_entries);
+
 	lockdep_assert_held(&ctx->uring_lock);
 
 	if (!io_allowed_run_tw(ctx))
@@ -2537,6 +2539,8 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, u32 flags,
 	ktime_t start_time;
 	int ret;
 
+	min_events = min_t(int, min_events, ctx->cq_entries);
+
 	if (!io_allowed_run_tw(ctx))
 		return -EEXIST;
 	if (io_local_work_pending(ctx))
@@ -3420,22 +3424,16 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 			mutex_lock(&ctx->uring_lock);
 iopoll_locked:
 			ret2 = io_validate_ext_arg(ctx, flags, argp, argsz);
-			if (likely(!ret2)) {
-				min_complete = min(min_complete,
-						   ctx->cq_entries);
+			if (likely(!ret2))
 				ret2 = io_iopoll_check(ctx, min_complete);
-			}
 			mutex_unlock(&ctx->uring_lock);
 		} else {
 			struct ext_arg ext_arg = { .argsz = argsz };
 
 			ret2 = io_get_ext_arg(ctx, flags, argp, &ext_arg);
-			if (likely(!ret2)) {
-				min_complete = min(min_complete,
-						   ctx->cq_entries);
+			if (likely(!ret2))
 				ret2 = io_cqring_wait(ctx, min_complete, flags,
 						      &ext_arg);
-			}
 		}
 
 		if (!ret) {
-- 
2.48.1


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

* Re: [PATCH 0/5] reissue fix and various cleanups
  2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
                   ` (4 preceding siblings ...)
  2025-03-24 15:32 ` [PATCH 5/5] io_uring: move min_events sanitisation Pavel Begunkov
@ 2025-03-25 13:06 ` Jens Axboe
  5 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2025-03-25 13:06 UTC (permalink / raw)
  To: io-uring, Pavel Begunkov


On Mon, 24 Mar 2025 15:32:31 +0000, Pavel Begunkov wrote:
> Patch 1 handle REQ_F_REISSUE off the iowq path, The rest are
> random cleanups.
> 
> Pavel Begunkov (5):
>   io_uring: fix retry handling off iowq
>   io_uring: defer iowq cqe overflow via task_work
>   io_uring: open code __io_post_aux_cqe()
>   io_uring: rename "min" arg in io_iopoll_check()
>   io_uring: move min_events sanitisation
> 
> [...]

Applied, thanks!

[1/5] io_uring: fix retry handling off iowq
      (no commit info)
[2/5] io_uring: defer iowq cqe overflow via task_work
      (no commit info)
[3/5] io_uring: open code __io_post_aux_cqe()
      (no commit info)
[4/5] io_uring: rename "min" arg in io_iopoll_check()
      (no commit info)
[5/5] io_uring: move min_events sanitisation
      (no commit info)

Best regards,
-- 
Jens Axboe




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

end of thread, other threads:[~2025-03-25 13:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24 15:32 [PATCH 0/5] reissue fix and various cleanups Pavel Begunkov
2025-03-24 15:32 ` [PATCH 1/5] io_uring: fix retry handling off iowq Pavel Begunkov
2025-03-24 15:32 ` [PATCH 2/5] io_uring: defer iowq cqe overflow via task_work Pavel Begunkov
2025-03-24 15:32 ` [PATCH 3/5] io_uring: open code __io_post_aux_cqe() Pavel Begunkov
2025-03-24 15:32 ` [PATCH 4/5] io_uring: rename "min" arg in io_iopoll_check() Pavel Begunkov
2025-03-24 15:32 ` [PATCH 5/5] io_uring: move min_events sanitisation Pavel Begunkov
2025-03-25 13:06 ` [PATCH 0/5] reissue fix and various cleanups Jens Axboe

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