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

[Byungchul Park <byungchul@sk.com>]

On Sat, Jul 12, 2025 at 04:09:13PM +0100, Pavel Begunkov wrote:
> On 7/12/25 15:52, David Hildenbrand wrote:
> > On 12.07.25 15:58, Pavel Begunkov wrote:
> > > On 7/11/25 02:14, Byungchul Park wrote:
> > > ...>>> +#ifdef CONFIG_PAGE_POOL
> > > > > > +/* XXX: This would better be moved to mm, once mm gets its way to
> > > > > > + * identify the type of page for page pool.
> > > > > > + */
> > > > > > +static inline bool page_pool_page_is_pp(struct page *page)
> > > > > > +{
> > > > > > +       struct netmem_desc *desc = page_to_nmdesc(page);
> > > > > > +
> > > > > > +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > > > > +}
> > > > > 
> > > > > pages can be pp pages (where they have pp fields inside of them) or
> > > > > non-pp pages (where they don't have pp fields inside them, because
> > > > > they were never allocated from the page_pool).
> > > > > 
> > > > > Casting a page to a netmem_desc, and then checking if the page was a
> > > > > pp page doesn't makes sense to me on a fundamental level. The
> > > > > netmem_desc is only valid if the page was a pp page in the first
> > > > > place. Maybe page_to_nmdesc should reject the cast if the page is not
> > > > > a pp page or something.
> > > > 
> > > > Right, as you already know, the current mainline code already has the
> > > > same problem but we've been using the werid way so far, in other words,
> > > > mm code is checking if it's a pp page or not by using ->pp_magic, but
> > > > it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
> > > > 
> > > > Both the mainline code and this patch can make sense *only if* it's
> > > > actually a pp page.  It's unevitable until mm provides a way to identify
> > > > the type of page for page pool.  Thoughts?
> > > Question to mm folks, can we add a new PGTY for page pool and use
> > > that to filter page pool originated pages? Like in the incomplete
> > > and untested diff below?
> > 
> > https://lore.kernel.org/all/77c6a6dd-0e03-4b81-a9c7-eaecaa4ebc0b@redhat.com/
> 
> Great, then it'll be the right thing to do here. I somehow missed
> the post, will add your suggested-by.

It'd be the ideal.  I will wait and work on top of your patch then.

	Byungchul
> 
> --
> Pavel Begunkov