On Thu, May 29, 2025 at 12:54:31PM -0700, Mina Almasry wrote: > On Wed, May 28, 2025 at 8:11 PM Byungchul Park <byungchul@xxxxxx> wrote: > > > > To simplify struct page, the effort to separate its own descriptor from > > struct page is required and the work for page pool is on going. > > > > To achieve that, all the code should avoid directly accessing page pool > > members of struct page. > > > > Access ->pp_magic through struct netmem_desc instead of directly > > accessing it through struct page in page_pool_page_is_pp(). Plus, move > > page_pool_page_is_pp() from mm.h to netmem.h to use struct netmem_desc > > without header dependency issue. > > > > Signed-off-by: Byungchul Park <byungchul@xxxxxx> > > --- > > include/linux/mm.h | 12 ------------ > > include/net/netmem.h | 14 ++++++++++++++ > > mm/page_alloc.c | 1 + > > 3 files changed, 15 insertions(+), 12 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 8dc012e84033..de10ad386592 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -4311,16 +4311,4 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status); > > */ > > #define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL) > > > > -#ifdef CONFIG_PAGE_POOL > > -static inline bool page_pool_page_is_pp(struct page *page) > > -{ > > - return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE; > > -} > > -#else > > -static inline bool page_pool_page_is_pp(struct page *page) > > -{ > > - return false; > > -} > > -#endif > > - > > #endif /* _LINUX_MM_H */ > > diff --git a/include/net/netmem.h b/include/net/netmem.h > > index f05a8b008d00..9e4ed3530788 100644 > > --- a/include/net/netmem.h > > +++ b/include/net/netmem.h > > @@ -53,6 +53,20 @@ NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count); > > */ > > static_assert(sizeof(struct netmem_desc) <= offsetof(struct page, _refcount)); > > > > +#ifdef CONFIG_PAGE_POOL > > +static inline bool page_pool_page_is_pp(struct page *page) > > +{ > > + struct netmem_desc *desc = (__force struct netmem_desc *)page; > > + > > Is it expected that page can be cast to netmem_desc freely? I know it > works now since netmem_desc and page have the same layout, but how is > it going to continue to work when page is shrunk and no longer has This should be updated once struct netmem_desc has its own instance from slab. As Pavel mentioned, that should be done another way. > 'pp_magic' inside of it? Is that series going to fixup all the places > where casts are done? > > Is it also allowed that we can static cast netmem_desc to page? > > Consider creating netmem_desc_page helper like ptdesc_page. Do we need casting netmem_desc to page? > > I'm not sure the __force is needed too. Ah, I will remove it. Byungchul > > -- > Thanks, > Mina