* [PATCH v2 0/4] forced sync mshot read + cleanups
@ 2025-02-19 1:33 Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 1/4] io_uring/rw: forbid multishot async reads Pavel Begunkov
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Pavel Begunkov @ 2025-02-19 1:33 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
A respin of the patch forcing mshot reads to be executed synchornously
with cleanups on top. Let me know if you want me to separate the set,
as ideally patches should target different versions.
v2: clarified commit message
+ patches 2-4
Pavel Begunkov (4):
io_uring/rw: forbid multishot async reads
io_uring/rw: don't directly use ki_complete
io_uring/rw: move ki_complete init into prep
io_uring/rw: clean up mshot forced sync mode
io_uring/rw.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
--
2.48.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/4] io_uring/rw: forbid multishot async reads
2025-02-19 1:33 [PATCH v2 0/4] forced sync mshot read + cleanups Pavel Begunkov
@ 2025-02-19 1:33 ` Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 2/4] io_uring/rw: don't directly use ki_complete Pavel Begunkov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2025-02-19 1:33 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence, chase xd
At the moment we can't sanely handle queuing an async request from a
multishot context, so disable them. It shouldn't matter as pollable
files / socekts don't normally do async.
Patching it in __io_read() is not the cleanest way, but it's simpler
than other options, so let's fix it there and clean up on top.
Cc: [email protected]
Reported-by: chase xd <[email protected]>
Fixes: fc68fcda04910 ("io_uring/rw: add support for IORING_OP_READ_MULTISHOT")
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/rw.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 7aa1e4c9f64a..e8efd97fdee5 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -880,7 +880,15 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(ret))
return ret;
- ret = io_iter_do_read(rw, &io->iter);
+ if (unlikely(req->opcode == IORING_OP_READ_MULTISHOT)) {
+ void *cb_copy = rw->kiocb.ki_complete;
+
+ rw->kiocb.ki_complete = NULL;
+ ret = io_iter_do_read(rw, &io->iter);
+ rw->kiocb.ki_complete = cb_copy;
+ } else {
+ ret = io_iter_do_read(rw, &io->iter);
+ }
/*
* Some file systems like to return -EOPNOTSUPP for an IOCB_NOWAIT
@@ -904,7 +912,8 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
} else if (ret == -EIOCBQUEUED) {
return IOU_ISSUE_SKIP_COMPLETE;
} else if (ret == req->cqe.res || ret <= 0 || !force_nonblock ||
- (req->flags & REQ_F_NOWAIT) || !need_complete_io(req)) {
+ (req->flags & REQ_F_NOWAIT) || !need_complete_io(req) ||
+ (issue_flags & IO_URING_F_MULTISHOT)) {
/* read all, failed, already did sync or don't want to retry */
goto done;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] io_uring/rw: don't directly use ki_complete
2025-02-19 1:33 [PATCH v2 0/4] forced sync mshot read + cleanups Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 1/4] io_uring/rw: forbid multishot async reads Pavel Begunkov
@ 2025-02-19 1:33 ` Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 3/4] io_uring/rw: move ki_complete init into prep Pavel Begunkov
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2025-02-19 1:33 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
We want to avoid checking ->ki_complete directly in the io_uring
completion path. Fortunately we have only two callback the selection
of which depend on the ring constant flags, i.e. IOPOLL, so use that
to infer the function.
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/rw.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/io_uring/rw.c b/io_uring/rw.c
index e8efd97fdee5..27ccc82d7843 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -563,8 +563,10 @@ static void io_complete_rw_iopoll(struct kiocb *kiocb, long res)
smp_store_release(&req->iopoll_completed, 1);
}
-static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret)
+static inline void io_rw_done(struct io_kiocb *req, ssize_t ret)
{
+ struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
+
/* IO was queued async, completion will happen later */
if (ret == -EIOCBQUEUED)
return;
@@ -586,8 +588,10 @@ static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret)
}
}
- INDIRECT_CALL_2(kiocb->ki_complete, io_complete_rw_iopoll,
- io_complete_rw, kiocb, ret);
+ if (req->ctx->flags & IORING_SETUP_IOPOLL)
+ io_complete_rw_iopoll(&rw->kiocb, ret);
+ else
+ io_complete_rw(&rw->kiocb, ret);
}
static int kiocb_done(struct io_kiocb *req, ssize_t ret,
@@ -598,7 +602,7 @@ static int kiocb_done(struct io_kiocb *req, ssize_t ret,
if (ret >= 0 && req->flags & REQ_F_CUR_POS)
req->file->f_pos = rw->kiocb.ki_pos;
- if (ret >= 0 && (rw->kiocb.ki_complete == io_complete_rw)) {
+ if (ret >= 0 && !(req->ctx->flags & IORING_SETUP_IOPOLL)) {
__io_complete_rw_common(req, ret);
/*
* Safe to call io_end from here as we're inline
@@ -609,7 +613,7 @@ static int kiocb_done(struct io_kiocb *req, ssize_t ret,
io_req_rw_cleanup(req, issue_flags);
return IOU_OK;
} else {
- io_rw_done(&rw->kiocb, ret);
+ io_rw_done(req, ret);
}
return IOU_ISSUE_SKIP_COMPLETE;
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] io_uring/rw: move ki_complete init into prep
2025-02-19 1:33 [PATCH v2 0/4] forced sync mshot read + cleanups Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 1/4] io_uring/rw: forbid multishot async reads Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 2/4] io_uring/rw: don't directly use ki_complete Pavel Begunkov
@ 2025-02-19 1:33 ` Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 4/4] io_uring/rw: clean up mshot forced sync mode Pavel Begunkov
2025-02-19 20:56 ` [PATCH v2 0/4] forced sync mshot read + cleanups Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2025-02-19 1:33 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
Initialise ki_complete during request prep stage, we'll depend on it not
being reset during issue in the following patch.
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/rw.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 27ccc82d7843..d16256505389 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -23,6 +23,9 @@
#include "poll.h"
#include "rw.h"
+static void io_complete_rw(struct kiocb *kiocb, long res);
+static void io_complete_rw_iopoll(struct kiocb *kiocb, long res);
+
struct io_rw {
/* NOTE: kiocb has the file as the first member, so don't do it here */
struct kiocb kiocb;
@@ -289,6 +292,11 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
rw->kiocb.dio_complete = NULL;
rw->kiocb.ki_flags = 0;
+ if (req->ctx->flags & IORING_SETUP_IOPOLL)
+ rw->kiocb.ki_complete = io_complete_rw_iopoll;
+ else
+ rw->kiocb.ki_complete = io_complete_rw;
+
rw->addr = READ_ONCE(sqe->addr);
rw->len = READ_ONCE(sqe->len);
rw->flags = READ_ONCE(sqe->rw_flags);
@@ -817,10 +825,8 @@ static int io_rw_init_file(struct io_kiocb *req, fmode_t mode, int rw_type)
if (ctx->flags & IORING_SETUP_IOPOLL) {
if (!(kiocb->ki_flags & IOCB_DIRECT) || !file->f_op->iopoll)
return -EOPNOTSUPP;
-
kiocb->private = NULL;
kiocb->ki_flags |= IOCB_HIPRI;
- kiocb->ki_complete = io_complete_rw_iopoll;
req->iopoll_completed = 0;
if (ctx->flags & IORING_SETUP_HYBRID_IOPOLL) {
/* make sure every req only blocks once*/
@@ -830,7 +836,6 @@ static int io_rw_init_file(struct io_kiocb *req, fmode_t mode, int rw_type)
} else {
if (kiocb->ki_flags & IOCB_HIPRI)
return -EINVAL;
- kiocb->ki_complete = io_complete_rw;
}
if (req->flags & REQ_F_HAS_METADATA) {
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] io_uring/rw: clean up mshot forced sync mode
2025-02-19 1:33 [PATCH v2 0/4] forced sync mshot read + cleanups Pavel Begunkov
` (2 preceding siblings ...)
2025-02-19 1:33 ` [PATCH v2 3/4] io_uring/rw: move ki_complete init into prep Pavel Begunkov
@ 2025-02-19 1:33 ` Pavel Begunkov
2025-02-19 20:56 ` [PATCH v2 0/4] forced sync mshot read + cleanups Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2025-02-19 1:33 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence
Move code forcing synchronous execution of multishot read requests out
a more generic __io_read().
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/rw.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/io_uring/rw.c b/io_uring/rw.c
index d16256505389..9edc6baebd01 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -889,15 +889,7 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(ret))
return ret;
- if (unlikely(req->opcode == IORING_OP_READ_MULTISHOT)) {
- void *cb_copy = rw->kiocb.ki_complete;
-
- rw->kiocb.ki_complete = NULL;
- ret = io_iter_do_read(rw, &io->iter);
- rw->kiocb.ki_complete = cb_copy;
- } else {
- ret = io_iter_do_read(rw, &io->iter);
- }
+ ret = io_iter_do_read(rw, &io->iter);
/*
* Some file systems like to return -EOPNOTSUPP for an IOCB_NOWAIT
@@ -995,6 +987,8 @@ int io_read_mshot(struct io_kiocb *req, unsigned int issue_flags)
if (!io_file_can_poll(req))
return -EBADFD;
+ /* make it sync, multishot doesn't support async execution */
+ rw->kiocb.ki_complete = NULL;
ret = __io_read(req, issue_flags);
/*
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/4] forced sync mshot read + cleanups
2025-02-19 1:33 [PATCH v2 0/4] forced sync mshot read + cleanups Pavel Begunkov
` (3 preceding siblings ...)
2025-02-19 1:33 ` [PATCH v2 4/4] io_uring/rw: clean up mshot forced sync mode Pavel Begunkov
@ 2025-02-19 20:56 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2025-02-19 20:56 UTC (permalink / raw)
To: io-uring, Pavel Begunkov
On Wed, 19 Feb 2025 01:33:36 +0000, Pavel Begunkov wrote:
> A respin of the patch forcing mshot reads to be executed synchornously
> with cleanups on top. Let me know if you want me to separate the set,
> as ideally patches should target different versions.
>
> v2: clarified commit message
> + patches 2-4
>
> [...]
Applied, thanks!
[1/4] io_uring/rw: forbid multishot async reads
commit: 67b0025d19f99fb9fbb8b62e6975553c183f3a16
[2/4] io_uring/rw: don't directly use ki_complete
commit: 4e43133c6f2319d3e205ea986c507b25d9b41e64
[3/4] io_uring/rw: move ki_complete init into prep
commit: 74f3e875268f1ce2dd01029c29560263212077df
[4/4] io_uring/rw: clean up mshot forced sync mode
commit: 4614de748e78a295ee9b1f54ca87280b101fbdf0
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-19 20:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-19 1:33 [PATCH v2 0/4] forced sync mshot read + cleanups Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 1/4] io_uring/rw: forbid multishot async reads Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 2/4] io_uring/rw: don't directly use ki_complete Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 3/4] io_uring/rw: move ki_complete init into prep Pavel Begunkov
2025-02-19 1:33 ` [PATCH v2 4/4] io_uring/rw: clean up mshot forced sync mode Pavel Begunkov
2025-02-19 20:56 ` [PATCH v2 0/4] forced sync mshot read + 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