Re: [PATCH v1 14/15] io_uring/zcrx: set pp memory provider for an rx queue

Jens Axboe
2024-10-09T18:42:23Z

On 10/7/24 4:16 PM, David Wei wrote:
> From: David Wei <davidhwei@meta.com>
>
> Set the page pool memory provider for the rx queue configured for zero copy to
> io_uring. Then the rx queue is reset using netdev_rx_queue_restart() and netdev
> core + page pool will take care of filling the rx queue from the io_uring zero
> copy memory provider.
>
> For now, there is only one ifq so its destruction happens implicitly during
> io_uring cleanup.

Bit wide...

> @@ -237,15 +309,20 @@ int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
> reg.offsets.tail = offsetof(struct io_uring, tail);
>
> if (copy_to_user(arg, &reg, sizeof(reg))) {
> + io_close_zc_rxq(ifq);
> ret = -EFAULT;
> goto err;
> }
> if (copy_to_user(u64_to_user_ptr(reg.area_ptr), &area, sizeof(area))) {
> + io_close_zc_rxq(ifq);
> ret = -EFAULT;
> goto err;
> }
> ctx->ifq = ifq;
> return 0;

Not added in this patch, but since I was looking at rtnl lock coverage,
it's OK to potentially fault while holding this lock? I'm assuming it
is, as I can't imagine any faulting needing to grab it. Not even from
nbd ;-)

Looks fine to me.

--
Jens Axboe