Re: [RFC PATCH 3/7] io_uring: support to register bvec buffer to specified io_uring

[Pavel Begunkov <asml.silence@gmail.com>]

On 4/28/25 10:44, Ming Lei wrote:
> Extend io_buffer_register_bvec() and io_buffer_unregister_bvec() for
> supporting to register/unregister bvec buffer to specified io_uring,
> which FD is usually passed from userspace.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   include/linux/io_uring/cmd.h |  4 ++
>   io_uring/rsrc.c              | 83 +++++++++++++++++++++++++++---------
>   2 files changed, 67 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index 78fa336a284b..7516fe5cd606 100644
> --- a/include/linux/io_uring/cmd.h
> +++ b/include/linux/io_uring/cmd.h
> @@ -25,6 +25,10 @@ struct io_uring_cmd_data {
...
>   	io_ring_submit_lock(ctx, issue_flags);
> -	ret = __io_buffer_unregister_bvec(ctx, buf);
> +	if (reg)
> +		ret = __io_buffer_register_bvec(ctx, buf);
> +	else
> +		ret = __io_buffer_unregister_bvec(ctx, buf);
>   	io_ring_submit_unlock(ctx, issue_flags);
>   
>   	return ret;
>   }
> +
> +static int io_buffer_reg_unreg_bvec(struct io_ring_ctx *ctx,
> +				    struct io_buf_data *buf,
> +				    unsigned int issue_flags,
> +				    bool reg)
> +{
> +	struct io_ring_ctx *remote_ctx = ctx;
> +	struct file *file = NULL;
> +	int ret;
> +
> +	if (buf->has_fd) {
> +		file = io_uring_register_get_file(buf->ring_fd, buf->registered_fd);

io_uring_register_get_file() accesses task private data and the request
doesn't control from which task it's executed. IOW, you can't use the
helper here. It can be iowq or sqpoll, but either way nothing is
promised.

> +		if (IS_ERR(file))
> +			return PTR_ERR(file);
> +		remote_ctx = file->private_data;
> +		if (!remote_ctx)
> +			return -EINVAL;

nit: this check is not needed.

> +	}
> +
> +	if (remote_ctx == ctx) {
> +		do_reg_unreg_bvec(ctx, buf, issue_flags, reg);
> +	} else {
> +		if (!(issue_flags & IO_URING_F_UNLOCKED))
> +			mutex_unlock(&ctx->uring_lock);

We shouldn't be dropping the lock in random helpers, for example
it'd be pretty nasty suspending a submission loop with a submission
from another task.

You can try lock first, if fails it'll need a fresh context via
iowq to be task-work'ed into the ring. see msg_ring.c for how
it's done for files.

> +
> +		do_reg_unreg_bvec(remote_ctx, buf, IO_URING_F_UNLOCKED, reg);
> +
> +		if (!(issue_flags & IO_URING_F_UNLOCKED))
> +			mutex_lock(&ctx->uring_lock);
> +	}
> +
> +	if (file)
> +		fput(file);
> +
> +	return ret;
> +}
-- 
Pavel Begunkov