Re: [PATCH net-next 1/3] net: netmem: add net_iov_area freelist helpers

[Mina Almasry]

On Tue, Sep 22, 2026 at 1:43 PM Stanislav Fomichev <sdf.kernel@xxxxxxxxx> wrote:
>
> io_uring zero-copy receive and devmem both maintain a bounded LIFO for
> net_iovs in a contiguous area. Store the freelist in struct net_iov_area
> and provide common push and pop helpers.
>
> Leave synchronization to area owners. Keep devmem's area adjacent to its

This could be a follow up change, but I think synchronization should
be provided by the netmem/niov infra, rather than the area owners. TBH
the infra providing an unsynchronized data structure and letting the
area owner use it and shoot themselves in the foot feels error prone.
For now we could use a comment.

I also think we should not provide _push and_pop functions, rather we
should provide _push_bulk() and _pop_bulk(), and the caller can decide
to only push and pop 1 at a time if they need to. The reason is that
in allocation paths, we almost always want to alloc in bulk. If the
infra had the lock it could lock once and alloc a bulk.

The free path is more nuanced. I think _push_bulk() is actually not
currently that useful, because page_pool_put_netmem_bulk() ends up
internally looping over individual calls to __page_pool_put_page(). It
seems like there is a very low hanging fruit optimization possible
here where we change things such that page_pool_put_netmem_bulk()
actually does free the entire bulk at once, and if the pp is using a
memory provider, it uses push _push_bulk() to free the entire stack of
netmems with 1 lock acquire.

But these can be future optimizations, so,

Reviewed-by: Mina Almasry <almasrymina@xxxxxxxxxx>

> lock. Use u32 indices and counts, which halves devmem's freelist storage
> on 64-bit systems. Reject devmem areas with more than U32_MAX entries
> before narrowing the count. With 4 KiB chunks, the limit is almost 16 TiB.
>
> Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxxx>
> ---
>  include/net/netmem.h | 28 ++++++++++++++++++++++++++-
>  io_uring/zcrx.c      | 25 +++++++++---------------
>  io_uring/zcrx.h      |  4 ----
>  net/core/devmem.c    | 46 ++++++++++++++++++++++----------------------
>  net/core/devmem.h    |  7 +++----
>  5 files changed, 62 insertions(+), 48 deletions(-)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index bccacd21b6c3..da885d95ea63 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -101,10 +101,15 @@ struct net_iov {
>  struct net_iov_area {
>         /* Array of net_iovs for this area. */
>         struct net_iov *niovs;
> -       size_t num_niovs;
> +
> +       /* Stack of free net_iov indices. */
> +       u32 *freelist;
>
>         /* Offset into the dma-buf where this chunk starts.  */
>         unsigned long base_virtual;
> +
> +       u32 num_niovs;
> +       u32 free_count;

Why not keep num_niovs a size_t and make free_count a size_t as well
(so that essentially we can support anything up to SIZE_T_MAX - if
that exists - num entries)? Do we gain anything by converting these to
u32? I know in practice in really doens't make a difference, but since
struct dma_buf->size is a size_t and that's usually the type we use
for memory sizes we should use it unless we see a reason not to. Tbh I
don't think the size of the freelist array is a big deal?


-- 
Thanks,
Mina