Re: [PATCH 4/5] io_uring/zcrx: move zcrx region to struct io_zcrx_ifq

[Pavel Begunkov <asml.silence@gmail.com>]

On 4/18/25 17:05, David Wei wrote:
> On 2025-04-16 08:21, Pavel Begunkov wrote:
>> Refill queue region is a part of zcrx and should stay in struct
>> io_zcrx_ifq. We can't have multiple queues without it.
>>
>> Note: ctx->ifq assignments are now protected by mmap_lock as it's in
>> the mmap region look up path.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   include/linux/io_uring_types.h |  2 --
>>   io_uring/zcrx.c                | 20 ++++++++++++--------
>>   io_uring/zcrx.h                |  1 +
>>   3 files changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
>> index 3b467879bca8..06d722289fc5 100644
>> --- a/include/linux/io_uring_types.h
>> +++ b/include/linux/io_uring_types.h
>> @@ -448,8 +448,6 @@ struct io_ring_ctx {
>>   	struct io_mapped_region		ring_region;
>>   	/* used for optimised request parameter and wait argument passing  */
>>   	struct io_mapped_region		param_region;
>> -	/* just one zcrx per ring for now, will move to io_zcrx_ifq eventually */
>> -	struct io_mapped_region		zcrx_region;
>>   };
>>   
>>   /*
>> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
>> index 652daff0eb8d..d56665fd103d 100644
>> --- a/io_uring/zcrx.c
>> +++ b/io_uring/zcrx.c
>> @@ -160,12 +160,11 @@ static int io_allocate_rbuf_ring(struct io_zcrx_ifq *ifq,
>>   	if (size > rd->size)
>>   		return -EINVAL;
>>   
>> -	ret = io_create_region_mmap_safe(ifq->ctx, &ifq->ctx->zcrx_region, rd,
>> -					 IORING_MAP_OFF_ZCRX_REGION);
>> +	ret = io_create_region(ifq->ctx, &ifq->region, rd, IORING_MAP_OFF_ZCRX_REGION);
> 
> Why is this changed to io_create_region()? I don't see the caller of
> io_allocate_rbuf_ring() changing or taking mmap_lock.

We only care about mmap seeing a consistent region. The mmap holds
the ctx->mmap_lock, and *mmap_safe was making sure the region is
updated atomically from its perspective.

Now, instead of protecting the region itself, this patch is protecting
how ifq and subsequently the region is published:

-	ctx->ifq = ifq;
+	scoped_guard(mutex, &ctx->mmap_lock)
+		ctx->ifq = ifq;

And io_zcrx_get_region() is either sees NULL or ifq with a
correct region.

-- 
Pavel Begunkov