Re: [PATCH v1 03/11] io_uring/kbuf: add support for kernel-managed buffer rings

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

On 2/10/26 00:28, Joanne Koong wrote:
> Add support for kernel-managed buffer rings (kmbuf rings), which allow
> the kernel to allocate and manage the backing buffers for a buffer
> ring, rather than requiring the application to provide and manage them.
> 
> This introduces two new registration opcodes:
> - IORING_REGISTER_KMBUF_RING: Register a kernel-managed buffer ring
> - IORING_UNREGISTER_KMBUF_RING: Unregister a kernel-managed buffer ring
> 
> The existing io_uring_buf_reg structure is extended with a union to
> support both application-provided buffer rings (pbuf) and kernel-managed
> buffer rings (kmbuf):
> - For pbuf rings: ring_addr specifies the user-provided ring address
> - For kmbuf rings: buf_size specifies the size of each buffer. buf_size
>    must be non-zero and page-aligned.
> 
> The implementation follows the same pattern as pbuf ring registration,
> reusing the validation and buffer list allocation helpers introduced in
> earlier refactoring. The IOBL_KERNEL_MANAGED flag marks buffer lists as
> kernel-managed for appropriate handling in the I/O path.
> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
>   include/uapi/linux/io_uring.h |  15 ++++-
>   io_uring/kbuf.c               |  81 ++++++++++++++++++++++++-
>   io_uring/kbuf.h               |   7 ++-
>   io_uring/memmap.c             | 111 ++++++++++++++++++++++++++++++++++
>   io_uring/memmap.h             |   4 ++
>   io_uring/register.c           |   7 +++
>   6 files changed, 219 insertions(+), 6 deletions(-)
> 
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index fc473af6feb4..a0889c1744bd 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -715,6 +715,10 @@ enum io_uring_register_op {
>   	/* register bpf filtering programs */
>   	IORING_REGISTER_BPF_FILTER		= 37,
>   
> +	/* register/unregister kernel-managed ring buffer group */
> +	IORING_REGISTER_KMBUF_RING		= 38,
> +	IORING_UNREGISTER_KMBUF_RING		= 39,
> +
>   	/* this goes last */
>   	IORING_REGISTER_LAST,
>   
> @@ -891,9 +895,16 @@ enum io_uring_register_pbuf_ring_flags {
>   	IOU_PBUF_RING_INC	= 2,
>   };
>   
> -/* argument for IORING_(UN)REGISTER_PBUF_RING */
> +/* argument for IORING_(UN)REGISTER_PBUF_RING and
> + * IORING_(UN)REGISTER_KMBUF_RING
> + */
>   struct io_uring_buf_reg {
> -	__u64	ring_addr;
> +	union {
> +		/* used for pbuf rings */
> +		__u64	ring_addr;
> +		/* used for kmbuf rings */
> +		__u32   buf_size;

If you're creating a region, there should be no reason why it
can't work with user passed memory. You're fencing yourself off
optimisations that are already there like huge pages.

> +	};
>   	__u32	ring_entries;
>   	__u16	bgid;
>   	__u16	flags;
> diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
> index aa9b70b72db4..9bc36451d083 100644
> --- a/io_uring/kbuf.c
> +++ b/io_uring/kbuf.c
...
> +static int io_setup_kmbuf_ring(struct io_ring_ctx *ctx,
> +			       struct io_buffer_list *bl,
> +			       struct io_uring_buf_reg *reg)
> +{
> +	struct io_uring_buf_ring *ring;
> +	unsigned long ring_size;
> +	void *buf_region;
> +	unsigned int i;
> +	int ret;
> +
> +	/* allocate pages for the ring structure */
> +	ring_size = flex_array_size(ring, bufs, bl->nr_entries);
> +	ring = kzalloc(ring_size, GFP_KERNEL_ACCOUNT);
> +	if (!ring)
> +		return -ENOMEM;
> +
> +	ret = io_create_region_multi_buf(ctx, &bl->region, bl->nr_entries,
> +					 reg->buf_size);

Please use io_create_region(), the new function does nothing new
and only violates abstractions.

Provided buffer rings with kernel addresses could be an interesting
abstraction, but why is it also responsible for allocating buffers?
What I'd do:

1. Strip buffer allocation from IORING_REGISTER_KMBUF_RING.
2. Replace *_REGISTER_KMBUF_RING with *_REGISTER_PBUF_RING + a new flag.
    Or maybe don't expose it to the user at all and create it from
    fuse via internal API.
3. Require the user to register a memory region of appropriate size,
    see IORING_REGISTER_MEM_REGION, ctx->param_region. Make fuse
    populating the buffer ring using the memory region.

I wanted to make regions shareable anyway (need it for other purposes),
I can toss patches for that tomorrow.

A separate question is whether extending buffer rings is the right
approach as it seems like you're only using it for fuse requests and
not for passing buffers to normal requests, but I don't see the
big picture here.

> +	if (ret) {
> +		kfree(ring);
> +		return ret;
> +	}
> +
> +	/* initialize ring buf entries to point to the buffers */
> +	buf_region = bl->region.ptr;

io_region_get_ptr()

> +	for (i = 0; i < bl->nr_entries; i++) {
> +		struct io_uring_buf *buf = &ring->bufs[i];
> +
> +		buf->addr = (u64)(uintptr_t)buf_region;
> +		buf->len = reg->buf_size;
> +		buf->bid = i;
> +
> +		buf_region += reg->buf_size;
> +	}
> +	ring->tail = bl->nr_entries;
> +
> +	bl->buf_ring = ring;
> +	bl->flags |= IOBL_KERNEL_MANAGED;
> +
> +	return 0;
> +}
> +
> +int io_register_kmbuf_ring(struct io_ring_ctx *ctx, void __user *arg)
> +{
> +	struct io_uring_buf_reg reg;
> +	struct io_buffer_list *bl;
> +	int ret;
> +
> +	lockdep_assert_held(&ctx->uring_lock);
> +
> +	ret = io_copy_and_validate_buf_reg(arg, &reg, 0);
> +	if (ret)
> +		return ret;
> +
> +	if (!reg.buf_size || !PAGE_ALIGNED(reg.buf_size))

With io_create_region_multi_buf() gone, you shouldn't need
to align every buffer, that could be a lot of wasted memory
(thinking about 64KB pages).

> +		return -EINVAL;
> +
> +	bl = io_alloc_new_buffer_list(ctx, &reg);
> +	if (IS_ERR(bl))
> +		return PTR_ERR(bl);
> +
> +	ret = io_setup_kmbuf_ring(ctx, bl, &reg);
> +	if (ret) {
> +		kfree(bl);
> +		return ret;
> +	}
> +
> +	ret = io_buffer_add_list(ctx, bl, reg.bgid);
> +	if (ret)
> +		io_put_bl(ctx, bl);
> +
> +	return ret;

-- 
Pavel Begunkov