Re: [PATCHv3 5/5] io_uring: cache nodes and mapped buffers

[Keith Busch <kbusch@kernel.org>]

On Sun, Feb 16, 2025 at 02:43:40PM -0800, Caleb Sander Mateos wrote:
> > > +       io_alloc_cache_free(&table->imu_cache, kfree);
> > > +}
> > > +
> > >  int io_sqe_buffers_unregister(struct io_ring_ctx *ctx)
> > >  {
> > >         if (!ctx->buf_table.data.nr)
> > >                 return -ENXIO;
> > > -       io_rsrc_data_free(ctx, &ctx->buf_table.data);
> > > +       io_rsrc_buffer_free(ctx, &ctx->buf_table);
> > >         return 0;
> > >  }
> > >
> > > @@ -716,6 +767,15 @@ bool io_check_coalesce_buffer(struct page **page_array, int nr_pages,
> > >         return true;
> > >  }
> > >
> > > +static struct io_mapped_ubuf *io_alloc_imu(struct io_ring_ctx *ctx,
> > > +                                          int nr_bvecs)
> > > +{
> > > +       if (nr_bvecs <= IO_CACHED_BVECS_SEGS)
> > > +               return io_cache_alloc(&ctx->buf_table.imu_cache, GFP_KERNEL);
> >
> > If there is no entry available in the cache, this will heap-allocate
> > one with enough space for all IO_CACHED_BVECS_SEGS bvecs. Consider
> > using io_alloc_cache_get() instead of io_cache_alloc(), so the
> > heap-allocated fallback uses the minimal size.
> >
> > Also, where are these allocations returned to the imu_cache? Looks
> > like kvfree(imu) in io_buffer_unmap() and io_sqe_buffer_register()
> > needs to try io_alloc_cache_put() first.
> 
> Another issue I see is that io_alloc_cache elements are allocated with
> kmalloc(), so they can't be freed with kvfree(). 

You actually can kvfree(kmalloc()); Here's the kernel doc for it:

  kvfree frees memory allocated by any of vmalloc(), kmalloc() or kvmalloc()

> When the imu is
> freed, we could check nr_bvecs <= IO_CACHED_BVECS_SEGS to tell whether
> to call io_alloc_cache_put() (with a fallback to kfree()) or kvfree().

But you're right, it shouldn't even hit this path because it's supposed
to try to insert the imu into the cache if that's where it was allocated
from.