Re: [PATCH v4] mm: introduce a new page type for page pool in page type
From: David Hildenbrand (Arm)
Date: Wed May 13 2026 - 09:26:26 EST
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