On 2024-03-29 13:09, Jens Axboe wrote:
> All our task_work handling is targeted at the state in the io_kiocb
> itself, which is what it is being used for. However, MSG_RING rolls its
> own task_work handling, ignoring how that is usually done.
>
> In preparation for switching MSG_RING to be able to use the normal
> task_work handling, add io_req_task_work_add_remote() which allows the
> caller to pass in the target io_ring_ctx.
>
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
> ---
> io_uring/io_uring.c | 30 ++++++++++++++++++++++--------
> io_uring/io_uring.h | 2 ++
> 2 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index fddaefb9cbff..a311a244914b 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -1232,9 +1232,10 @@ void tctx_task_work(struct callback_head *cb)
> WARN_ON_ONCE(ret);
> }
>
> -static inline void io_req_local_work_add(struct io_kiocb *req, unsigned flags)
> +static inline void io_req_local_work_add(struct io_kiocb *req,
> + struct io_ring_ctx *ctx,
> + unsigned flags)
> {
> - struct io_ring_ctx *ctx = req->ctx;
> unsigned nr_wait, nr_tw, nr_tw_prev;
> struct llist_node *head;
>
> @@ -1300,9 +1301,10 @@ static inline void io_req_local_work_add(struct io_kiocb *req, unsigned flags)
> wake_up_state(ctx->submitter_task, TASK_INTERRUPTIBLE);
> }
>
> -static void io_req_normal_work_add(struct io_kiocb *req)
> +static void io_req_normal_work_add(struct io_kiocb *req,
> + struct task_struct *task)
> {
> - struct io_uring_task *tctx = req->task->io_uring;
> + struct io_uring_task *tctx = task->io_uring;
> struct io_ring_ctx *ctx = req->ctx;
>
> /* task_work already pending, we're done */
> @@ -1321,7 +1323,7 @@ static void io_req_normal_work_add(struct io_kiocb *req)
> return;
> }
>
> - if (likely(!task_work_add(req->task, &tctx->task_work, ctx->notify_method)))
> + if (likely(!task_work_add(task, &tctx->task_work, ctx->notify_method)))
> return;
>
> io_fallback_tw(tctx, false);
> @@ -1331,10 +1333,22 @@ void __io_req_task_work_add(struct io_kiocb *req, unsigned flags)
> {
> if (req->ctx->flags & IORING_SETUP_DEFER_TASKRUN) {
> rcu_read_lock();
> - io_req_local_work_add(req, flags);
> + io_req_local_work_add(req, req->ctx, flags);
> + rcu_read_unlock();
> + } else {
> + io_req_normal_work_add(req, req->task);
Why does this not require a READ_ONCE() like
io_req_task_work_add_remote()?
> + }
> +}
> +
> +void io_req_task_work_add_remote(struct io_kiocb *req, struct io_ring_ctx *ctx,
> + unsigned flags)
> +{
> + if (ctx->flags & IORING_SETUP_DEFER_TASKRUN) {
> + rcu_read_lock();
> + io_req_local_work_add(req, ctx, flags);
> rcu_read_unlock();
> } else {
> - io_req_normal_work_add(req);
> + io_req_normal_work_add(req, READ_ONCE(ctx->submitter_task));
> }
> }
>
> @@ -1348,7 +1362,7 @@ static void __cold io_move_task_work_from_local(struct io_ring_ctx *ctx)
> io_task_work.node);
>
> node = node->next;
> - io_req_normal_work_add(req);
> + io_req_normal_work_add(req, req->task);
> }
> }
>
> diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
> index 1eb65324792a..4155379ee586 100644
> --- a/io_uring/io_uring.h
> +++ b/io_uring/io_uring.h
> @@ -74,6 +74,8 @@ struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
> unsigned issue_flags);
>
> void __io_req_task_work_add(struct io_kiocb *req, unsigned flags);
> +void io_req_task_work_add_remote(struct io_kiocb *req, struct io_ring_ctx *ctx,
> + unsigned flags);
> bool io_alloc_async_data(struct io_kiocb *req);
> void io_req_task_queue(struct io_kiocb *req);
> void io_req_task_complete(struct io_kiocb *req, struct io_tw_state *ts);
