All of lore.kernel.org
 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 <asml.silence@gmail.com>
---
 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 <asml.silence@gmail.com>
---
 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 <asml.silence@gmail.com>
---
 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 <asml.silence@gmail.com>
---
 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 <asml.silence@gmail.com>
---
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.