[v1] Split netmem from struct page

[RFC 02/19] netmem: introduce netmem alloc/put API to wrap page alloc/put API

Posted by Byungchul Park 7 months, 1 week ago

To eliminate the use of struct page in page pool, the page pool code
should use netmem descriptor and API instead.

As part of the work, introduce netmem alloc/put API allowing the code to
use them rather than struct page things.

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 include/net/netmem.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 45c209d6cc689..c87a604e980b9 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -138,6 +138,24 @@ static inline netmem_ref page_to_netmem(struct page *page)
 	return (__force netmem_ref)page;
 }
 
+static inline netmem_ref alloc_netmems_node(int nid, gfp_t gfp_mask,
+		unsigned int order)
+{
+	return page_to_netmem(alloc_pages_node(nid, gfp_mask, order));
+}
+
+static inline unsigned long alloc_netmems_bulk_node(gfp_t gfp, int nid,
+		unsigned long nr_netmems, netmem_ref *netmem_array)
+{
+	return alloc_pages_bulk_node(gfp, nid, nr_netmems,
+			(struct page **)netmem_array);
+}
+
+static inline void put_netmem(netmem_ref netmem)
+{
+	put_page(netmem_to_page(netmem));
+}
+
 /**
  * virt_to_netmem - convert virtual memory pointer to a netmem reference
  * @data: host memory pointer to convert
-- 
2.17.1

Re: [RFC 02/19] netmem: introduce netmem alloc/put API to wrap page alloc/put API

Posted by Mina Almasry 7 months, 1 week ago

On Fri, May 9, 2025 at 4:51 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To eliminate the use of struct page in page pool, the page pool code
> should use netmem descriptor and API instead.
>
> As part of the work, introduce netmem alloc/put API allowing the code to
> use them rather than struct page things.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  include/net/netmem.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 45c209d6cc689..c87a604e980b9 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -138,6 +138,24 @@ static inline netmem_ref page_to_netmem(struct page *page)
>         return (__force netmem_ref)page;
>  }
>
> +static inline netmem_ref alloc_netmems_node(int nid, gfp_t gfp_mask,
> +               unsigned int order)
> +{
> +       return page_to_netmem(alloc_pages_node(nid, gfp_mask, order));
> +}
> +
> +static inline unsigned long alloc_netmems_bulk_node(gfp_t gfp, int nid,
> +               unsigned long nr_netmems, netmem_ref *netmem_array)
> +{
> +       return alloc_pages_bulk_node(gfp, nid, nr_netmems,
> +                       (struct page **)netmem_array);
> +}
> +
> +static inline void put_netmem(netmem_ref netmem)
> +{
> +       put_page(netmem_to_page(netmem));
> +}

We can't really do this. netmem_ref is an abstraction that can be a
struct page or struct net_iov underneath. We can't be sure when
put_netmem is called that it is safe to convert to a page via
netmem_to_page(). This will crash if put_netmem is called on a
netmem_ref that is a net_iov underneath.

Please read the patch series that introduced netmem_ref to familiarize
yourself with the background here:

https://lkml.iu.edu/hypermail/linux/kernel/2401.2/09140.html

-- 
Thanks,
Mina

Re: [RFC 02/19] netmem: introduce netmem alloc/put API to wrap page alloc/put API

Posted by Mina Almasry 7 months, 1 week ago

j

On Fri, May 9, 2025 at 6:39 AM Mina Almasry <almasrymina@google.com> wrote:
>
> On Fri, May 9, 2025 at 4:51 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > To eliminate the use of struct page in page pool, the page pool code
> > should use netmem descriptor and API instead.
> >
> > As part of the work, introduce netmem alloc/put API allowing the code to
> > use them rather than struct page things.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> >  include/net/netmem.h | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index 45c209d6cc689..c87a604e980b9 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -138,6 +138,24 @@ static inline netmem_ref page_to_netmem(struct page *page)
> >         return (__force netmem_ref)page;
> >  }
> >
> > +static inline netmem_ref alloc_netmems_node(int nid, gfp_t gfp_mask,
> > +               unsigned int order)
> > +{
> > +       return page_to_netmem(alloc_pages_node(nid, gfp_mask, order));
> > +}
> > +
> > +static inline unsigned long alloc_netmems_bulk_node(gfp_t gfp, int nid,
> > +               unsigned long nr_netmems, netmem_ref *netmem_array)
> > +{
> > +       return alloc_pages_bulk_node(gfp, nid, nr_netmems,
> > +                       (struct page **)netmem_array);
> > +}
> > +
> > +static inline void put_netmem(netmem_ref netmem)
> > +{
> > +       put_page(netmem_to_page(netmem));
> > +}
>
> We can't really do this. netmem_ref is an abstraction that can be a
> struct page or struct net_iov underneath. We can't be sure when
> put_netmem is called that it is safe to convert to a page via
> netmem_to_page(). This will crash if put_netmem is called on a
> netmem_ref that is a net_iov underneath.
>

On a closer look, it looks like this put_netmem is only called from
code paths where you are sure the netmem_ref is a page underneath, so
this is likely fine for now. There is a net_next series that is adding
proper put_netmem support [1]. It's probably best to rebase your work
on top of that, but this should be fine in the meantime.

[1] https://lore.kernel.org/netdev/20250508004830.4100853-1-almasrymina@google.com/

--
Thanks,
Mina

Re: [RFC 02/19] netmem: introduce netmem alloc/put API to wrap page alloc/put API

Posted by Byungchul Park 7 months, 1 week ago

On Fri, May 09, 2025 at 07:08:23AM -0700, Mina Almasry wrote:
> j
> 
> On Fri, May 9, 2025 at 6:39 AM Mina Almasry <almasrymina@google.com> wrote:
> >
> > On Fri, May 9, 2025 at 4:51 AM Byungchul Park <byungchul@sk.com> wrote:
> > >
> > > To eliminate the use of struct page in page pool, the page pool code
> > > should use netmem descriptor and API instead.
> > >
> > > As part of the work, introduce netmem alloc/put API allowing the code to
> > > use them rather than struct page things.
> > >
> > > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > > ---
> > >  include/net/netmem.h | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > > index 45c209d6cc689..c87a604e980b9 100644
> > > --- a/include/net/netmem.h
> > > +++ b/include/net/netmem.h
> > > @@ -138,6 +138,24 @@ static inline netmem_ref page_to_netmem(struct page *page)
> > >         return (__force netmem_ref)page;
> > >  }
> > >
> > > +static inline netmem_ref alloc_netmems_node(int nid, gfp_t gfp_mask,
> > > +               unsigned int order)
> > > +{
> > > +       return page_to_netmem(alloc_pages_node(nid, gfp_mask, order));
> > > +}
> > > +
> > > +static inline unsigned long alloc_netmems_bulk_node(gfp_t gfp, int nid,
> > > +               unsigned long nr_netmems, netmem_ref *netmem_array)
> > > +{
> > > +       return alloc_pages_bulk_node(gfp, nid, nr_netmems,
> > > +                       (struct page **)netmem_array);
> > > +}
> > > +
> > > +static inline void put_netmem(netmem_ref netmem)
> > > +{
> > > +       put_page(netmem_to_page(netmem));
> > > +}
> >
> > We can't really do this. netmem_ref is an abstraction that can be a
> > struct page or struct net_iov underneath. We can't be sure when
> > put_netmem is called that it is safe to convert to a page via
> > netmem_to_page(). This will crash if put_netmem is called on a
> > netmem_ref that is a net_iov underneath.
> >
> 
> On a closer look, it looks like this put_netmem is only called from
> code paths where you are sure the netmem_ref is a page underneath, so
> this is likely fine for now. There is a net_next series that is adding
> proper put_netmem support [1]. It's probably best to rebase your work
> on top of that, but this should be fine in the meantime.

Indeed.  Hm..  It'd be the best to work on the top of yours.

If it takes too long, I keep working on as it is for now and will adjust
this patch later once your work gets merged.

	Byungchul

> [1] https://lore.kernel.org/netdev/20250508004830.4100853-1-almasrymina@google.com/
> 
> --
> Thanks,
> Mina