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: <69762ce3-ead1-4324-ba33-9839efbe31e7@gmail.com>
Date: Mon, 23 Jun 2025 19:14:41 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Mina Almasry <almasrymina@...gle.com>
Cc: David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
 Byungchul Park <byungchul@...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 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.

> 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