Re: [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations

[Mina Almasry]

On Mon, Aug 31, 2026 at 11:35 AM Stanislav Fomichev
<sdf.kernel@xxxxxxxxx> wrote:
>
> Replace devmem's gen_pool based fixed-size allocator with a binding-level
> freelist similar to the one used by io_uring zero-copy receive.
>
> This is motivated by allocation latency observed in the NAPI receive path:
>
>   [ 1036.228913]  ? gen_pool_create+0x90/0x90
>   [ 1036.228915]  net_devmem_alloc_dmabuf+0x1f/0x60
>   [ 1036.228918]  mp_dmabuf_devmem_alloc_netmems+0x17/0x80
>   [ 1036.228920]  mlx5e_post_rx_mpwqes+0xdbe/0xdd0
>   [ 1036.228926]  mlx5e_napi_poll+0x113/0x830
>   [ 1036.228928]  ? sched_clock+0x5/0x10
>   [ 1036.228931]  ? wake_up_process+0x778/0x14b0
>   [ 1036.228933]  net_rx_action+0x15d/0x570
>   [ 1036.228934]  ? update_rq_clock+0x31/0x240
>   [ 1036.228937]  ? __napi_schedule+0x55/0xa0
>   [ 1036.228938]  ? mlx5_eq_comp_int+0x137/0x230
>   [ 1036.228940]  ? atomic_notifier_call_chain+0x36/0x90
>   [ 1036.228943]  ? sched_clock+0x5/0x10
>   [ 1036.228944]  ? sched_clock_cpu+0xc/0x170
>   [ 1036.228947]  irq_exit_rcu+0x12b/0x370
>   [ 1036.228950]  common_interrupt+0x85/0x90
>
> udmabuf can create a very large number of SG entries. In the worst case,
> devmem ends up adding one gen_pool chunk for each net_iov allocation
> unit backed by those entries. The gen_pool allocation path then has to
> traverse a linked list that can become too long for this hot path.
>
> Patch 1 removes the gen_pool and replaces it with a simple freelist of
> net_iov pointers protected by the same spin_lock_bh() pattern used by
> io_uring zcrx. Patch 2 removes the now-unnecessary chunk owner wrapper by
> embedding the net_iov_area directly in the dma-buf binding.
>

Oh boy, this is going to be a bit tricky.

I ran into this exact horrible perf bug (sorry for it in the first
place), but my solution was different. My solution [1] was to coalesce
the SG entries that are contigious (and they usually are in practice),
and I got 'acceptable' perf after that. Kaifeng is actually working on
cleaning up my hacky patch up to send it upstream now.

Now I don't know which approach is better. Thinking about the pros and
cons of your approach:

+ your approach is much simpler, and removes gen_pool overheads for a
single queue case. It should be (much?) faster for that case.
- your approach adds a lock and allocations from multiple queues in
parallel will contend on this lock. There should be some value of # of
queues N where your approach starts to completely trash. gen_pool is
lockless so I wouldn't expect it to degrade significantly in the
multi-queue case.

The question for me is what the performance is for a real use case
(NCCL all-to-all for example) over a realistic number of shared queues
(it's 4-8 for me). I need that perf data to be honest before judging
this.

The io_uring zcrx comparision is not completely valid. io_uring zcrx
is built from the ground up to be one-buffer-is-bound-to-one-rx-queue,
and devmem tcp is built from the ground up to be
one-buffer-can-be-bound-to-N-rx-queues.

Are you able to get NCCL all-to-all tests for N=4/8 yourself?
Otherwise please wait for me to backport this to my release kernel and
test it. ETA sometime this week, I hope.

But please no merge without real perf data. This has potential to be
great, but is very risky :(


[1] patch:
commit 7bb6d32e21b6e ("net: devmem: coalesce sg chunks before feeding
into gen_pool")
Author: Mina Almasry <almasrymina@xxxxxxxxxx>
Date:   Mon Dec 8 01:44:39 2025 +0000

    net: devmem: coalesce sg chunks before feeding into gen_pool

    On dma_buf_map_attachment drivers typically return an sglist where each
    sg is of size 64KB. When mapping a very large dmabuf (like 4GB or so),
    this results in an sglist of 62500 entries, but usually they all map to
    a contiguous range.

    The current implementation inserts each sg as a separate chunk into the
    gen_pool. This results in much slower gen_pool_alloc and gen_pool_free
    performance due to the gen_pool actually looping through all the chunks
    and trying to free/alloc from each of them.

    There is no reason to insert each individual sg into a gen_pool chunk.
    Instead, detect that the next sg starts at the dma_addr of the current
    chunk and coalesce them.

    This results in a huge improvement in all-to-all devmem collectives, for
    example sweeping 1G and 2G message sizes:

    Before:

           0     0x0         AlltoAll  1073741824       8388608
float    none      -1   289212    3.71    3.60      0 1796277    0.60
  0.58    N/A
           0     0x0         AlltoAll  2147483648      16777216
float    none      -1 4242107    0.51    0.49      0 6301030    0.34
 0.33    N/A

    After:

           0     0x0         AlltoAll  1073741824       8388608
float    none      -1   284860    3.77    3.65      0   252456    4.25
   4.12    N/A
           0     0x0         AlltoAll  2147483648      16777216
float    none      -1   569225    3.77    3.65      0   553717    3.88
   3.76    N/A

    Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>

    TEST=Tested using the all-to-all devmem collective outlined above.

    Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
    Reviewed-by: Kevin Berry <kpberry@xxxxxxxxxx>

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 606e556d1935b..07ba32b177390 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -274,6 +274,18 @@ net_devmem_bind_dmabuf(struct net_device *dev,
                size_t len = sg_dma_len(sg);
                struct net_iov *niov;

+               /* Detect contiguous sg's and coalesce them. This improves the
+                * gen_pool allocs/frees with O(GB) dmabufs, because the
+                * gen_pool code attempts 1 allocation/free per chunk in a
+                * list_for_each_entry_rcu.
+                */
+               while (sg_idx < binding->sgt->orig_nents - 1 &&
+                      dma_addr + len == sg_dma_address(sg_next(sg))) {
+                       len += sg_dma_len(sg_next(sg));
+                       sg = sg_next(sg);
+                       sg_idx++;
+               }
+
                owner = kzalloc_node(sizeof(*owner), GFP_KERNEL,
                                     dev_to_node(&dev->dev));
                if (!owner) {
> Stanislav Fomichev (2):
>   net: devmem: replace gen_pool with freelist
>   net: devmem: embed net_iov_area in binding
>
>  net/Kconfig       |   1 -
>  net/core/devmem.c | 158 ++++++++++++++++++----------------------------
>  net/core/devmem.h |  38 ++++-------
>  3 files changed, 72 insertions(+), 125 deletions(-)
>
> --
> 2.53.0-Meta
>


-- 
Thanks,
Mina