Re: [PATCHv2 3/6] io_uring: add support for kernel registered bvecs

Pavel Begunkov
2025-02-13T01:32:24Z

On 2/11/25 00:56, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Provide an interface for the kernel to leverage the existing
> pre-registered buffers that io_uring provides. User space can reference
> these later to achieve zero-copy IO.
>
> User space must register an empty fixed buffer table with io_uring in
> order for the kernel to make use of it.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> include/linux/io_uring.h | 1 +
> include/linux/io_uring_types.h | 4 ++
> io_uring/rsrc.c | 100 +++++++++++++++++++++++++++++++--
> io_uring/rsrc.h | 1 +
> 4 files changed, 100 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
> index 85fe4e6b275c7..b5637a2aae340 100644
> --- a/include/linux/io_uring.h
> +++ b/include/linux/io_uring.h
> @@ -5,6 +5,7 @@
> #include <linux/sched.h>
> #include <linux/xarray.h>
> #include <uapi/linux/io_uring.h>
> +#include <linux/blk-mq.h>
>
> #if defined(CONFIG_IO_URING)
> void __io_uring_cancel(bool cancel_all);
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index e2fef264ff8b8..99aac2d52fbae 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -693,4 +693,8 @@ static inline bool io_ctx_cqe32(struct io_ring_ctx *ctx)
> return ctx->flags & IORING_SETUP_CQE32;
> }
>
> +int io_buffer_register_bvec(struct io_ring_ctx *ctx, struct request *rq,
> + void (*release)(void *), unsigned int index);
> +void io_buffer_unregister_bvec(struct io_ring_ctx *ctx, unsigned int tag);
> +
> #endif
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 30f08cf13ef60..14efec8587888 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -110,8 +110,9 @@ static void io_buffer_unmap(struct io_ring_ctx *ctx, struct io_rsrc_node *node)
>
> if (!refcount_dec_and_test(&imu->refs))
> return;
> - for (i = 0; i < imu->nr_bvecs; i++)
> - unpin_user_page(imu->bvec[i].bv_page);
> + if (node->type == IORING_RSRC_BUFFER)
> + for (i = 0; i < imu->nr_bvecs; i++)
> + unpin_user_page(imu->bvec[i].bv_page);
> if (imu->acct_pages)
> io_unaccount_mem(ctx, imu->acct_pages);
> kvfree(imu);
> @@ -240,6 +241,13 @@ static int __io_sqe_buffers_update(struct io_ring_ctx *ctx,
> struct io_rsrc_node *node;
> u64 tag = 0;
>
> + i = array_index_nospec(up->offset + done, ctx->buf_table.nr);
> + node = io_rsrc_node_lookup(&ctx->buf_table, i);
> + if (node && node->type != IORING_RSRC_BUFFER) {

Please drop the special rule. It's handled, so if the user wants
to do it, it can be allowed in a generic way.

> + err = -EBUSY;
> + break;
> + }
> +
...
> +int io_buffer_register_bvec(struct io_ring_ctx *ctx, struct request *rq,
> + void (*release)(void *), unsigned int index)
> +{
> + struct io_rsrc_data *data = &ctx->buf_table;
> + struct req_iterator rq_iter;
> + struct io_mapped_ubuf *imu;
> + struct io_rsrc_node *node;
> + struct bio_vec bv;
> + u16 nr_bvecs;
> + int i = 0;
> +
> + lockdep_assert_held(&ctx->uring_lock);
> +
> + if (!data->nr)
> + return -EINVAL;
> + if (index >= data->nr)
> + return -EINVAL;

array_index_nospec

> +
> + node = data->nodes[index];
> + if (node)
> + return -EBUSY;
...> +void io_buffer_unregister_bvec(struct io_ring_ctx *ctx, unsigned int index)
> +{
> + struct io_rsrc_data *data = &ctx->buf_table;
> + struct io_rsrc_node *node;
> +
> + lockdep_assert_held(&ctx->uring_lock);
> +
> + if (!data->nr)
> + return;
> + if (index >= data->nr)
> + return;

array_index_nospec

> +
> + node = data->nodes[index];
> + if (!node || !node->buf)

How can node->buf be NULL?

> + return;
> + if (node->type != IORING_RSRC_KBUFFER)
> + return;
> + io_reset_rsrc_node(ctx, data, index);
> +}
> +EXPORT_SYMBOL_GPL(io_buffer_unregister_bvec);
...> diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
> index a3826ab84e666..8147dfc26f737 100644
> --- a/io_uring/rsrc.h
> +++ b/io_uring/rsrc.h
> @@ -13,6 +13,7 @@
> enum {
> IORING_RSRC_FILE = 0,
> IORING_RSRC_BUFFER = 1,
> + IORING_RSRC_KBUFFER = 2,

nit: you don't even need it, just check for presence of the
callback in io_buffer_unmap()

--
Pavel Begunkov