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

[Keith Busch <kbusch@kernel.org>]

On Tue, Feb 18, 2025 at 08:22:36PM -0800, Caleb Sander Mateos wrote:
> > +struct io_alloc_cache {
> > +       void                    **entries;
> > +       unsigned int            nr_cached;
> > +       unsigned int            max_cached;
> > +       size_t                  elem_size;
> 
> Is growing this field from unsigned to size_t really necessary? It
> probably doesn't make sense to be caching allocations > 4 GB.

It used to be a size_t when I initially moved the struct to here, but
it's not anymore, so I'm out of sync. I'll fix it up.
 
> > @@ -859,10 +924,8 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
> >                         }
> >                         node->tag = tag;
> >                 }
> > -               data.nodes[i] = node;
> > +               table.data.nodes[i] = node;
> >         }
> > -
> > -       ctx->buf_table.data = data;
> 
> Still don't see the need to move this assignment. Is there a reason
> you prefer setting ctx->buf_table before initializing its nodes? I
> find the existing code easier to follow, where the table is moved to
> ctx->buf_table after filling it in. It's also consistent with
> io_clone_buffers().

Yes, it needs to move to earlier. The ctx buf_table needs to be set
before any allocations from io_rsrc_node_alloc() can happen.