linux-kernel - Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is

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: <20250624015458.GD5820@system.software.com>
Date: Tue, 24 Jun 2025 10:54:58 +0900
From: Byungchul Park <byungchul@...com>
To: Pavel Begunkov <asml.silence@...il.com>
Cc: Mina Almasry <almasrymina@...gle.com>,
	David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
	willy@...radead.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	kernel_team@...ynix.com, kuba@...nel.org,
	ilias.apalodimas@...aro.org, harry.yoo@...cle.com, hawk@...nel.org,
	akpm@...ux-foundation.org, davem@...emloft.net,
	john.fastabend@...il.com, andrew+netdev@...n.ch, toke@...hat.com,
	tariqt@...dia.com, edumazet@...gle.com, pabeni@...hat.com,
	saeedm@...dia.com, leon@...nel.org, ast@...nel.org,
	daniel@...earbox.net, lorenzo.stoakes@...cle.com,
	Liam.Howlett@...cle.com, vbabka@...e.cz, rppt@...nel.org,
	surenb@...gle.com, mhocko@...e.com, horms@...nel.org,
	linux-rdma@...r.kernel.org, bpf@...r.kernel.org,
	vishal.moola@...il.com, hannes@...xchg.org, jackmanb@...gle.com
Subject: Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through
 struct netmem_desc in page_pool_page_is_pp()

On Mon, Jun 23, 2025 at 07:14:41PM +0100, Pavel Begunkov wrote:
> On 6/23/25 18:28, Mina Almasry wrote:
> > On Mon, Jun 23, 2025 at 10:05 AM Pavel Begunkov <asml.silence@...il.com> wrote:
> ...>> As you said, it's just a sanity check, all page pool pages should
> > > be freed by the networking code. It checks the ownership with
> > > netmem_is_pp(), which is basically the same as page_pool_page_is_pp()
> > > but done though some aliasing.
> > > 
> > > static inline bool netmem_is_pp(netmem_ref netmem)
> > > {
> > >          return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > }
> > > 
> > > I assume there is no point in moving the check to skbuff.c as it
> > > already does exactly same test, but we can probably just kill it.
> > > 
> > 
> > Even if we do kill it, maybe lets do that in a separate patch, and
> > maybe a separate series. I would recommend not complicating this one?
> FWIW, the discussion somewhat mentioned "long term", but I'm not
> suggesting actually removing it, it serves the purpose. And in
> long term the helper will be converted to use page->type / etc.
> without touching pp fields, that should reduce the degree of
> ugliness and make it more acceptable for keeping in mm.

Agree.

	Byungchul
> 
> > Also, AFAIU, this is about removing/moving the checks in
> > bad_page_reason() and page_expected_state()? I think this check does
> > fire sometimes. I saw at least 1 report in the last year of a
> > bad_page_reason() check firing because the page_pool got its
> > accounting wrong and released a page to the buddy allocator early, so
> > maybe that new patch that removes that check should explain why this
> > check is no longer necessary.
> 
> --
> Pavel Begunkov