linux-kernel - Re: [PATCH linux-next v3] mm, page_pool: introduce a new page type for page pool in page type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250804010307.GA39461@system.software.com>
Date: Mon, 4 Aug 2025 10:03:07 +0900
From: Byungchul Park <byungchul@...com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: linux-mm@...ck.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel_team@...ynix.com,
	harry.yoo@...cle.com, ast@...nel.org, daniel@...earbox.net,
	davem@...emloft.net, hawk@...nel.org, john.fastabend@...il.com,
	sdf@...ichev.me, saeedm@...dia.com, leon@...nel.org,
	tariqt@...dia.com, mbloch@...dia.com, andrew+netdev@...n.ch,
	edumazet@...gle.com, pabeni@...hat.com, akpm@...ux-foundation.org,
	david@...hat.com, lorenzo.stoakes@...cle.com,
	Liam.Howlett@...cle.com, vbabka@...e.cz, rppt@...nel.org,
	surenb@...gle.com, mhocko@...e.com, horms@...nel.org,
	jackmanb@...gle.com, hannes@...xchg.org, ziy@...dia.com,
	ilias.apalodimas@...aro.org, willy@...radead.org,
	brauner@...nel.org, kas@...nel.org, yuzhao@...gle.com,
	usamaarif642@...il.com, baolin.wang@...ux.alibaba.com,
	almasrymina@...gle.com, toke@...hat.com, asml.silence@...il.com,
	bpf@...r.kernel.org, linux-rdma@...r.kernel.org,
	sfr@...b.auug.org.au
Subject: Re: [PATCH linux-next v3] mm, page_pool: introduce a new page type
 for page pool in page type

On Fri, Aug 01, 2025 at 04:08:05PM -0700, Jakub Kicinski wrote:
> On Tue, 29 Jul 2025 20:02:10 +0900 Byungchul Park wrote:
> > [PATCH linux-next v3] mm, page_pool: introduce a new page type for page pool in page type
> 
> linux-next does not accept patches. This has to go either via networking or MM.

Yeah.  That's what I found.  Thanks for the confirmation.

> > -     if (unlikely(page_has_type(page)))
> > +     if (unlikely(page_has_type(page))) {
> 
> Maybe add :
> 
>                 /* networking expects to clear its page type before releasing */

I will.

> > +             WARN_ON_ONCE(PageNetpp(page));
> >               /* Reset the page_type (which overlays _mapcount) */
> >               page->page_type = UINT_MAX;
> > +     }
> 
> >  static inline bool netmem_is_pp(netmem_ref netmem)
> >  {
> > -     return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
> > +     /* Use ->pp for net_iov to identify if it's pp,
> 
> Please try to use precise language, this code is confusing as is.
> net_iov may _belong_ to a page pool.
> 
>         * which requires that non-pp net_iov should have ->pp NULL'd.

Thank you.  I will.

> I don't think this adds any information.
> 
> > +      */
> > +     if (netmem_is_net_iov(netmem))
> > +             return !!__netmem_clear_lsb(netmem)->pp;
> > +
> > +     /* For system memory, page type bit in struct page can be used
> 
> "page type bit" -> "page type", it's not a bit.
> 
> > +      * to identify if it's pp.
> 
> ... to identify pages which belong to a page pool.

I will.

> > +      */
> > +     return PageNetpp(__netmem_to_page(netmem));
> >  }
> >
> >  static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
> > diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> > index 05e2e22a8f7c..37eeab76c41c 100644
> > --- a/net/core/page_pool.c
> > +++ b/net/core/page_pool.c
> > @@ -654,7 +654,6 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict)
> >  void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> >  {
> >       netmem_set_pp(netmem, pool);
> > -     netmem_or_pp_magic(netmem, PP_SIGNATURE);
> >
> >       /* Ensuring all pages have been split into one fragment initially:
> >        * page_pool_set_pp_info() is only called once for every page when it
> > @@ -665,12 +664,19 @@ void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> >       page_pool_fragment_netmem(netmem, 1);
> >       if (pool->has_init_callback)
> >               pool->slow.init_callback(netmem, pool->slow.init_arg);
> > +
> > +     /* If it's page-backed */
> 
> Please don't add obvious comments.

I added the comment since it's _not_ obvious.  !net_iov means it's not
net_iov, not it's page-backed.  However, since you, as the maintainer,
are okay, I will remove it.  Thanks.

	Byungchul

> > +     if (!netmem_is_net_iov(netmem))
> > +             __SetPageNetpp(__netmem_to_page(netmem));