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

["David Hildenbrand (Arm)" <david@kernel.org>]

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.

-- 
Cheers,

David