* [PATCH for-6.2 v2 1/3] io_uring/msg_ring: fix flagging remote execution
2023-01-20 16:38 [PATCH for-6.2 v2 0/3] msg_ring fixes Pavel Begunkov
@ 2023-01-20 16:38 ` Pavel Begunkov
2023-01-20 16:38 ` [PATCH for-6.2 v2 2/3] io_uring/msg_ring: fix remote queue to disabled ring Pavel Begunkov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2023-01-20 16:38 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
There is a couple of problems with queueing a tw in io_msg_ring_data()
for remote execution. First, once we queue it the target ring can
go away and so setting IORING_SQ_TASKRUN there is not safe. Secondly,
the userspace might not expect IORING_SQ_TASKRUN.
Extract a helper and uniformly use TWA_SIGNAL without TWA_SIGNAL_NO_IPI
tricks for now, just as it was done in the original patch.
Fixes: 6d043ee1164ca ("io_uring: do msg_ring in target task via tw")
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/msg_ring.c | 40 +++++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index a333781565d3..bb868447dcdf 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -58,6 +58,25 @@ void io_msg_ring_cleanup(struct io_kiocb *req)
msg->src_file = NULL;
}
+static inline bool io_msg_need_remote(struct io_ring_ctx *target_ctx)
+{
+ if (!target_ctx->task_complete)
+ return false;
+ return current != target_ctx->submitter_task;
+}
+
+static int io_msg_exec_remote(struct io_kiocb *req, task_work_func_t func)
+{
+ struct io_ring_ctx *ctx = req->file->private_data;
+ struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+
+ init_task_work(&msg->tw, func);
+ if (task_work_add(ctx->submitter_task, &msg->tw, TWA_SIGNAL))
+ return -EOWNERDEAD;
+
+ return IOU_ISSUE_SKIP_COMPLETE;
+}
+
static void io_msg_tw_complete(struct callback_head *head)
{
struct io_msg *msg = container_of(head, struct io_msg, tw);
@@ -96,15 +115,8 @@ static int io_msg_ring_data(struct io_kiocb *req, unsigned int issue_flags)
if (msg->src_fd || msg->dst_fd || msg->flags)
return -EINVAL;
- if (target_ctx->task_complete && current != target_ctx->submitter_task) {
- init_task_work(&msg->tw, io_msg_tw_complete);
- if (task_work_add(target_ctx->submitter_task, &msg->tw,
- TWA_SIGNAL_NO_IPI))
- return -EOWNERDEAD;
-
- atomic_or(IORING_SQ_TASKRUN, &target_ctx->rings->sq_flags);
- return IOU_ISSUE_SKIP_COMPLETE;
- }
+ if (io_msg_need_remote(target_ctx))
+ return io_msg_exec_remote(req, io_msg_tw_complete);
ret = -EOVERFLOW;
if (target_ctx->flags & IORING_SETUP_IOPOLL) {
@@ -202,14 +214,8 @@ static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
req->flags |= REQ_F_NEED_CLEANUP;
}
- if (target_ctx->task_complete && current != target_ctx->submitter_task) {
- init_task_work(&msg->tw, io_msg_tw_fd_complete);
- if (task_work_add(target_ctx->submitter_task, &msg->tw,
- TWA_SIGNAL))
- return -EOWNERDEAD;
-
- return IOU_ISSUE_SKIP_COMPLETE;
- }
+ if (io_msg_need_remote(target_ctx))
+ return io_msg_exec_remote(req, io_msg_tw_fd_complete);
return io_msg_install_complete(req, issue_flags);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH for-6.2 v2 2/3] io_uring/msg_ring: fix remote queue to disabled ring
2023-01-20 16:38 [PATCH for-6.2 v2 0/3] msg_ring fixes Pavel Begunkov
2023-01-20 16:38 ` [PATCH for-6.2 v2 1/3] io_uring/msg_ring: fix flagging remote execution Pavel Begunkov
@ 2023-01-20 16:38 ` Pavel Begunkov
2023-01-20 16:38 ` [PATCH for-6.2 v2 3/3] io_uring/msg_ring: optimise with correct tw notify method Pavel Begunkov
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2023-01-20 16:38 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
IORING_SETUP_R_DISABLED rings don't have the submitter task set, so
it's not always safe to use ->submitter_task. Disallow posting msg_ring
messaged to disabled rings. Also add task NULL check for loosy sync
around testing for IORING_SETUP_R_DISABLED.
Fixes: 6d043ee1164ca ("io_uring: do msg_ring in target task via tw")
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/io_uring.c | 4 ++--
io_uring/msg_ring.c | 8 ++++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 2ac1cd8d23ea..0a4efada9b3c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3674,7 +3674,7 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
if (ctx->flags & IORING_SETUP_SINGLE_ISSUER
&& !(ctx->flags & IORING_SETUP_R_DISABLED))
- ctx->submitter_task = get_task_struct(current);
+ WRITE_ONCE(ctx->submitter_task, get_task_struct(current));
file = io_uring_get_file(ctx);
if (IS_ERR(file)) {
@@ -3868,7 +3868,7 @@ static int io_register_enable_rings(struct io_ring_ctx *ctx)
return -EBADFD;
if (ctx->flags & IORING_SETUP_SINGLE_ISSUER && !ctx->submitter_task)
- ctx->submitter_task = get_task_struct(current);
+ WRITE_ONCE(ctx->submitter_task, get_task_struct(current));
if (ctx->restrictions.registered)
ctx->restricted = 1;
diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index bb868447dcdf..15602a136821 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -69,6 +69,10 @@ static int io_msg_exec_remote(struct io_kiocb *req, task_work_func_t func)
{
struct io_ring_ctx *ctx = req->file->private_data;
struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+ struct task_struct *task = READ_ONCE(ctx->submitter_task);
+
+ if (unlikely(!task))
+ return -EOWNERDEAD;
init_task_work(&msg->tw, func);
if (task_work_add(ctx->submitter_task, &msg->tw, TWA_SIGNAL))
@@ -114,6 +118,8 @@ static int io_msg_ring_data(struct io_kiocb *req, unsigned int issue_flags)
if (msg->src_fd || msg->dst_fd || msg->flags)
return -EINVAL;
+ if (target_ctx->flags & IORING_SETUP_R_DISABLED)
+ return -EBADFD;
if (io_msg_need_remote(target_ctx))
return io_msg_exec_remote(req, io_msg_tw_complete);
@@ -206,6 +212,8 @@ static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
if (target_ctx == ctx)
return -EINVAL;
+ if (target_ctx->flags & IORING_SETUP_R_DISABLED)
+ return -EBADFD;
if (!src_file) {
src_file = io_msg_grab_file(req, issue_flags);
if (!src_file)
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH for-6.2 v2 3/3] io_uring/msg_ring: optimise with correct tw notify method
2023-01-20 16:38 [PATCH for-6.2 v2 0/3] msg_ring fixes Pavel Begunkov
2023-01-20 16:38 ` [PATCH for-6.2 v2 1/3] io_uring/msg_ring: fix flagging remote execution Pavel Begunkov
2023-01-20 16:38 ` [PATCH for-6.2 v2 2/3] io_uring/msg_ring: fix remote queue to disabled ring Pavel Begunkov
@ 2023-01-20 16:38 ` Pavel Begunkov
2023-01-20 16:45 ` (subset) [PATCH for-6.2 v2 0/3] msg_ring fixes Jens Axboe
2023-01-20 16:46 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2023-01-20 16:38 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
We may want to use TWA_SIGNAL_NO_IPI instead of TWA_SIGNAL if the target
ring is configured with IORING_SETUP_COOP_TASKRUN, change
io_msg_exec_remote() to use the target ring's ->notify_method.
The caveat is that we have to set IORING_SQ_TASKRUN if the rings asks
for it. However, once task_work_add() succeeds the target ring might go
away and so we grab a ctx reference to pin the ring until we set
IORING_SQ_TASKRUN.
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/msg_ring.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index 15602a136821..af802cd645b4 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -70,15 +70,22 @@ static int io_msg_exec_remote(struct io_kiocb *req, task_work_func_t func)
struct io_ring_ctx *ctx = req->file->private_data;
struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
struct task_struct *task = READ_ONCE(ctx->submitter_task);
+ int ret = IOU_ISSUE_SKIP_COMPLETE;
if (unlikely(!task))
return -EOWNERDEAD;
+ percpu_ref_get(&ctx->refs);
init_task_work(&msg->tw, func);
- if (task_work_add(ctx->submitter_task, &msg->tw, TWA_SIGNAL))
- return -EOWNERDEAD;
-
- return IOU_ISSUE_SKIP_COMPLETE;
+ if (task_work_add(ctx->submitter_task, &msg->tw, ctx->notify_method)) {
+ ret = -EOWNERDEAD;
+ goto out;
+ }
+ if (ctx->flags & IORING_SETUP_TASKRUN_FLAG)
+ atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);
+out:
+ percpu_ref_put(&ctx->refs);
+ return ret;
}
static void io_msg_tw_complete(struct callback_head *head)
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: (subset) [PATCH for-6.2 v2 0/3] msg_ring fixes
2023-01-20 16:38 [PATCH for-6.2 v2 0/3] msg_ring fixes Pavel Begunkov
` (2 preceding siblings ...)
2023-01-20 16:38 ` [PATCH for-6.2 v2 3/3] io_uring/msg_ring: optimise with correct tw notify method Pavel Begunkov
@ 2023-01-20 16:45 ` Jens Axboe
2023-01-20 16:46 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2023-01-20 16:45 UTC (permalink / raw)
To: io-uring, Pavel Begunkov
On Fri, 20 Jan 2023 16:38:04 +0000, Pavel Begunkov wrote:
> First two patches are msg_ring fixes. 3/3 can go 6.3
>
> v2: fail msg_ring'ing to disabled rings
>
> Pavel Begunkov (3):
> io_uring/msg_ring: fix flagging remote execution
> io_uring/msg_ring: fix remote queue to disabled ring
> io_uring/msg_ring: optimise with correct tw notify method
>
> [...]
Applied, thanks!
[1/3] io_uring/msg_ring: fix flagging remote execution
commit: 79ee63a3cfb1da7464814f1514fefb6a69615702
[2/3] io_uring/msg_ring: fix remote queue to disabled ring
commit: b85dfd720541567b2d56714aa3b1ece85d7ec971
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH for-6.2 v2 0/3] msg_ring fixes
2023-01-20 16:38 [PATCH for-6.2 v2 0/3] msg_ring fixes Pavel Begunkov
` (3 preceding siblings ...)
2023-01-20 16:45 ` (subset) [PATCH for-6.2 v2 0/3] msg_ring fixes Jens Axboe
@ 2023-01-20 16:46 ` Jens Axboe
4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2023-01-20 16:46 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 1/20/23 9:38 AM, Pavel Begunkov wrote:
> First two patches are msg_ring fixes. 3/3 can go 6.3
>
> v2: fail msg_ring'ing to disabled rings
>
> Pavel Begunkov (3):
> io_uring/msg_ring: fix flagging remote execution
> io_uring/msg_ring: fix remote queue to disabled ring
> io_uring/msg_ring: optimise with correct tw notify method
>
> io_uring/io_uring.c | 4 ++--
> io_uring/msg_ring.c | 55 +++++++++++++++++++++++++++++++--------------
> 2 files changed, 40 insertions(+), 19 deletions(-)
I took 1-2 for 6.2, and then will get 3 applied for 6.3 once these
two hit mainline.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread