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

[Stanislav Fomichev]

On 09/24, Mina Almasry wrote:
> 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>

Ack, let's discuss separately, this is probably a larger refactor.

> > 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?

My reasoning: 4K * UINT_MAX is ~18TB which should be enough for a
foreseeable future. So just saving a few bytes, free_count was already
u32 on iour side.