On Mon, Apr 28, 2025 at 2:44 AM Ming Lei <ming.lei@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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 {
>
> struct io_buf_data {
> unsigned short index;
> + bool has_fd;
> + bool registered_fd;
> +
> + int ring_fd;
> struct request *rq;
> void (*release)(void *);
> };
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 5f8ab130a573..701dd33fecf7 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -969,21 +969,6 @@ static int __io_buffer_register_bvec(struct io_ring_ctx *ctx,
> return 0;
> }
>
> -int io_buffer_register_bvec(struct io_uring_cmd *cmd,
> - struct io_buf_data *buf,
> - unsigned int issue_flags)
> -{
> - struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> - int ret;
> -
> - io_ring_submit_lock(ctx, issue_flags);
> - ret = __io_buffer_register_bvec(ctx, buf);
> - io_ring_submit_unlock(ctx, issue_flags);
> -
> - return ret;
> -}
> -EXPORT_SYMBOL_GPL(io_buffer_register_bvec);
> -
> static int __io_buffer_unregister_bvec(struct io_ring_ctx *ctx,
> struct io_buf_data *buf)
> {
> @@ -1006,19 +991,77 @@ static int __io_buffer_unregister_bvec(struct io_ring_ctx *ctx,
> return 0;
> }
>
> -int io_buffer_unregister_bvec(struct io_uring_cmd *cmd,
> - struct io_buf_data *buf,
> - unsigned int issue_flags)
> +static inline int do_reg_unreg_bvec(struct io_ring_ctx *ctx,
> + struct io_buf_data *buf,
> + unsigned int issue_flags,
> + bool reg)
> {
> - struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> int ret;
>
> 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);
It feels like unifying __io_buffer_register_bvec() and
__io_buffer_unregister_bvec() would belong better in the prior patch
that changes their signatures.
> 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);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
It would be good to avoid the overhead of this lookup and
reference-counting in the I/O path. Would it be possible to move this
lookup to when UBLK_IO_FETCH_REQ (and UBLK_IO_COMMIT_AND_FETCH_REQ, if
it specifies a different ring_fd) is submitted? I guess that might
require storing an extra io_ring_ctx pointer in struct ublk_io.
> + remote_ctx = file->private_data;
> + if (!remote_ctx)
> + return -EINVAL;
> + }
> +
> + 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);
> +
> + 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;
> +}
> +
> +int io_buffer_register_bvec(struct io_uring_cmd *cmd,
> + struct io_buf_data *buf,
> + unsigned int issue_flags)
If buf->has_fd is set, this struct io_uring_cmd *cmd is unused. Could
you define separate functions that take a struct io_uring_cmd * vs. a
ring_fd?
Best,
Caleb
> +{
> + struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> +
> + return io_buffer_reg_unreg_bvec(ctx, buf, issue_flags, true);
> +}
> +EXPORT_SYMBOL_GPL(io_buffer_register_bvec);
> +
> +int io_buffer_unregister_bvec(struct io_uring_cmd *cmd,
> + struct io_buf_data *buf,
> + unsigned int issue_flags)
> +{
> + struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> +
> + return io_buffer_reg_unreg_bvec(ctx, buf, issue_flags, false);
> +}
> EXPORT_SYMBOL_GPL(io_buffer_unregister_bvec);
>
> static int validate_fixed_range(u64 buf_addr, size_t len,
> --
> 2.47.0
>
