Re: [PATCH v3 3/3] io_uring/zcrx: share an ifq between rings

[David Wei <dw@davidwei.uk>]

On 2025-10-27 04:47, Pavel Begunkov wrote:
> On 10/27/25 10:20, Pavel Begunkov wrote:
>> On 10/26/25 17:34, David Wei wrote:
>>> Add a way to share an ifq from a src ring that is real i.e. bound to a
>>> HW RX queue with other rings. This is done by passing a new flag
>>> IORING_ZCRX_IFQ_REG_SHARE in the registration struct
>>> io_uring_zcrx_ifq_reg, alongside the fd of the src ring and the ifq id
>>> to be shared.
>>>
>>> To prevent the src ring or ifq from being cleaned up or freed while
>>> there are still shared ifqs, take the appropriate refs on the src ring
>>> (ctx->refs) and src ifq (ifq->refs).
>>>
>>> Signed-off-by: David Wei <dw@davidwei.uk>
>>> ---
>>>   include/uapi/linux/io_uring.h |  4 ++
>>>   io_uring/zcrx.c               | 74 ++++++++++++++++++++++++++++++++++-
>>>   2 files changed, 76 insertions(+), 2 deletions(-)
>>>
[...]
>>> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
>>> index 569cc0338acb..7418c959390a 100644
>>> --- a/io_uring/zcrx.c
>>> +++ b/io_uring/zcrx.c
[...]
>>> @@ -734,6 +797,13 @@ void io_shutdown_zcrx_ifqs(struct io_ring_ctx *ctx)
>>>           if (xa_get_mark(&ctx->zcrx_ctxs, index, XA_MARK_0))
>>>               continue;
>>> +        /*
>>> +         * Only shared ifqs want to put ctx->refs on the owning ifq
>>> +         * ring. This matches the get in io_share_zcrx_ifq().
>>> +         */
>>> +        if (ctx != ifq->ctx)
>>> +            percpu_ref_put(&ifq->ctx->refs);
>>
>> After you put this and ifq->refs below down, the zcrx object can get
>> destroyed, but this ctx might still have requests using the object.
>> Waiting on ctx refs would ensure requests are killed, but that'd
>> create a cycle.
> 
> Another concerning part is long term cross ctx referencing,
> which is even worse than pp locking it up. I mentioned
> that it'd be great to reverse the refcounting relation,
> but that'd also need additional ground work to break
> dependencies.

Yeah, Jens said the same. I did refactoring to break the dep, so now
rings take refs on ifqs that have an independent lifetime.
io_shutdown_zcrx_ifqs() is gone, and all cleanup is done after ctx->refs
drops to 0 in io_unregister_zcrx_ifqs(). From each ring's perspective,
the ifq remains alive until all of its requests are done, and the last
ring frees the ifq. I'll send it a bit later today.