Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Vlastimil Babka]

On 6/23/25 19:28, Mina Almasry wrote:
> On Mon, Jun 23, 2025 at 10:05 AM Pavel Begunkov <asml.silence@xxxxxxxxx> 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?
> 
> 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.

Note these checks are these days only done with CONFIG_DEBUG_VM, or
debugging/hardening options like debug_pagealloc/init_on_alloc/init_on_free,
see mem_debugging_and_hardening_init() and when check_pages_enabled static
key is enabled.