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

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Feb 11, 2026 at 2:06 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Wed, Feb 11, 2026 at 4:01 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >
> > On 2/10/26 19:39, Joanne Koong wrote:
> > > On Tue, Feb 10, 2026 at 8:34 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > >
> > >>> 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.
> > >
> > > There's separate checks needed between io_create_region() and
> > > io_create_region_multi_buf() (eg IORING_MEM_REGION_TYPE_USER flag
> >
> > If io_create_region() is too strict, let's discuss that in
> > examples if there are any, but it's likely not a good idea changing
> > that. If it's too lax, filter arguments in the caller. IOW, don't
> > pass IORING_MEM_REGION_TYPE_USER if it's not used.
> >
> > > checking) and different allocation calls (eg
> > > io_region_allocate_pages() vs io_region_allocate_pages_multi_buf()).
> >
> > I saw that and saying that all memmap.c changes can get dropped.
> > You're using it as one big virtually contig kernel memory range then
> > chunked into buffers, and that's pretty much what you're getting with
> > normal io_create_region(). I get that you only need it to be
> > contiguous within a single buffer, but that's not what you're doing,
> > and it'll be only worse than default io_create_region() e.g.
> > effectively disabling any usefulness of io_mem_alloc_compound(),
> > and ultimately you don't need to care.
>
> When I originally implemented it, I had it use
> io_region_allocate_pages() but this fails because it's allocating way
> too much memory at once. For fuse's use case, each buffer is usually
> at least 1 MB if not more. Allocating the memory one buffer a time in
> io_region_allocate_pages_multi_buf() bypasses the allocation errors I
> was seeing. That's the main reason I don't think this can just use
> io_create_region().
>
> >
> > Regions shouldn't know anything about your buffers, how it's
> > subdivided after, etc.
> >

I still think the memory for the buffers should be tied to the ring
itself and allocated physically contiguously per buffer. Per-buffer
contiguity will enable the most efficient DMA path for servers to send
read/write data to local storage or the network. If the buffers for
the bufring have to be allocated as one single memory region, the
io_mem_alloc_compound() call will fail for this large allocation size.
Even if io_mem_alloc_compound() did succeed, this is a waste as the
buffer pool as an entity doesn't need to be physically contiguous,
just the individual buffers themselves. For fuse, the server
configures what buffer pool size it wants to use, depending on what
queue depth and max request size it needs. So for most use cases, at
least for high-performance servers, allocation will have to fall back
to alloc_pages_bulk_node(), which doesn't allocate contiguously. You
mentioned in an earlier comment that this "only violates abstractions"
- which abstractions does this break? The pre-existing behavior
already defaults to allocating pages non-contiguously if the mem
region can't be allocated fully contiguously.

Going through registered buffers doesn't help either. Fuse servers can
be unprivileged and it's not guaranteed that there are enough huge
pages reserved or that another process hasn't taken them or that the
server has privileges to pre-reserve pages for the allocation. Also
the 2 MB granularity is inflexible while 1 GB is too much.

I'm not really seeing a way where we can honor the physical contiguity
requirements for the buffers without going through kernel-managed
bufrings with the allocation done on a per-buffer basis. Or am I
missing something here?

Thanks,
Joanne