Re: [PATCH v4] mm: introduce a new page type for page pool in page type

[Byungchul Park <byungchul@sk.com>]

On Wed, May 13, 2026 at 03:02:43PM +0200, David Hildenbrand (Arm) wrote:
> On 5/13/26 14:39, Byungchul Park wrote:
> > On Wed, May 13, 2026 at 02:29:46PM +0200, David Hildenbrand (Arm) wrote:
> >  On 5/13/26 14:18, Byungchul Park wrote:
> >>>
> >>> Hi,
> >>>
> >>> The problem comes from the fact that page_type and _mapcount are
> >>> union'ed but there is a case where these two information should be kept
> >>> at the same time.
> >>>
> >>> Why don't we allow these two information can be kept in the 4 bytes at
> >>> the same time until Zi Yan's work on _mapcount and page_type will be
> >>> done, instead of taking a step back?
> >>>
> >>> It can be more optimized but I suggest the approach I just mentioned:
> >>> ---
> >>> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> >>> index 64dc44832808..e5ec204866dc 100644
> >>> --- a/fs/proc/internal.h
> >>> +++ b/fs/proc/internal.h
> >>> @@ -185,8 +185,7 @@ static inline int folio_precise_page_mapcount(struct folio *folio,
> >>>  {
> >>>       int mapcount = atomic_read(&page->_mapcount) + 1;
> >>>
> >>> -     if (page_mapcount_is_type(mapcount))
> >>> -             mapcount = 0;
> >>> +     mapcount = page_mapcount_clear_type(mapcount);
> >>>       if (folio_test_large(folio))
> >>>               mapcount += folio_entire_mapcount(folio);
> >>>
> >>> diff --git a/include/linux/mm.h b/include/linux/mm.h
> >>> index 8260e28205e9..f45064796313 100644
> >>> --- a/include/linux/mm.h
> >>> +++ b/include/linux/mm.h
> >>> @@ -1865,8 +1865,7 @@ static inline int folio_mapcount(const struct folio *folio)
> >>>
> >>>       if (likely(!folio_test_large(folio))) {
> >>>               mapcount = atomic_read(&folio->_mapcount) + 1;
> >>> -             if (page_mapcount_is_type(mapcount))
> >>> -                     mapcount = 0;
> >>> +             mapcount = page_mapcount_clear_type(mapcount);
> >>>               return mapcount;
> >>>       }
> >>>       return folio_large_mapcount(folio);
> >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> >>> index 0e03d816e8b9..f3b0d1fa262d 100644
> >>> --- a/include/linux/page-flags.h
> >>> +++ b/include/linux/page-flags.h
> >>> @@ -934,9 +934,9 @@ static inline bool page_type_has_type(int page_type)
> >>>  }
> >>>
> >>>  /* This takes a mapcount which is one more than page->_mapcount */
> >>> -static inline bool page_mapcount_is_type(unsigned int mapcount)
> >>> +static inline unsigned int page_mapcount_clear_type(unsigned int mapcount)
> >>>  {
> >>> -     return page_type_has_type(mapcount - 1);
> >>> +     return (unsigned int)(((int)(mapcount << 8)) >> 8);
> >>>  }
> >>>
> >>>  static inline bool page_has_type(const struct page *page)
> >>> @@ -953,16 +953,20 @@ static __always_inline void __folio_set_##fname(struct folio *folio)    \
> >>>  {                                                                    \
> >>>       if (folio_test_##fname(folio))                                  \
> >>>               return;                                                 \
> >>> -     VM_BUG_ON_FOLIO(data_race(folio->page.page_type) != UINT_MAX,   \
> >>> +     VM_BUG_ON_FOLIO(page_type_has_type(data_race(folio->page.page_type)), \
> >>>                       folio);                                         \
> >>> -     folio->page.page_type = (unsigned int)PGTY_##lname << 24;       \
> >>> +     folio->page.page_type &= ~(PGTY_mapcount_underflow << 24);      \
> >>> +     folio->page.page_type |= (unsigned int)PGTY_##lname << 24;      \
> >>>  }                                                                    \
> >>>  static __always_inline void __folio_clear_##fname(struct folio *folio)       \
> >>>  {                                                                    \
> >>> -     if (folio->page.page_type == UINT_MAX)                          \
> >>> +     int mapcount;                                                   \
> >>> +                                                                     \
> >>> +     if (!page_type_has_type(folio->page.page_type))                 \
> >>>               return;                                                 \
> >>>       VM_BUG_ON_FOLIO(!folio_test_##fname(folio), folio);             \
> >>> -     folio->page.page_type = UINT_MAX;                               \
> >>> +     mapcount = atomic_read(&folio->page._mapcount);                 \
> >>> +     folio->page.page_type = page_mapcount_clear_type(mapcount);     \
> >>>  }
> >>>
> >>>  #define PAGE_TYPE_OPS(uname, lname, fname)                           \
> >>> @@ -975,15 +979,20 @@ static __always_inline void __SetPage##uname(struct page *page)         \
> >>>  {                                                                    \
> >>>       if (Page##uname(page))                                          \
> >>>               return;                                                 \
> >>> -     VM_BUG_ON_PAGE(data_race(page->page_type) != UINT_MAX, page);   \
> >>> -     page->page_type = (unsigned int)PGTY_##lname << 24;             \
> >>> +     VM_BUG_ON_PAGE(page_type_has_type(data_race(page->page_type)),  \
> >>> +                             page);                                  \
> >>> +     page->page_type &= ~(PGTY_mapcount_underflow << 24);            \
> >>> +     page->page_type |= (unsigned int)PGTY_##lname << 24;            \
> >>>  }                                                                    \
> >>>  static __always_inline void __ClearPage##uname(struct page *page)    \
> >>>  {                                                                    \
> >>> -     if (page->page_type == UINT_MAX)                                \
> >>> +     int mapcount;                                                   \
> >>> +                                                                     \
> >>> +     if (!page_type_has_type(page->page_type))                       \
> >>>               return;                                                 \
> >>>       VM_BUG_ON_PAGE(!Page##uname(page), page);                       \
> >>> -     page->page_type = UINT_MAX;                                     \
> >>> +     mapcount = atomic_read(&page->_mapcount);                       \
> >>> +     page->page_type = page_mapcount_clear_type(mapcount);           \
> >>>  }
> >>>
> >>>  /*
> >>> diff --git a/mm/debug.c b/mm/debug.c
> >>> index 77fa8fe1d641..9a932ded09d4 100644
> >>> --- a/mm/debug.c
> >>> +++ b/mm/debug.c
> >>> @@ -74,8 +74,7 @@ static void __dump_folio(const struct folio *folio, const struct page *page,
> >>>       int mapcount = atomic_read(&page->_mapcount) + 1;
> >>>       char *type = "";
> >>>
> >>> -     if (page_mapcount_is_type(mapcount))
> >>> -             mapcount = 0;
> >>> +     mapcount = page_mapcount_clear_type(mapcount);
> >>>
> >>>       pr_warn("page: refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
> >>>                       folio_ref_count(folio), mapcount, mapping,
> >>> ---
> >>>
> >>> Thoughts?
> >>
> >> God no.
> >
> > This is not final patch, but for sharing the rough idea *with code* -
> > maybe there are more points in code that should be adjusted by the
> > change.  I just typed the draft patch quick just for sharing idea.
> >
> > If we should allow pp type pages to be used in mapping as well, then
> > we should allow a page to keep both its type and mapcount at the same
> > time.  Am I missing something?
> 
> We don't want code to accidentally overflow mapcounts into these bits and have
> them wrongly be detected as page types.
> 
> This is just very fragile.

Okay.  Thanks for the explanation.  Plus, the adjustment I mentioned
might not be as simple as I thought it'd be.

So sorry about the noise.  I'm a zombie now.  I'll think about it after
some rest.

	Byungchul

> --
> Cheers,
> 
> David