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

[David Wei <dw@davidwei.uk>]

On 2025-10-25 16:41, Jens Axboe wrote:
> On 10/25/25 1:15 PM, David Wei wrote:
>> @@ -541,6 +541,74 @@ struct io_mapped_region *io_zcrx_get_region(struct io_ring_ctx *ctx,
>>   	return ifq ? &ifq->region : NULL;
>>   }
>>   
>> +static int io_proxy_zcrx_ifq(struct io_ring_ctx *ctx,
>> +			     struct io_uring_zcrx_ifq_reg __user *arg,
>> +			     struct io_uring_zcrx_ifq_reg *reg)
>> +{
>> +	struct io_zcrx_ifq *ifq, *src_ifq;
>> +	struct io_ring_ctx *src_ctx;
>> +	struct file *file;
>> +	int src_fd, ret;
>> +	u32 src_id, id;
>> +
>> +	src_fd = reg->if_idx;
>> +	src_id = reg->if_rxq;
>> +
>> +	file = io_uring_register_get_file(src_fd, false);
>> +	if (IS_ERR(file))
>> +		return PTR_ERR(file);
>> +
>> +	src_ctx = file->private_data;
>> +	if (src_ctx == ctx)
>> +		return -EBADFD;
>> +
>> +	mutex_unlock(&ctx->uring_lock);
>> +	io_lock_two_rings(ctx, src_ctx);
>> +
>> +	ret = -EINVAL;
>> +	src_ifq = xa_load(&src_ctx->zcrx_ctxs, src_id);
>> +	if (!src_ifq || src_ifq->proxy)
>> +		goto err_unlock;
>> +
>> +	percpu_ref_get(&src_ctx->refs);
>> +	refcount_inc(&src_ifq->refs);
>> +
>> +	ifq = kzalloc(sizeof(*ifq), GFP_KERNEL);
> 
> This still needs a:
> 
> 	if (!ifq)
> 		handle error
> 
> addition, like mentioned for v1. Would probably make sense to just
> assume that everything is honky dory and allocate it upfront/early, and
> just kill it in the error path. Would probably help remove one of the
> goto labels.

Sorry I missed this during the splitting. Will include in v3.

> 
>> +	ifq->proxy = src_ifq;
> 
> For this, since the ifq is shared and reference counted, why don't they
> just point at the same memory here? Would avoid having this ->proxy
> thing and just skipping to that in other spots where the actual
> io_zcrx_ifq is required?
> 

I wanted a way to separate src and dst rings, while also decrementing
refcounts once and only once. I used separate ifq objects to do this,
but having learnt about xarray marks, I think I can use that instead.