Re: [PATCH 5/5] io_uring/zcrx: add support for multiple ifqs

[David Wei <dw@davidwei.uk>]

On 2025-04-16 08:21, Pavel Begunkov wrote:
> Allow the user to register multiple ifqs / zcrx contexts. With that we
> can use multiple interfaces / interface queues in a single io_uring
> instance.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  include/linux/io_uring_types.h |  5 ++--
>  io_uring/io_uring.c            |  3 +-
>  io_uring/net.c                 |  8 ++---
>  io_uring/zcrx.c                | 53 +++++++++++++++++++++-------------
>  4 files changed, 40 insertions(+), 29 deletions(-)
> 
[...]
> diff --git a/io_uring/net.c b/io_uring/net.c
> index 5f1a519d1fc6..b3a643675ce8 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -1185,16 +1185,14 @@ int io_recvzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  	struct io_recvzc *zc = io_kiocb_to_cmd(req, struct io_recvzc);
>  	unsigned ifq_idx;
>  
> -	if (unlikely(sqe->file_index || sqe->addr2 || sqe->addr ||
> -		     sqe->addr3))
> +	if (unlikely(sqe->addr2 || sqe->addr || sqe->addr3))
>  		return -EINVAL;

Why remove sqe->file_index?

>  
>  	ifq_idx = READ_ONCE(sqe->zcrx_ifq_idx);
> -	if (ifq_idx != 0)
> -		return -EINVAL;
> -	zc->ifq = req->ctx->ifq;
> +	zc->ifq = xa_load(&req->ctx->zcrx_ctxs, ifq_idx);
>  	if (!zc->ifq)
>  		return -EINVAL;
> +
>  	zc->len = READ_ONCE(sqe->len);
>  	zc->flags = READ_ONCE(sqe->ioprio);
>  	zc->msg_flags = READ_ONCE(sqe->msg_flags);
> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
> index d56665fd103d..e4ce971b1257 100644
> --- a/io_uring/zcrx.c
> +++ b/io_uring/zcrx.c
> @@ -172,9 +172,6 @@ static int io_allocate_rbuf_ring(struct io_zcrx_ifq *ifq,
>  
>  static void io_free_rbuf_ring(struct io_zcrx_ifq *ifq)
>  {
> -	if (WARN_ON_ONCE(ifq->ctx->ifq))
> -		return;
> -

I think this should stay.

>  	io_free_region(ifq->ctx, &ifq->region);
>  	ifq->rq_ring = NULL;
>  	ifq->rqes = NULL;
[...]
> @@ -440,16 +443,23 @@ int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
>  
>  void io_unregister_zcrx_ifqs(struct io_ring_ctx *ctx)
>  {
> -	struct io_zcrx_ifq *ifq = ctx->ifq;
> +	struct io_zcrx_ifq *ifq;
> +	unsigned long id;
>  
>  	lockdep_assert_held(&ctx->uring_lock);
>  
> -	if (!ifq)
> -		return;
> +	while (1) {
> +		scoped_guard(mutex, &ctx->mmap_lock) {
> +			ifq = xa_find(&ctx->zcrx_ctxs, &id, ULONG_MAX, XA_PRESENT);
> +			if (ifq)
> +				xa_erase(&ctx->zcrx_ctxs, id);
> +		}
> +		if (!ifq)
> +			break;
> +		io_zcrx_ifq_free(ifq);
> +	}

Why not xa_for_each()? Is it weirdness with scoped_guard macro?