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

[Jens Axboe <axboe@kernel.dk>]

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)?

-- 
Jens Axboe