Re: [PATCH v3 21/25] io_uring/rsrc: split io_buffer_register_request() logic

[Joanne Koong <joannelkoong@gmail.com>]

On Thu, Jan 8, 2026 at 1:04 PM Caleb Sander Mateos
<csander@purestorage.com> wrote:
>
> On Mon, Dec 22, 2025 at 4:36 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > Split the main initialization logic in io_buffer_register_request() into
> > a helper function.
> >
> > This is a preparatory patch for supporting kernel-populated buffers in
> > fuse io-uring, which will be reusing this logic.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > ---
> >  io_uring/rsrc.c | 89 ++++++++++++++++++++++++++++++-------------------
> >  1 file changed, 54 insertions(+), 35 deletions(-)
> >
> > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> > index b25b418e5c11..5fe2695dafb6 100644
> > --- a/io_uring/rsrc.c
> > +++ b/io_uring/rsrc.c
> > @@ -936,67 +936,86 @@ int io_sqe_buffers_register(struct io_ring_ctx *ctx, void __user *arg,
> >         return ret;
> >  }
> >
> > -int io_buffer_register_request(struct io_uring_cmd *cmd, struct request *rq,
> > -                              void (*release)(void *), unsigned int index,
> > -                              unsigned int issue_flags)
> > +static struct io_mapped_ubuf *io_kernel_buffer_init(struct io_ring_ctx *ctx,
> > +                                                   unsigned int nr_bvecs,
> > +                                                   unsigned int total_bytes,
> > +                                                   u8 dir,
> > +                                                   void (*release)(void *),
> > +                                                   void *priv,
> > +                                                   unsigned int index)
> >  {
> > -       struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> >         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;
> > -       unsigned int nr_bvecs = 0;
> > -       int ret = 0;
> >
> > -       io_ring_submit_lock(ctx, issue_flags);
> > -       if (index >= data->nr) {
> > -               ret = -EINVAL;
> > -               goto unlock;
> > -       }
> > +       if (index >= data->nr)
> > +               return ERR_PTR(-EINVAL);
> >         index = array_index_nospec(index, data->nr);
> >
> > -       if (data->nodes[index]) {
> > -               ret = -EBUSY;
> > -               goto unlock;
> > -       }
> > +       if (data->nodes[index])
> > +               return ERR_PTR(-EBUSY);
> >
> >         node = io_rsrc_node_alloc(ctx, IORING_RSRC_BUFFER);
> > -       if (!node) {
> > -               ret = -ENOMEM;
> > -               goto unlock;
> > -       }
> > +       if (!node)
> > +               return ERR_PTR(-ENOMEM);
> >
> > -       /*
> > -        * blk_rq_nr_phys_segments() may overestimate the number of bvecs
> > -        * but avoids needing to iterate over the bvecs
> > -        */
> > -       imu = io_alloc_imu(ctx, blk_rq_nr_phys_segments(rq));
> > +       imu = io_alloc_imu(ctx, nr_bvecs);
> >         if (!imu) {
> >                 kfree(node);
> > -               ret = -ENOMEM;
> > -               goto unlock;
> > +               return ERR_PTR(-ENOMEM);
> >         }
> >
> >         imu->ubuf = 0;
> > -       imu->len = blk_rq_bytes(rq);
> > +       imu->len = total_bytes;
> >         imu->acct_pages = 0;
> >         imu->folio_shift = PAGE_SHIFT;
> > +       imu->nr_bvecs = nr_bvecs;
> >         refcount_set(&imu->refs, 1);
> >         imu->release = release;
> > -       imu->priv = rq;
> > +       imu->priv = priv;
> >         imu->is_kbuf = true;
> > -       imu->dir = 1 << rq_data_dir(rq);
> > +       imu->dir = 1 << dir;
> >
> > +       node->buf = imu;
> > +       data->nodes[index] = node;
> > +
> > +       return imu;
> > +}
> > +
> > +int io_buffer_register_request(struct io_uring_cmd *cmd, struct request *rq,
> > +                              void (*release)(void *), unsigned int index,
> > +                              unsigned int issue_flags)
> > +{
> > +       struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> > +       struct req_iterator rq_iter;
> > +       struct io_mapped_ubuf *imu;
> > +       struct bio_vec bv;
> > +       unsigned int nr_bvecs;
> > +       unsigned int total_bytes;
> > +
> > +       /*
> > +        * blk_rq_nr_phys_segments() may overestimate the number of bvecs
> > +        * but avoids needing to iterate over the bvecs
> > +        */
> > +       nr_bvecs = blk_rq_nr_phys_segments(rq);
> > +       total_bytes = blk_rq_bytes(rq);
>
> Could combine these initializations with the variable declarations

I don't have a preference but was thinking it would look unaesthetic
with having that comment block in the declaration section. I don't
think it matters though, so I'm happy to combine the two sections for
v4.

>
> > +
> > +       io_ring_submit_lock(ctx, issue_flags);
> > +
> > +       imu = io_kernel_buffer_init(ctx, nr_bvecs, total_bytes, rq_data_dir(rq),
> > +                                   release, rq, index);
> > +       if (IS_ERR(imu)) {
> > +               io_ring_submit_unlock(ctx, issue_flags);
>
> I would prefer to leave the existing goto unlock; pattern. The goto
> pattern is easier to extend in the future with additional resource
> acquisitions. And keeping it would make the diff slightly smaller. For
> new functions with a single early return path, I don't feel all that
> strongly, but it seems like unnecessary refactoring of this existing
> code.

Sounds good, I'll leave in the existing goto unlock; pattern.

Thanks for reviewing this patchset.

>
> Other than that,
> Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
>
> > +               return PTR_ERR(imu);
> > +       }
> > +
> > +       nr_bvecs = 0;
> >         rq_for_each_bvec(bv, rq, rq_iter)
> >                 imu->bvec[nr_bvecs++] = bv;
> >         imu->nr_bvecs = nr_bvecs;
> >
> > -       node->buf = imu;
> > -       data->nodes[index] = node;
> > -unlock:
> >         io_ring_submit_unlock(ctx, issue_flags);
> > -       return ret;
> > +       return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(io_buffer_register_request);
> >
> > --
> > 2.47.3
> >