* [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