[PATCH v3 mm-new] fs/proc/page: avoid anon folio checks on typed pages except hugetlb

[PATCH v3 mm-new] fs/proc/page: avoid anon folio checks on typed pages except hugetlb

[Harry Yoo]

folio_test_anon() and folio_test_ksm() may return false positives when
invoked on typed pages (except hugetlb folios), because lower bits of
folio->mapping may be set even when they do not indicate
FOLIO_MAPPING_* flags.

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

Currently, and going forward (The New York interpretation),
typed pages except hugetlb do not have FOLIO_MAPPING_* flags.
In the future, they won't even cast to folios.

For now, avoid checking FOLIO_MAPPING_* flags on pages if they are
typed pages that are not hugetlb folios.

Update the comment in FOLIO_MAPPING_* flags accordingly.

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>
---

v2->v3: Did not introduce folio_has_mapcount() per David's suggestion
v2: https://lore.kernel.org/linux-mm/20250707120740.4413-1-harry.yoo@oracle.com

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

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 0cdc78c0d23f..07582dbdef45 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 (!page_has_type(&folio->page) || folio_test_hugetlb(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/page-flags.h b/include/linux/page-flags.h
index 8e4d6eda8a8d..26ae4c7cf8dd 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 !page_has_type(&folio->page) || folio_test_hugetlb(folio)
+ * actually evaluates to true before calling the following functions
+ * (e.g., folio_test_anon).
+ *
+ * This check can be skipped if the folio is mapped to userspace, since
+ * typed pages except hugetlb cannot be mapped to userspace at all.
  */
 #define FOLIO_MAPPING_ANON	0x1
 #define FOLIO_MAPPING_ANON_KSM	0x2
-- 
2.43.0

Re: [PATCH v3 mm-new] fs/proc/page: avoid anon folio checks on typed pages except hugetlb

[David Hildenbrand]

On 15.07.25 14:07, Harry Yoo wrote:
> folio_test_anon() and folio_test_ksm() may return false positives when
> invoked on typed pages (except hugetlb folios), because lower bits of
> folio->mapping may be set even when they do not indicate
> FOLIO_MAPPING_* flags.
> 
> This leads to tools/mm/page-types reporting pages with
> KPF_SLAB, KPF_ANON and KPF_KSM (with flags, page-counts, MB omitted):
>    $ sudo ./page-types | grep slab
>    _______S___________________________________   slab
>    _______S____a________x_____________________   slab,anonymous,ksm
> 
> Currently, and going forward (The New York interpretation),
> typed pages except hugetlb do not have FOLIO_MAPPING_* flags.
> In the future, they won't even cast to folios.
> 
> For now, avoid checking FOLIO_MAPPING_* flags on pages if they are
> typed pages that are not hugetlb folios.
> 
> Update the comment in FOLIO_MAPPING_* flags accordingly.
> 
> 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>
> ---
> 
> v2->v3: Did not introduce folio_has_mapcount() per David's suggestion
> v2: https://lore.kernel.org/linux-mm/20250707120740.4413-1-harry.yoo@oracle.com
> 
>   fs/proc/page.c             | 19 +++++++++++--------
>   include/linux/page-flags.h | 15 +++++++++------
>   2 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 0cdc78c0d23f..07582dbdef45 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 (!page_has_type(&folio->page) || folio_test_hugetlb(folio)) {
> +		mapping = (unsigned long)folio->mapping;
> +		is_anon = mapping & FOLIO_MAPPING_ANON;
> +
> +		if (page_mapped(page))
> +			u |= 1 << KPF_MMAP;

Note: Luiz switches to folio_mapped() in his patch that is in mm-new.

> +		if (is_anon) {
> +			u |= 1 << KPF_ANON;
> +			if (mapping & FOLIO_MAPPING_KSM)
> +				u |= 1 << KPF_KSM;

Can we just switch to folio_test_anon() and folio_test_ksm() ?

I don't really see a reason to not do that. Willy converted these
checks from page -> mapping, but we should really just use the folio_test_* functions
I think.

And looking at it, I think Willy introduced an issue in:

commit dee3d0bef2b00772be430425832ead6aa9d707f9
Author: Matthew Wilcox (Oracle) <willy@infradead.org>
Date:   Tue Mar 26 17:10:32 2024 +0000

     proc: rewrite stable_page_flags()


We replaced

	if (PageKsm(page))

essentially by

	if (mapping & PAGE_MAPPING_KSM)

But

	#define PAGE_MAPPING_KSM       (PAGE_MAPPING_ANON | PAGE_MAPPING_ANON_KSM)

So wouldn't we just indicate *all* anon pages as ... KSM pages?

> +		}
>   	}
>   
>   	/*
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 8e4d6eda8a8d..26ae4c7cf8dd 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

I would write that explicitly:

that you are dealing with an actual folio: either page has no type or the type indicates a folio (hugetlb).


-- 
Cheers,

David / dhildenb

Re: [PATCH v3 mm-new] fs/proc/page: avoid anon folio checks on typed pages except hugetlb

[Harry Yoo]

On Tue, Jul 15, 2025 at 02:31:45PM +0200, David Hildenbrand wrote:
> On 15.07.25 14:07, Harry Yoo wrote:
> > folio_test_anon() and folio_test_ksm() may return false positives when
> > invoked on typed pages (except hugetlb folios), because lower bits of
> > folio->mapping may be set even when they do not indicate
> > FOLIO_MAPPING_* flags.
> > 
> > This leads to tools/mm/page-types reporting pages with
> > KPF_SLAB, KPF_ANON and KPF_KSM (with flags, page-counts, MB omitted):
> >    $ sudo ./page-types | grep slab
> >    _______S___________________________________   slab
> >    _______S____a________x_____________________   slab,anonymous,ksm
> > 
> > Currently, and going forward (The New York interpretation),
> > typed pages except hugetlb do not have FOLIO_MAPPING_* flags.
> > In the future, they won't even cast to folios.
> > 
> > For now, avoid checking FOLIO_MAPPING_* flags on pages if they are
> > typed pages that are not hugetlb folios.
> > 
> > Update the comment in FOLIO_MAPPING_* flags accordingly.
> > 
> > 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>
> > ---
> > 
> > v2->v3: Did not introduce folio_has_mapcount() per David's suggestion
> > v2: https://lore.kernel.org/linux-mm/20250707120740.4413-1-harry.yoo@oracle.com
> > 
> >   fs/proc/page.c             | 19 +++++++++++--------
> >   include/linux/page-flags.h | 15 +++++++++------
> >   2 files changed, 20 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/proc/page.c b/fs/proc/page.c
> > index 0cdc78c0d23f..07582dbdef45 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 (!page_has_type(&folio->page) || folio_test_hugetlb(folio)) {
> > +		mapping = (unsigned long)folio->mapping;
> > +		is_anon = mapping & FOLIO_MAPPING_ANON;
> > +
> > +		if (page_mapped(page))
> > +			u |= 1 << KPF_MMAP;
> 
> Note: Luiz switches to folio_mapped() in his patch that is in mm-new.

Right, but in mm-new Luiz's patch already depends on my v2 patch,
which still leaves me confused about what I should do... :)

> > +		if (is_anon) {
> > +			u |= 1 << KPF_ANON;
> > +			if (mapping & FOLIO_MAPPING_KSM)
> > +				u |= 1 << KPF_KSM;
> 
> Can we just switch to folio_test_anon() and folio_test_ksm() ?
> 
> I don't really see a reason to not do that. Willy converted these
> checks from page -> mapping, but we should really just use the folio_test_* functions
> I think.
> 
> And looking at it, I think Willy introduced an issue in:
> 
> commit dee3d0bef2b00772be430425832ead6aa9d707f9
> Author: Matthew Wilcox (Oracle) <willy@infradead.org>
> Date:   Tue Mar 26 17:10:32 2024 +0000
> 
>     proc: rewrite stable_page_flags()
> 
> 
> We replaced
> 
> 	if (PageKsm(page))
> 
> essentially by
> 
> 	if (mapping & PAGE_MAPPING_KSM)
> 
> But
> 
> 	#define PAGE_MAPPING_KSM       (PAGE_MAPPING_ANON | PAGE_MAPPING_ANON_KSM)
> 
> So wouldn't we just indicate *all* anon pages as ... KSM pages?

Oops, I didn't notice that!

I think you're right. I don't see any problem with going back to
using folio_test_anon/ksm().

> > +		}
> >   	}
> >   	/*
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 8e4d6eda8a8d..26ae4c7cf8dd 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
> 
> I would write that explicitly:
> 
> that you are dealing with an actual folio: either page has no type or the type indicates a folio (hugetlb).

Will adjust, thanks!

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v3 mm-new] fs/proc/page: avoid anon folio checks on typed pages except hugetlb

[David Hildenbrand]

On 15.07.25 15:30, Harry Yoo wrote:
> On Tue, Jul 15, 2025 at 02:31:45PM +0200, David Hildenbrand wrote:
>> On 15.07.25 14:07, Harry Yoo wrote:
>>> folio_test_anon() and folio_test_ksm() may return false positives when
>>> invoked on typed pages (except hugetlb folios), because lower bits of
>>> folio->mapping may be set even when they do not indicate
>>> FOLIO_MAPPING_* flags.
>>>
>>> This leads to tools/mm/page-types reporting pages with
>>> KPF_SLAB, KPF_ANON and KPF_KSM (with flags, page-counts, MB omitted):
>>>     $ sudo ./page-types | grep slab
>>>     _______S___________________________________   slab
>>>     _______S____a________x_____________________   slab,anonymous,ksm
>>>
>>> Currently, and going forward (The New York interpretation),
>>> typed pages except hugetlb do not have FOLIO_MAPPING_* flags.
>>> In the future, they won't even cast to folios.
>>>
>>> For now, avoid checking FOLIO_MAPPING_* flags on pages if they are
>>> typed pages that are not hugetlb folios.
>>>
>>> Update the comment in FOLIO_MAPPING_* flags accordingly.
>>>
>>> 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>
>>> ---
>>>
>>> v2->v3: Did not introduce folio_has_mapcount() per David's suggestion
>>> v2: https://lore.kernel.org/linux-mm/20250707120740.4413-1-harry.yoo@oracle.com
>>>
>>>    fs/proc/page.c             | 19 +++++++++++--------
>>>    include/linux/page-flags.h | 15 +++++++++------
>>>    2 files changed, 20 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/fs/proc/page.c b/fs/proc/page.c
>>> index 0cdc78c0d23f..07582dbdef45 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 (!page_has_type(&folio->page) || folio_test_hugetlb(folio)) {
>>> +		mapping = (unsigned long)folio->mapping;
>>> +		is_anon = mapping & FOLIO_MAPPING_ANON;
>>> +
>>> +		if (page_mapped(page))
>>> +			u |= 1 << KPF_MMAP;
>>
>> Note: Luiz switches to folio_mapped() in his patch that is in mm-new.
> 
> Right, but in mm-new Luiz's patch already depends on my v2 patch,
> which still leaves me confused about what I should do... :)

Haha, right. Yes, just send it as a replacement to you current patch 
then. The conflict should be easy to resolve.

-- 
Cheers,

David / dhildenb