[PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[Harry Yoo]

folio_test_anon() and folio_test_ksm() may return false positives when
the folio is a typed page (except hugetlb), because lower bits of
folio->mapping field can be set even if it doesn't mean
FOLIO_MAPPING_* flags.

To avoid false positives, folio_test_{anon,ksm}() should be called
only if !page_has_type(&folio->page) || folio_test_hugetlb(folio).
However, the check can be skipped if a folio is or will be mapped
to userspace because typed pages that are not hugetlb folios cannot
be mapped to userspace.

As folio_expected_ref_count() already does the check, introduce a helper
function folio_has_mapcount() and use it in folio_expected_ref_count()
and stable_page_flags().

Update the comment in FOLIO_MAPPING_* flags accordingly.

This fixes tools/mm/page-types reporting pages with
KPF_SLAB, KPF_ANON and KPF_SLAB (with flags, page-counts, MB omitted):
  $ sudo ./page-types | grep slab
  _______S___________________________________	slab
  _______S____a________x_____________________	slab,anonymous,ksm

Fixes: 130d4df57390 ("mm/sl[au]b: rearrange struct slab fields to allow larger rcu_head")
Suggested-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Harry Yoo <harry.yoo@oracle.com>
---

No Cc: stable because incorrect kpageflags is not considered a serious bug.

v1: https://lore.kernel.org/linux-mm/20250609132723.13118-1-harry.yoo@oracle.com/

v1 -> v2:
  - Dropped tricks to fix folio_test_slab() false positives and instead
    adjusted Vlastimil's feedback: it sounds like a better approach to
    check folio_test_slab() before folio_test_{anon,ksm}().

    But a slight change from Vlastimil's suggestion: 
    check (!page_has_type() || PageHuge()) instead of folio_test_slab(),
    which should be more robust. (Borrowed from David's patch)

 fs/proc/page.c             | 19 +++++++++++--------
 include/linux/mm.h         |  2 +-
 include/linux/page-flags.h | 20 ++++++++++++++------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 0cdc78c0d23f..d6efddf6adf9 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -148,18 +148,21 @@ u64 stable_page_flags(const struct page *page)
 	folio = page_folio(page);
 
 	k = folio->flags;
-	mapping = (unsigned long)folio->mapping;
-	is_anon = mapping & FOLIO_MAPPING_ANON;
 
 	/*
 	 * pseudo flags for the well known (anonymous) memory mapped pages
 	 */
-	if (page_mapped(page))
-		u |= 1 << KPF_MMAP;
-	if (is_anon) {
-		u |= 1 << KPF_ANON;
-		if (mapping & FOLIO_MAPPING_KSM)
-			u |= 1 << KPF_KSM;
+	if (folio_has_mapcount(folio)) {
+		mapping = (unsigned long)folio->mapping;
+		is_anon = mapping & FOLIO_MAPPING_ANON;
+
+		if (page_mapped(page))
+			u |= 1 << KPF_MMAP;
+		if (is_anon) {
+			u |= 1 << KPF_ANON;
+			if (mapping & FOLIO_MAPPING_KSM)
+				u |= 1 << KPF_KSM;
+		}
 	}
 
 	/*
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 805108d7bbc3..8bc6a38e0f9d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2167,7 +2167,7 @@ static inline int folio_expected_ref_count(const struct folio *folio)
 	const int order = folio_order(folio);
 	int ref_count = 0;
 
-	if (WARN_ON_ONCE(page_has_type(&folio->page) && !folio_test_hugetlb(folio)))
+	if (WARN_ON_ONCE(!folio_has_mapcount(folio)))
 		return 0;
 
 	if (folio_test_anon(folio)) {
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 970600d79dac..00ba2c8b221e 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
  * address_space which maps the folio from disk; whereas "folio_mapped"
  * refers to user virtual address space into which the folio is mapped.
  *
- * For slab pages, since slab reuses the bits in struct page to store its
- * internal states, the folio->mapping does not exist as such, nor do
- * these flags below.  So in order to avoid testing non-existent bits,
- * please make sure that folio_test_slab(folio) actually evaluates to
- * false before calling the following functions (e.g., folio_test_anon).
- * See mm/slab.h.
+ * For certain typed pages like slabs, since they reuse bits in struct page
+ * to store internal states, folio->mapping does not point to a valid
+ * mapping, nor do these flags exist. To avoid testing non-existent bits,
+ * make sure folio_has_mapcount() actually evaluates to true before calling
+ * the following functions (e.g., folio_test_anon).
+ *
+ * The folio_has_mapcount() check can be skipped if the folio is mapped
+ * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
+ * to userspace at all.
  */
 #define FOLIO_MAPPING_ANON	0x1
 #define FOLIO_MAPPING_ANON_KSM	0x2
@@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
 	return folio_test_hugetlb(page_folio(page));
 }
 
+static inline bool folio_has_mapcount(const struct folio *folio)
+{
+	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
+}
+
 /*
  * Check if a page is currently marked HWPoisoned. Note that this check is
  * best effort only and inherently racy: there is no way to synchronize with
-- 
2.43.0

Re: [PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[David Hildenbrand]

>   	if (folio_test_anon(folio)) {
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 970600d79dac..00ba2c8b221e 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
>    * address_space which maps the folio from disk; whereas "folio_mapped"
>    * refers to user virtual address space into which the folio is mapped.
>    *
> - * For slab pages, since slab reuses the bits in struct page to store its
> - * internal states, the folio->mapping does not exist as such, nor do
> - * these flags below.  So in order to avoid testing non-existent bits,
> - * please make sure that folio_test_slab(folio) actually evaluates to
> - * false before calling the following functions (e.g., folio_test_anon).
> - * See mm/slab.h.
> + * For certain typed pages like slabs, since they reuse bits in struct page
> + * to store internal states, folio->mapping does not point to a valid
> + * mapping, nor do these flags exist. To avoid testing non-existent bits,
> + * make sure folio_has_mapcount() actually evaluates to true before calling
> + * the following functions (e.g., folio_test_anon).
> + *
> + * The folio_has_mapcount() check can be skipped if the folio is mapped
> + * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
> + * to userspace at all.
>    */
>   #define FOLIO_MAPPING_ANON	0x1
>   #define FOLIO_MAPPING_ANON_KSM	0x2
> @@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
>   	return folio_test_hugetlb(page_folio(page));
>   }
>   
> +static inline bool folio_has_mapcount(const struct folio *folio)
> +{
> +	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
> +}
> +

I don't like the naming.

The thing is, in the future only folios will have a mapcount.

Asking whether a folio has a mapcount is asking the wrong question.

It's rather, that we want to reject *pages* early, and not cast them to 
the folio in the first place.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[Harry Yoo]

On Tue, Jul 08, 2025 at 10:07:26AM +0200, David Hildenbrand wrote:
> >   	if (folio_test_anon(folio)) {
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 970600d79dac..00ba2c8b221e 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
> >    * address_space which maps the folio from disk; whereas "folio_mapped"
> >    * refers to user virtual address space into which the folio is mapped.
> >    *
> > - * For slab pages, since slab reuses the bits in struct page to store its
> > - * internal states, the folio->mapping does not exist as such, nor do
> > - * these flags below.  So in order to avoid testing non-existent bits,
> > - * please make sure that folio_test_slab(folio) actually evaluates to
> > - * false before calling the following functions (e.g., folio_test_anon).
> > - * See mm/slab.h.
> > + * For certain typed pages like slabs, since they reuse bits in struct page
> > + * to store internal states, folio->mapping does not point to a valid
> > + * mapping, nor do these flags exist. To avoid testing non-existent bits,
> > + * make sure folio_has_mapcount() actually evaluates to true before calling
> > + * the following functions (e.g., folio_test_anon).
> > + *
> > + * The folio_has_mapcount() check can be skipped if the folio is mapped
> > + * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
> > + * to userspace at all.
> >    */
> >   #define FOLIO_MAPPING_ANON	0x1
> >   #define FOLIO_MAPPING_ANON_KSM	0x2
> > @@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
> >   	return folio_test_hugetlb(page_folio(page));
> >   }
> > +static inline bool folio_has_mapcount(const struct folio *folio)
> > +{
> > +	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
> > +}
> > +
> 
> I don't like the naming.

I don't like it either, but I couldn't come up with a better name :(

> The thing is, in the future only folios will have a mapcount.

Agreed.

> Asking whether a folio has a mapcount is asking the wrong question.

Agreed.

> It's rather, that we want to reject *pages* early, and not cast them to the
> folio in the first place.

Totally agreed.

But that's the future, and we're not there yet.
What should we do now?

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[David Hildenbrand]

On 08.07.25 10:30, Harry Yoo wrote:
> On Tue, Jul 08, 2025 at 10:07:26AM +0200, David Hildenbrand wrote:
>>>    	if (folio_test_anon(folio)) {
>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>> index 970600d79dac..00ba2c8b221e 100644
>>> --- a/include/linux/page-flags.h
>>> +++ b/include/linux/page-flags.h
>>> @@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
>>>     * address_space which maps the folio from disk; whereas "folio_mapped"
>>>     * refers to user virtual address space into which the folio is mapped.
>>>     *
>>> - * For slab pages, since slab reuses the bits in struct page to store its
>>> - * internal states, the folio->mapping does not exist as such, nor do
>>> - * these flags below.  So in order to avoid testing non-existent bits,
>>> - * please make sure that folio_test_slab(folio) actually evaluates to
>>> - * false before calling the following functions (e.g., folio_test_anon).
>>> - * See mm/slab.h.
>>> + * For certain typed pages like slabs, since they reuse bits in struct page
>>> + * to store internal states, folio->mapping does not point to a valid
>>> + * mapping, nor do these flags exist. To avoid testing non-existent bits,
>>> + * make sure folio_has_mapcount() actually evaluates to true before calling
>>> + * the following functions (e.g., folio_test_anon).
>>> + *
>>> + * The folio_has_mapcount() check can be skipped if the folio is mapped
>>> + * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
>>> + * to userspace at all.
>>>     */
>>>    #define FOLIO_MAPPING_ANON	0x1
>>>    #define FOLIO_MAPPING_ANON_KSM	0x2
>>> @@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
>>>    	return folio_test_hugetlb(page_folio(page));
>>>    }
>>> +static inline bool folio_has_mapcount(const struct folio *folio)
>>> +{
>>> +	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
>>> +}
>>> +
>>
>> I don't like the naming.
> 
> I don't like it either, but I couldn't come up with a better name :(
> 
>> The thing is, in the future only folios will have a mapcount.
> 
> Agreed.
> 
>> Asking whether a folio has a mapcount is asking the wrong question.
> 
> Agreed.
> 
>> It's rather, that we want to reject *pages* early, and not cast them to the
>> folio in the first place.
> 
> Totally agreed.
> 
> But that's the future, and we're not there yet.
> What should we do now?

Check before converting from page to folio in stable_page_flags() and 
leave folio_expected_ref_count() alone. In folio_expected_ref_count() 
it's a sanity check that should never happen.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[Harry Yoo]

On Tue, Jul 08, 2025 at 10:32:44AM +0200, David Hildenbrand wrote:
> On 08.07.25 10:30, Harry Yoo wrote:
> > On Tue, Jul 08, 2025 at 10:07:26AM +0200, David Hildenbrand wrote:
> > > >    	if (folio_test_anon(folio)) {
> > > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > > > index 970600d79dac..00ba2c8b221e 100644
> > > > --- a/include/linux/page-flags.h
> > > > +++ b/include/linux/page-flags.h
> > > > @@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
> > > >     * address_space which maps the folio from disk; whereas "folio_mapped"
> > > >     * refers to user virtual address space into which the folio is mapped.
> > > >     *
> > > > - * For slab pages, since slab reuses the bits in struct page to store its
> > > > - * internal states, the folio->mapping does not exist as such, nor do
> > > > - * these flags below.  So in order to avoid testing non-existent bits,
> > > > - * please make sure that folio_test_slab(folio) actually evaluates to
> > > > - * false before calling the following functions (e.g., folio_test_anon).
> > > > - * See mm/slab.h.
> > > > + * For certain typed pages like slabs, since they reuse bits in struct page
> > > > + * to store internal states, folio->mapping does not point to a valid
> > > > + * mapping, nor do these flags exist. To avoid testing non-existent bits,
> > > > + * make sure folio_has_mapcount() actually evaluates to true before calling
> > > > + * the following functions (e.g., folio_test_anon).
> > > > + *
> > > > + * The folio_has_mapcount() check can be skipped if the folio is mapped
> > > > + * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
> > > > + * to userspace at all.
> > > >     */
> > > >    #define FOLIO_MAPPING_ANON	0x1
> > > >    #define FOLIO_MAPPING_ANON_KSM	0x2
> > > > @@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
> > > >    	return folio_test_hugetlb(page_folio(page));
> > > >    }
> > > > +static inline bool folio_has_mapcount(const struct folio *folio)
> > > > +{
> > > > +	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
> > > > +}
> > > > +
> > > 
> > > I don't like the naming.
> > 
> > I don't like it either, but I couldn't come up with a better name :(
> > 
> > > The thing is, in the future only folios will have a mapcount.
> > 
> > Agreed.
> > 
> > > Asking whether a folio has a mapcount is asking the wrong question.
> > 
> > Agreed.
> > 
> > > It's rather, that we want to reject *pages* early, and not cast them to the
> > > folio in the first place.
> > 
> > Totally agreed.
> > 
> > But that's the future, and we're not there yet.
> > What should we do now?
> 
> Check before converting from page to folio in stable_page_flags() and leave
> folio_expected_ref_count() alone. In folio_expected_ref_count() it's a
> sanity check that should never happen.

TBH it wasn't clear when the page-to-folio conversion should be
rejected... (I can't just change page_folio() to reject that, right now)

But now I get your point. It shouldn't cast to folio and do
folio-specific checks if it can't be a folio.

The suggestion makes sense and will do that.
Appreciate your comment, David!

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v2 mm-new] mm: check if folio has valid mapcount before folio_test_{anon,ksm}() when necessary

[David Hildenbrand]

On 08.07.25 10:56, Harry Yoo wrote:
> On Tue, Jul 08, 2025 at 10:32:44AM +0200, David Hildenbrand wrote:
>> On 08.07.25 10:30, Harry Yoo wrote:
>>> On Tue, Jul 08, 2025 at 10:07:26AM +0200, David Hildenbrand wrote:
>>>>>     	if (folio_test_anon(folio)) {
>>>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>>>> index 970600d79dac..00ba2c8b221e 100644
>>>>> --- a/include/linux/page-flags.h
>>>>> +++ b/include/linux/page-flags.h
>>>>> @@ -706,12 +706,15 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
>>>>>      * address_space which maps the folio from disk; whereas "folio_mapped"
>>>>>      * refers to user virtual address space into which the folio is mapped.
>>>>>      *
>>>>> - * For slab pages, since slab reuses the bits in struct page to store its
>>>>> - * internal states, the folio->mapping does not exist as such, nor do
>>>>> - * these flags below.  So in order to avoid testing non-existent bits,
>>>>> - * please make sure that folio_test_slab(folio) actually evaluates to
>>>>> - * false before calling the following functions (e.g., folio_test_anon).
>>>>> - * See mm/slab.h.
>>>>> + * For certain typed pages like slabs, since they reuse bits in struct page
>>>>> + * to store internal states, folio->mapping does not point to a valid
>>>>> + * mapping, nor do these flags exist. To avoid testing non-existent bits,
>>>>> + * make sure folio_has_mapcount() actually evaluates to true before calling
>>>>> + * the following functions (e.g., folio_test_anon).
>>>>> + *
>>>>> + * The folio_has_mapcount() check can be skipped if the folio is mapped
>>>>> + * to userspace, since a folio with !folio_has_mapcount() cannot be mapped
>>>>> + * to userspace at all.
>>>>>      */
>>>>>     #define FOLIO_MAPPING_ANON	0x1
>>>>>     #define FOLIO_MAPPING_ANON_KSM	0x2
>>>>> @@ -1092,6 +1095,11 @@ static inline bool PageHuge(const struct page *page)
>>>>>     	return folio_test_hugetlb(page_folio(page));
>>>>>     }
>>>>> +static inline bool folio_has_mapcount(const struct folio *folio)
>>>>> +{
>>>>> +	return !page_has_type(&folio->page) || folio_test_hugetlb(folio);
>>>>> +}
>>>>> +
>>>>
>>>> I don't like the naming.
>>>
>>> I don't like it either, but I couldn't come up with a better name :(
>>>
>>>> The thing is, in the future only folios will have a mapcount.
>>>
>>> Agreed.
>>>
>>>> Asking whether a folio has a mapcount is asking the wrong question.
>>>
>>> Agreed.
>>>
>>>> It's rather, that we want to reject *pages* early, and not cast them to the
>>>> folio in the first place.
>>>
>>> Totally agreed.
>>>
>>> But that's the future, and we're not there yet.
>>> What should we do now?
>>
>> Check before converting from page to folio in stable_page_flags() and leave
>> folio_expected_ref_count() alone. In folio_expected_ref_count() it's a
>> sanity check that should never happen.
> 
> TBH it wasn't clear when the page-to-folio conversion should be
> rejected... (I can't just change page_folio() to reject that, right now)

Yes, willy's plan is to make page_folio() fail later. For now, we can 
just do racy checks ahead of time (kpagecount code is inherently racy).

-- 
Cheers,

David / dhildenb