Commit bcd4c957 authored by Caleb Sander Mateos's avatar Caleb Sander Mateos Committed by Jens Axboe
Browse files

io_uring/msg_ring: drop unnecessary submitter_task checks 



__io_msg_ring_data() checks that the target_ctx isn't
IORING_SETUP_R_DISABLED before calling io_msg_data_remote(), which calls
io_msg_remote_post(). So submitter_task can't be modified concurrently
with the read in io_msg_remote_post(). Additionally, submitter_task must
exist, as io_msg_data_remote() is only called for io_msg_need_remote(),
i.e. task_complete is set, which requires IORING_SETUP_DEFER_TASKRUN,
which in turn requires IORING_SETUP_SINGLE_ISSUER. And submitter_task is
assigned in io_uring_create() or io_register_enable_rings() before
enabling any IORING_SETUP_SINGLE_ISSUER io_ring_ctx.
Similarly, io_msg_send_fd() checks IORING_SETUP_R_DISABLED and
io_msg_need_remote() before calling io_msg_fd_remote(). submitter_task
therefore can't be modified concurrently with the read in
io_msg_fd_remote() and must be non-null.
io_register_enable_rings() can't run concurrently because it's called
from io_uring_register() -> __io_uring_register() with uring_lock held.
Thus, replace the READ_ONCE() and WRITE_ONCE() of submitter_task with
plain loads and stores. And remove the NULL checks of submitter_task in
io_msg_remote_post() and io_msg_fd_remote().

Signed-off-by: default avatarCaleb Sander Mateos <csander@purestorage.com>
Reviewed-by: default avatarJoanne Koong <joannelkoong@gmail.com>
Reviewed-by: default avatarGabriel Krisman Bertazi <krisman@suse.de>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 7a8737e1

io_uring/io_uring.c

+1 −6
Original line number Diff line number Diff line
@@ -3637,13 +3637,8 @@ static __cold int io_uring_create(struct io_ctx_config *config) 
	}



	if (ctx->flags & IORING_SETUP_SINGLE_ISSUER

	    && !(ctx->flags & IORING_SETUP_R_DISABLED)) {

		/*

		 * Unlike io_register_enable_rings(), don't need WRITE_ONCE()

		 * since ctx isn't yet accessible from other tasks

		 */

	    && !(ctx->flags & IORING_SETUP_R_DISABLED))

		ctx->submitter_task = get_task_struct(current);

	}



	file = io_uring_get_file(ctx);

	if (IS_ERR(file)) {

io_uring/msg_ring.c

+5 −13
Original line number Diff line number Diff line
@@ -80,13 +80,9 @@ static void io_msg_tw_complete(struct io_tw_req tw_req, io_tw_token_t tw) 
	percpu_ref_put(&ctx->refs);

}



static int io_msg_remote_post(struct io_ring_ctx *ctx, struct io_kiocb *req,

static void io_msg_remote_post(struct io_ring_ctx *ctx, struct io_kiocb *req,

			      int res, u32 cflags, u64 user_data)

{

	if (!READ_ONCE(ctx->submitter_task)) {

		kfree_rcu(req, rcu_head);

		return -EOWNERDEAD;

	}

	req->opcode = IORING_OP_NOP;

	req->cqe.user_data = user_data;

	io_req_set_res(req, res, cflags);
@@ -95,7 +91,6 @@ static int io_msg_remote_post(struct io_ring_ctx *ctx, struct io_kiocb *req, 
	req->tctx = NULL;

	req->io_task_work.func = io_msg_tw_complete;

	io_req_task_work_add_remote(req, IOU_F_TWQ_LAZY_WAKE);

	return 0;

}



static int io_msg_data_remote(struct io_ring_ctx *target_ctx,
@@ -111,8 +106,8 @@ static int io_msg_data_remote(struct io_ring_ctx *target_ctx, 
	if (msg->flags & IORING_MSG_RING_FLAGS_PASS)

		flags = msg->cqe_flags;



	return io_msg_remote_post(target_ctx, target, msg->len, flags,

					msg->user_data);

	io_msg_remote_post(target_ctx, target, msg->len, flags, msg->user_data);

	return 0;

}



static int __io_msg_ring_data(struct io_ring_ctx *target_ctx,
@@ -127,7 +122,7 @@ static int __io_msg_ring_data(struct io_ring_ctx *target_ctx, 
		return -EINVAL;

	/*

	 * Keep IORING_SETUP_R_DISABLED check before submitter_task load

	 * in io_msg_data_remote() -> io_msg_remote_post()

	 * in io_msg_data_remote() -> io_req_task_work_add_remote()

	 */

	if (smp_load_acquire(&target_ctx->flags) & IORING_SETUP_R_DISABLED)

		return -EBADFD;
@@ -227,10 +222,7 @@ static int io_msg_fd_remote(struct io_kiocb *req) 
{

	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;

	struct task_struct *task = ctx->submitter_task;



	init_task_work(&msg->tw, io_msg_tw_fd_complete);

	if (task_work_add(task, &msg->tw, TWA_SIGNAL))

io_uring/register.c

+1 −1
Original line number Diff line number Diff line
@@ -181,7 +181,7 @@ static int io_register_enable_rings(struct io_ring_ctx *ctx) 
		return -EBADFD;



	if (ctx->flags & IORING_SETUP_SINGLE_ISSUER && !ctx->submitter_task) {

		WRITE_ONCE(ctx->submitter_task, get_task_struct(current));

		ctx->submitter_task = get_task_struct(current);

		/*

		 * Lazy activation attempts would fail if it was polled before

		 * submitter_task is set.