Skip to contentSkip to conversation

follow up zcrx fixes

PATCH Superseded Pavel Begunkov 1 participant 6 messages Mar 25

Subsystems (4)
Networking Network drivers io_uring io_uring zcrx

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Pavel Begunkov

author maintainer
#0 lore raw

cover follow up zcrx fixes

Follow up fixes for the recent update flagged by review.

Pavel Begunkov (4):
io_uring/zcrx: don't use mark0 for allocating xarray io_uring/zcrx: don't clear not allocated niovs io_uring/zcrx: use dma_len for chunk size calculation io_uring/zcrx: use correct mmap off constants

 io_uring/zcrx.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH 1/4 io_uring/zcrx: don't use mark0 for allocating xarray

XA_MARK_0 is not compatible with xarray allocating entries, use XA_MARK_1.

Fixes fda90d43f4fac ("io_uring/zcrx: return back two step unregistration")
Signed-off-by Pavel Begunkov <[email protected]>
---
 io_uring/zcrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
io_uring/zcrx.c +2 -2
index f94f74d0f566..695398d2f2e0 100644
@@ -927,12 +927,12 @@ int io_register_zcrx(struct io_ring_ctx *ctx,
 
 static inline bool is_zcrx_entry_marked(struct io_ring_ctx *ctx, unsigned long id)
 {
-	return xa_get_mark(&ctx->zcrx_ctxs, id, XA_MARK_0);
+	return xa_get_mark(&ctx->zcrx_ctxs, id, XA_MARK_1);
 }
 
 static inline void set_zcrx_entry_mark(struct io_ring_ctx *ctx, unsigned long id)
 {
-	xa_set_mark(&ctx->zcrx_ctxs, id, XA_MARK_0);
+	xa_set_mark(&ctx->zcrx_ctxs, id, XA_MARK_1);
 }
 
 void io_terminate_zcrx(struct io_ring_ctx *ctx)
Signature · 1 line
2.53.0
author maintainer
#2 lore raw

PATCH 2/4 io_uring/zcrx: don't clear not allocated niovs

In reply to Pavel Begunkov

Now that area->is_mapped is set earlier before niovs array is allocated, io_zcrx_free_area -> io_zcrx_unmap_area in an error path can try to clear dma addresses for unallocated niovs, fix it.

Fixes 8c0cab0b7bf76 ("always dma map in advance")
Signed-off-by Pavel Begunkov <[email protected]>
---
 io_uring/zcrx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
io_uring/zcrx.c +4 -2
index 695398d2f2e0..fc6199edad34 100644
@@ -289,8 +289,10 @@ static void io_zcrx_unmap_area(struct io_zcrx_ifq *ifq,
 		return;
 	area->is_mapped = false;
 
-	for (i = 0; i < area->nia.num_niovs; i++)
-		net_mp_niov_set_dma_addr(&area->nia.niovs[i], 0);
+	if (area->nia.niovs) {
+		for (i = 0; i < area->nia.num_niovs; i++)
+			net_mp_niov_set_dma_addr(&area->nia.niovs[i], 0);
+	}
 
 	if (area->mem.is_dmabuf) {
 		io_release_dmabuf(&area->mem);
Signature · 1 line
2.53.0
author maintainer
#3 lore raw

PATCH 3/4 io_uring/zcrx: use dma_len for chunk size calculation

In reply to Pavel Begunkov

Buffers are now dma-mapped earlier and we can sg_dma_len(), otherwise, since it's walking with for_each_sgtable_dma_sg(), it might wrongfully reject some configurations. As a bonus, it'd now be able to use larger chunks if dma addresses are coalesced e.g by iommu.

Fixes 8c0cab0b7bf76 ("always dma map in advance")
Signed-off-by Pavel Begunkov <[email protected]>
---
 io_uring/zcrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
io_uring/zcrx.c +1 -1
index fc6199edad34..0f98a3c74e2a 100644
@@ -63,7 +63,7 @@ static int io_area_max_shift(struct io_zcrx_mem *mem)
 	unsigned i;
 
 	for_each_sgtable_dma_sg(sgt, sg, i)
-		shift = min(shift, __ffs(sg->length));
+		shift = min(shift, __ffs(sg_dma_len(sg)));
 	return shift;
 }
Signature · 1 line
2.53.0
author maintainer
#4 lore raw

PATCH 4/4 io_uring/zcrx: use correct mmap off constants

In reply to Pavel Begunkov

zcrx was using IORING_OFF_PBUF_SHIFT during first iterations, but there is now a separate constant it should use. Both are 16 so it doesn't change anything, but improve it for the future.

Signed-off-by Pavel Begunkov <[email protected]>
---
 io_uring/zcrx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
io_uring/zcrx.c +1 -1
index 0f98a3c74e2a..f38d30dc0ac7 100644
@@ -386,7 +386,7 @@ static int io_allocate_rbuf_ring(struct io_ring_ctx *ctx,
 		return -EINVAL;
 
 	mmap_offset = IORING_MAP_OFF_ZCRX_REGION;
-	mmap_offset += id << IORING_OFF_PBUF_SHIFT;
+	mmap_offset += id << IORING_OFF_ZCRX_SHIFT;
 
 	ret = io_create_region(ctx, &ifq->rq_region, rd, mmap_offset);
 	if (ret < 0)
Signature · 1 line
2.53.0
author maintainer
#5 lore raw
In reply to Pavel Begunkov
Quoted 1 line · Pavel Begunkov

Follow up fixes for the recent update flagged by review.

Actually, I'm not happy how it interacts with the nodev mode, I'll send a v2

Signature · 1 line
Pavel Begunkov