Re: [PATCH v3 5/5] io_uring/rsrc: add io_uring_registered_mem_region_get()

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Mar 25, 2026 at 10:27 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 3/25/26 11:24 AM, Joanne Koong wrote:
> > On Wed, Mar 25, 2026 at 7:56?AM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 3/24/26 4:14 PM, Joanne Koong wrote:
> >>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> >>> index cf5638406a0c..c706324fd66d 100644
> >>> --- a/io_uring/rsrc.c
> >>> +++ b/io_uring/rsrc.c
> >>> @@ -1182,6 +1182,24 @@ int io_import_reg_buf(struct io_kiocb *req, struct iov_iter *iter,
> >>>       return io_import_fixed(ddir, iter, node->buf, buf_addr, len);
> >>>  }
> >>>
> >>> +void *io_uring_registered_mem_region_get(struct io_uring_cmd *cmd,
> >>> +                                      unsigned *nr_pages,
> >>> +                                      unsigned issue_flags)
> >>> +{
> >>> +     struct io_ring_ctx *ctx = cmd_to_io_kiocb(cmd)->ctx;
> >>> +     void *ptr;
> >>> +
> >>> +     io_ring_submit_lock(ctx, issue_flags);
> >>> +
> >>> +     ptr = ctx->param_region.ptr;
> >>> +     *nr_pages = ctx->param_region.nr_pages;
> >>> +
> >>> +     io_ring_submit_unlock(ctx, issue_flags);
> >>> +
> >>> +     return ptr;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(io_uring_registered_mem_region_get);
> >>
> >> This looks suspicious, but I actually think it looks suspicious because
> >> you add the submit locking around it. For patterns like that, it makes
> >> the brain go "hmm, what protects this from going invalid the instant
> >> io_ring_submit_unlock() is called??". But this should be stable for the
> >> duration of the ring, hence the locking should not be needed at all?
> >
> > My understanding is that once a memory region is registered to the
> > ring, it's registered for the ring's lifetime. There's no uapi to
> > unregister a memory region and my interpretation of the last paragraph
> > in this thread [1] is that unregistration is not intended to be
> > added/supported. I think the submit locking is needed in case another
> > thread is currently registering it so we don't see partially
> > initialized state between ptr and nr_pages (eg if the caller calls
> > this from a task work callback).
>
> Yes good point - can you please add a comment to that effect? Both why
> it's safe to return this state outside the lock, and also why the lock
> is actually required to ensure we see a sane state (either region fully
> registered, or not there)?

Good idea, I will add a comment about this to make this more clear,
something like:
/*
 * The submit lock ensures we don't see partially initialized state
 * if another thread is currently registering the region. Once registered,
 * the region is stable for the ring's lifetime (no unregister API exists),
 * so it's safe to access the returned pointer outside the lock.
 */

I will wait a day or two to send out v4 in case additional comments come in.

Thanks,
Joanne
>
> --
> Jens Axboe