diff mbox series

[v3] kpageflags: respect folio head-page flag placement

Message ID 20240320-kpageflags-svetly-v3-1-b6725843bfa7@memverge.com (mailing list archive)
State New
Headers show
Series [v3] kpageflags: respect folio head-page flag placement | expand

Commit Message

Svetly Todorov March 20, 2024, 5:28 p.m. UTC
---
Page flags are now stored per-folio. Change kpageflags to report these
per-folio flags where appropriate.

Most folio flags are set on the head page, i.e. *folio_flag(page, 0).

Only three are set on the second page -- PG_has_hwpoisoned, PG_large_rmappable,
and PG_hugetlb. kpageflags doesn't report the first two, and the PG_hugetlb
check is handled by calls to PageHuge/PageTransCompound(), so no code changes
are required for these flags.

mm/memory-failure.c applies PG_HWPoison via SetPageHWPoison(page).
This call falls through to the SetPage##uname macro in
include/linux/page-flags.h, which sets the corresponding PG_##lname bit
on the given page struct. Therefore KPF_HWPOISON must come from page->flags.

arch/xtensa/mm/cache.c tests and sets the PG_arch1 bit in both
folio->flags and page->flags. So both sources are OR'd into KPF_ARCH.

As for arch2 and arch3, Kefeng Wang reported that they appear only as
arm64-specific aliases: namely, PG_mte_tagged and PG_mte_lock. 
Both are applied to page->flags via set_bit() in arch/arm64/include/asm/mte.h.
So those must come from page->flags, too.

Signed-off-by: Svetly Todorov <svetly.todorov@memverge.com>
Suggested-by: Matthew Wilcox <willy@infradead.org>
Suggested-by: Gregory Price <gregory.price@memverge.com>
Suggested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Link: https://lore.kernel.org/all/20231030180005.2046-1-gregory.price@memverge.com/
Link: https://80x24.org/lore/linux-mm/20231110033324.2455523-4-wangkefeng.wang@huawei.com/
Link: https://80x24.org/lore/linux-mm/438ba640-c205-4034-886e-6a7231f3d210@huawei.com/
---
 fs/proc/page.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)


---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240319-kpageflags-svetly-9faa11b17022

Best regards,

Comments

Matthew Wilcox March 20, 2024, 7:24 p.m. UTC | #1
On Wed, Mar 20, 2024 at 10:28:09AM -0700, Svetly Todorov wrote:
> Page flags are now stored per-folio. Change kpageflags to report these
> per-folio flags where appropriate.

I have a somewhat different patch for this.  Let me know what you think.
It depends on a few other patches in my tree, so probably won't compile
for you.

From 42b20425c8b9b94f1273a410462babd4363622df Mon Sep 17 00:00:00 2001
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Date: Tue, 5 Mar 2024 05:38:33 +0000
Subject: [PATCH] proc: Rewrite stable_page_flags()

Reduce the usage of PageFlag tests and reduce the number of
compound_head() calls.  We also no longer need to check PageSlab before
checking page_mapped() as slub does not reuse the mapcount field.

For multi-page folios, we'll now show all pages as having the flags,
eg if it's locked, all pages will have the locked bit set instead of
just the head page.  The mapped bit is still per page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/proc/page.c             | 63 ++++++++++++++++++--------------------
 include/linux/page-flags.h |  2 +-
 2 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 195b077c0fac..0f9ef5866c0d 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -107,10 +107,13 @@ static inline u64 kpf_copy_bit(u64 kflags, int ubit, int kbit)
 	return ((kflags >> kbit) & 1) << ubit;
 }
 
-u64 stable_page_flags(struct page *page)
+u64 stable_page_flags(const struct page *page)
 {
-	u64 k;
-	u64 u;
+	const struct folio *folio;
+	unsigned long k;
+	unsigned long mapping;
+	bool is_anon;
+	u64 u = 0;
 
 	/*
 	 * pseudo flag: KPF_NOPAGE
@@ -118,52 +121,46 @@ u64 stable_page_flags(struct page *page)
 	 */
 	if (!page)
 		return 1 << KPF_NOPAGE;
+	folio = page_folio(page);
 
-	k = page->flags;
-	u = 0;
+	k = folio->flags;
+	mapping = (unsigned long)folio->mapping;
+	is_anon = mapping & PAGE_MAPPING_ANON;
 
 	/*
 	 * pseudo flags for the well known (anonymous) memory mapped pages
-	 *
-	 * Note that page->_mapcount is overloaded in SLAB, so the
-	 * simple test in page_mapped() is not enough.
 	 */
-	if (!PageSlab(page) && page_mapped(page))
+	if (page_mapped(page))
 		u |= 1 << KPF_MMAP;
-	if (PageAnon(page))
+	if (is_anon)
 		u |= 1 << KPF_ANON;
-	if (PageKsm(page))
+	if (mapping & PAGE_MAPPING_KSM)
 		u |= 1 << KPF_KSM;
 
 	/*
 	 * compound pages: export both head/tail info
 	 * they together define a compound page's start/end pos and order
 	 */
-	if (PageHead(page))
-		u |= 1 << KPF_COMPOUND_HEAD;
-	if (PageTail(page))
+	if (page == &folio->page)
+		u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
+	else
 		u |= 1 << KPF_COMPOUND_TAIL;
-	if (PageHuge(page))
+	if (folio_test_hugetlb(folio))
 		u |= 1 << KPF_HUGE;
 	/*
-	 * PageTransCompound can be true for non-huge compound pages (slab
-	 * pages or pages allocated by drivers with __GFP_COMP) because it
-	 * just checks PG_head/PG_tail, so we need to check PageLRU/PageAnon
+	 * We need to check PageLRU/PageAnon
 	 * to make sure a given page is a thp, not a non-huge compound page.
 	 */
-	else if (PageTransCompound(page)) {
-		struct page *head = compound_head(page);
-
-		if (PageLRU(head) || PageAnon(head))
+	else if (folio_test_large(folio)) {
+		if ((k & PG_lru) || is_anon)
 			u |= 1 << KPF_THP;
-		else if (is_huge_zero_page(head)) {
+		else if (is_huge_zero_folio(folio)) {
 			u |= 1 << KPF_ZERO_PAGE;
 			u |= 1 << KPF_THP;
 		}
 	} else if (is_zero_pfn(page_to_pfn(page)))
 		u |= 1 << KPF_ZERO_PAGE;
 
-
 	/*
 	 * Caveats on high order pages: PG_buddy and PG_slab will only be set
 	 * on the head page.
@@ -178,15 +175,15 @@ u64 stable_page_flags(struct page *page)
 	if (PageTable(page))
 		u |= 1 << KPF_PGTABLE;
 
-	if (page_is_idle(page))
+#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
+	u |= kpf_copy_bit(k, KPF_IDLE,          PG_idle);
+#else
+	if (folio_test_idle(folio))
 		u |= 1 << KPF_IDLE;
+#endif
 
 	u |= kpf_copy_bit(k, KPF_LOCKED,	PG_locked);
-
 	u |= kpf_copy_bit(k, KPF_SLAB,		PG_slab);
-	if (PageTail(page) && PageSlab(page))
-		u |= 1 << KPF_SLAB;
-
 	u |= kpf_copy_bit(k, KPF_ERROR,		PG_error);
 	u |= kpf_copy_bit(k, KPF_DIRTY,		PG_dirty);
 	u |= kpf_copy_bit(k, KPF_UPTODATE,	PG_uptodate);
@@ -197,7 +194,8 @@ u64 stable_page_flags(struct page *page)
 	u |= kpf_copy_bit(k, KPF_ACTIVE,	PG_active);
 	u |= kpf_copy_bit(k, KPF_RECLAIM,	PG_reclaim);
 
-	if (PageSwapCache(page))
+#define SWAPCACHE ((1 << PG_swapbacked) | (1 << PG_swapcache))
+	if ((k & SWAPCACHE) == SWAPCACHE)
 		u |= 1 << KPF_SWAPCACHE;
 	u |= kpf_copy_bit(k, KPF_SWAPBACKED,	PG_swapbacked);
 
@@ -231,7 +229,6 @@ static ssize_t kpageflags_read(struct file *file, char __user *buf,
 {
 	const unsigned long max_dump_pfn = get_max_dump_pfn();
 	u64 __user *out = (u64 __user *)buf;
-	struct page *ppage;
 	unsigned long src = *ppos;
 	unsigned long pfn;
 	ssize_t ret = 0;
@@ -248,9 +245,9 @@ static ssize_t kpageflags_read(struct file *file, char __user *buf,
 		 * TODO: ZONE_DEVICE support requires to identify
 		 * memmaps that were actually initialized.
 		 */
-		ppage = pfn_to_online_page(pfn);
+		struct page *page = pfn_to_online_page(pfn);
 
-		if (put_user(stable_page_flags(ppage), out)) {
+		if (put_user(stable_page_flags(page), out)) {
 			ret = -EFAULT;
 			break;
 		}
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 068c9bd43ebf..92a64faa851c 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -716,7 +716,7 @@ static __always_inline bool PageKsm(const struct page *page)
 TESTPAGEFLAG_FALSE(Ksm, ksm)
 #endif
 
-u64 stable_page_flags(struct page *page);
+u64 stable_page_flags(const struct page *page);
 
 /**
  * folio_xor_flags_has_waiters - Change some folio flags.
Svetly Todorov March 20, 2024, 11:40 p.m. UTC | #2
Hi Matthew,

> I have a somewhat different patch for this.  Let me know what you think.
> It depends on a few other patches in my tree, so probably won't compile
> for you.
I don't have extensive experience with folios or anything but on the
whole it looks good to me. I like the use of `mapping` to dodge the
compound_head() checks. Beyond that, only a few things caught my eye.

> -	if (PageKsm(page))
> +	if (mapping & PAGE_MAPPING_KSM)
>   		u |= 1 << KPF_KSM;
This might need an #ifdef?
Say mapping is movable and anon -- then (mapping & PAGE_MAPPING_KSM) is
true. Before, we called PageKsm, which falls through to a PG_ksm check.
If !CONFIG_KSM then that flag is always false. But now, we're liable to
report KPF_KSM even if !CONFIG_KSM.

>   	/*
>   	 * compound pages: export both head/tail info
>   	 * they together define a compound page's start/end pos and order
>   	 */
> -	if (PageHead(page))
> -		u |= 1 << KPF_COMPOUND_HEAD;
> -	if (PageTail(page))
> +	if (page == &folio->page)
> +		u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
> +	else
>   		u |= 1 << KPF_COMPOUND_TAIL;
This makes sense but it'd require changes to the documentation.
I ran a python3 memhog to see if anonymous pages are currently reported
as COMPOUND_HEAD or COMPOUND_TAIL and it seems to be a no on both.
But with this, I think every pfn will have one of the two set.
Unless you can have a page outside of a folio -- not sure.

Also, in
> -	if (page_is_idle(page))
> +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
> +	u |= kpf_copy_bit(k, KPF_IDLE,          PG_idle);
> +#else
> +	if (folio_test_idle(folio))
>   		u |= 1 << KPF_IDLE;
> +#endif
> 
and
> -	if (PageSwapCache(page))
> +#define SWAPCACHE ((1 << PG_swapbacked) | (1 << PG_swapcache))
> +	if ((k & SWAPCACHE) == SWAPCACHE)
>   		u |= 1 << KPF_SWAPCACHE;
>   	u |= kpf_copy_bit(k, KPF_SWAPBACKED,	PG_swapbacked);
it seems to me like the #ifdef/#define could be supplanted by
folio_test_idle and folio_test_swapcache. But I guess those would
require extra folio_flags queries and an #include <page_idle.h>.
So if this is more performant, I can understand the design.

Best,
Svetly
Matthew Wilcox March 21, 2024, 4:03 p.m. UTC | #3
On Wed, Mar 20, 2024 at 04:40:43PM -0700, Svetly Todorov wrote:
> 
> Hi Matthew,
> 
> > I have a somewhat different patch for this.  Let me know what you think.
> > It depends on a few other patches in my tree, so probably won't compile
> > for you.
> I don't have extensive experience with folios or anything but on the
> whole it looks good to me. I like the use of `mapping` to dodge the
> compound_head() checks. Beyond that, only a few things caught my eye.

Thanks for your careful review.

> > -	if (PageKsm(page))
> > +	if (mapping & PAGE_MAPPING_KSM)
> >   		u |= 1 << KPF_KSM;
> This might need an #ifdef?
> Say mapping is movable and anon -- then (mapping & PAGE_MAPPING_KSM) is
> true. Before, we called PageKsm, which falls through to a PG_ksm check.
> If !CONFIG_KSM then that flag is always false. But now, we're liable to
> report KPF_KSM even if !CONFIG_KSM.

I'm not sure where you see a PG_ksm check:

static __always_inline bool folio_test_ksm(const struct folio *folio)
{
        return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) ==
                                PAGE_MAPPING_KSM;
}

static __always_inline bool PageKsm(const struct page *page)
{
        return folio_test_ksm(page_folio(page));
}

There's no such thing as a movable anon page -- the two bits in the
bottom of the mapping pointer mean:

00	file (or NULL)
01	anon
10	movable
11	KSM

Perhaps it might be clearer to say that anon pages are inherently
movable; the movable type really means that the reset of the mapping
pointer refers to a movable_operations instead of a mapping or anon_vma.

> >   	/*
> >   	 * compound pages: export both head/tail info
> >   	 * they together define a compound page's start/end pos and order
> >   	 */
> > -	if (PageHead(page))
> > -		u |= 1 << KPF_COMPOUND_HEAD;
> > -	if (PageTail(page))
> > +	if (page == &folio->page)
> > +		u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
> > +	else
> >   		u |= 1 << KPF_COMPOUND_TAIL;
> This makes sense but it'd require changes to the documentation.
> I ran a python3 memhog to see if anonymous pages are currently reported
> as COMPOUND_HEAD or COMPOUND_TAIL and it seems to be a no on both.
> But with this, I think every pfn will have one of the two set.
> Unless you can have a page outside of a folio -- not sure.

I see your confusion.  We have three cases; head, tail and neither
(obviously a page is never both head & tail).  If a page is neither,
it's order-0 and it is the only page in the folio.  So we handle head
or neither in the first leg of the 'if' where we set KPF_COMPOUND_HEAD
if PG_head is set, and tail in the 'else' leg.

> Also, in
> > -	if (page_is_idle(page))
> > +#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
> > +	u |= kpf_copy_bit(k, KPF_IDLE,          PG_idle);
> > +#else
> > +	if (folio_test_idle(folio))
> >   		u |= 1 << KPF_IDLE;
> > +#endif
> > 
> and
> > -	if (PageSwapCache(page))
> > +#define SWAPCACHE ((1 << PG_swapbacked) | (1 << PG_swapcache))
> > +	if ((k & SWAPCACHE) == SWAPCACHE)
> >   		u |= 1 << KPF_SWAPCACHE;
> >   	u |= kpf_copy_bit(k, KPF_SWAPBACKED,	PG_swapbacked);
> it seems to me like the #ifdef/#define could be supplanted by
> folio_test_idle and folio_test_swapcache. But I guess those would
> require extra folio_flags queries and an #include <page_idle.h>.
> So if this is more performant, I can understand the design.

It's not so much the performance as it is the atomicity.  I'm doing my
best to get an atomic snapshot of the flags and report a consistent
state, even if it might be stale by the time the user sees it.
Svetly Todorov March 21, 2024, 7:08 p.m. UTC | #4
> Thanks for your careful review.
No problem!! It's a valuable learning experience for me.

>>> -	if (PageKsm(page))
>>> +	if (mapping & PAGE_MAPPING_KSM)
>>>    		u |= 1 << KPF_KSM;
>> This might need an #ifdef?
>> Say mapping is movable and anon -- then (mapping & PAGE_MAPPING_KSM) is
>> true. Before, we called PageKsm, which falls through to a PG_ksm check.
>> If !CONFIG_KSM then that flag is always false. But now, we're liable to
>> report KPF_KSM even if !CONFIG_KSM.
> 
> I'm not sure where you see a PG_ksm check:
> 
> static __always_inline bool folio_test_ksm(const struct folio *folio)
> {
>          return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) ==
>                                  PAGE_MAPPING_KSM;
> }
> 
> static __always_inline bool PageKsm(const struct page *page)
> {
>          return folio_test_ksm(page_folio(page));
> }
My bad. What I meant was, if CONFIG_KSM is undefined, then

 > #ifdef CONFIG_KSM
 > ...
 > static __always_inline bool PageKsm(struct page *page)
 > {
 > 	return folio_test_ksm(page_folio(page));
 > }

will fall through to

 > # else
 > TESTPAGEFLAG_FALSE(Ksm, ksm)
 > #endif

And you're right -- there is no PG_ksm comparison --
but the autogenerated PageKsm will always return false:

 > #define TESTPAGEFLAG_FALSE(uname, lname) \
 > ...
 > static inline int Page##uname(const struct page *page)
 > {
 > 	return 0;
 > }

But given your comments below, I'm realizing this isn't as important
as I thought it was.

> There's no such thing as a movable anon page -- the two bits in the
> bottom of the mapping pointer mean:
> 
> 00	file (or NULL)
> 01	anon
> 10	movable
> 11	KSM
> 
> Perhaps it might be clearer to say that anon pages are inherently
> movable; the movable type really means that the reset of the mapping
> pointer refers to a movable_operations instead of a mapping or anon_vma.
I see. I misunderstood how the flags are applied.
I thought that 11 == (01 | 10) -- i.e. that KSM was an intersection of
MOVABLE and ANON. But they're more like mutually-exclusive states. And
I doubt that a page will end up in the KSM "state" if CONFIG_KSM is
disabled. So we don't need to rely on PageKsm() for the CONFIG_KSM
check.

That said, won't

	if (mapping & PAGE_MAPPING_KSM)

return true even if a mapping is ANON (01) or MOVABLE (10)
but not KSM (11)? Shouldn't this at least be

	if (mapping & PAGE_MAPPING_KSM == PAGE_MAPPING_KSM)

?

>>>    	/*
>>>    	 * compound pages: export both head/tail info
>>>    	 * they together define a compound page's start/end pos and order
>>>    	 */
>>> -	if (PageHead(page))
>>> -		u |= 1 << KPF_COMPOUND_HEAD;
>>> -	if (PageTail(page))
>>> +	if (page == &folio->page)
>>> +		u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
>>> +	else
>>>    		u |= 1 << KPF_COMPOUND_TAIL;
>> This makes sense but it'd require changes to the documentation.
>> I ran a python3 memhog to see if anonymous pages are currently reported
>> as COMPOUND_HEAD or COMPOUND_TAIL and it seems to be a no on both.
>> But with this, I think every pfn will have one of the two set.
>> Unless you can have a page outside of a folio -- not sure.
> 
> I see your confusion.  We have three cases; head, tail and neither
> (obviously a page is never both head & tail).  If a page is neither,
> it's order-0 and it is the only page in the folio.  So we handle head
> or neither in the first leg of the 'if' where we set KPF_COMPOUND_HEAD
> if PG_head is set, and tail in the 'else' leg.

Dumb mistake on my part. For some reason, I thought that every
folio->page had its PG_head set.

> It's not so much the performance as it is the atomicity.  I'm doing my
> best to get an atomic snapshot of the flags and report a consistent
> state, even if it might be stale by the time the user sees it.
I see. That makes sense.

Cool! Thanks for bearing with me. Beyond the KSM stuff, my only
hangup is that this patch doesn't account for the handful of
remaining per-page flags (KPF_HWPOISON, KPF_ARCH_*). Should I
take this diff, tack those on in a second commit, and then put
up a v4? Forgive me, I'm very green to the kernel dev process...
Matthew Wilcox March 21, 2024, 7:59 p.m. UTC | #5
On Thu, Mar 21, 2024 at 12:08:01PM -0700, Svetly Todorov wrote:
> > > > -	if (PageKsm(page))
> > > > +	if (mapping & PAGE_MAPPING_KSM)
> > > >    		u |= 1 << KPF_KSM;
> > > This might need an #ifdef?
> > > Say mapping is movable and anon -- then (mapping & PAGE_MAPPING_KSM) is
> > > true. Before, we called PageKsm, which falls through to a PG_ksm check.
> > > If !CONFIG_KSM then that flag is always false. But now, we're liable to
> > > report KPF_KSM even if !CONFIG_KSM.
> > 
> > I'm not sure where you see a PG_ksm check:
> > 
> > static __always_inline bool folio_test_ksm(const struct folio *folio)
> > {
> >          return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) ==
> >                                  PAGE_MAPPING_KSM;
> > }
> > 
> > static __always_inline bool PageKsm(const struct page *page)
> > {
> >          return folio_test_ksm(page_folio(page));
> > }
> My bad. What I meant was, if CONFIG_KSM is undefined, then
> 
> > #ifdef CONFIG_KSM
> > ...
> > static __always_inline bool PageKsm(struct page *page)
> > {
> > 	return folio_test_ksm(page_folio(page));
> > }
> 
> will fall through to
> 
> > # else
> > TESTPAGEFLAG_FALSE(Ksm, ksm)
> > #endif
> 
> And you're right -- there is no PG_ksm comparison --
> but the autogenerated PageKsm will always return false:

Yes, that's true.  Usually we care about this because we can optimise
out large chunks of code if a config option (eg CONFIG_KSM) is disabled.
In this case, we're talking about a couple of instructions, and it's
generally not worth optimising those out in order to add an ifdef in
the code.  We've got quite a long way with Linux without it becoming
overrun with ifdefs (compare, eg, the Mach source code), and long may
that continue ;-)

> > 00	file (or NULL)
> > 01	anon
> > 10	movable
> > 11	KSM
> > 
> > Perhaps it might be clearer to say that anon pages are inherently
> > movable; the movable type really means that the reset of the mapping
> > pointer refers to a movable_operations instead of a mapping or anon_vma.
> I see. I misunderstood how the flags are applied.
> I thought that 11 == (01 | 10) -- i.e. that KSM was an intersection of
> MOVABLE and ANON. But they're more like mutually-exclusive states. And
> I doubt that a page will end up in the KSM "state" if CONFIG_KSM is
> disabled. So we don't need to rely on PageKsm() for the CONFIG_KSM
> check.
> 
> That said, won't
> 
> 	if (mapping & PAGE_MAPPING_KSM)
> 
> return true even if a mapping is ANON (01) or MOVABLE (10)
> but not KSM (11)? Shouldn't this at least be
> 
> 	if (mapping & PAGE_MAPPING_KSM == PAGE_MAPPING_KSM)

Uh, yeah, that was a mistake.  This should do the trick:

        if (is_anon) {
                u |= 1 << KPF_ANON;
                if (mapping & PAGE_MAPPING_KSM)
                        u |= 1 << KPF_KSM;
        }

(all KSM pages are reported as anon pages as well, both before and after
this patch; see how folio_test_anon() only checks the bottom bit)

> > I see your confusion.  We have three cases; head, tail and neither
> > (obviously a page is never both head & tail).  If a page is neither,
> > it's order-0 and it is the only page in the folio.  So we handle head
> > or neither in the first leg of the 'if' where we set KPF_COMPOUND_HEAD
> > if PG_head is set, and tail in the 'else' leg.
> 
> Dumb mistake on my part. For some reason, I thought that every
> folio->page had its PG_head set.

At this point, it's bad naming, but it's not worth the churn of fixing
it; we have a better destination in mind, and we'll get there soon enough.

> Cool! Thanks for bearing with me. Beyond the KSM stuff, my only
> hangup is that this patch doesn't account for the handful of
> remaining per-page flags (KPF_HWPOISON, KPF_ARCH_*). Should I
> take this diff, tack those on in a second commit, and then put
> up a v4? Forgive me, I'm very green to the kernel dev process...

Oh, yes, that's a bug on my part.  HWPOISON is definitely per-page,
not per-folio (although the handling of it differs for hugetlb)
and I haven't looked at the PG_arch gunk yet.  We are trying to
sliminate the per-page flags, because there's no space for them in the
future (we'll have special handling for hwpoison because that really is
very special)
diff mbox series

Patch

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 195b077c0fac..290470819fa2 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -110,6 +110,7 @@  static inline u64 kpf_copy_bit(u64 kflags, int ubit, int kbit)
 u64 stable_page_flags(struct page *page)
 {
 	u64 k;
+	u64 p;
 	u64 u;
 
 	/*
@@ -119,7 +120,8 @@  u64 stable_page_flags(struct page *page)
 	if (!page)
 		return 1 << KPF_NOPAGE;
 
-	k = page->flags;
+	k = *folio_flags(page, 0);
+	p = page->flags;
 	u = 0;
 
 	/*
@@ -205,7 +207,7 @@  u64 stable_page_flags(struct page *page)
 	u |= kpf_copy_bit(k, KPF_MLOCKED,	PG_mlocked);
 
 #ifdef CONFIG_MEMORY_FAILURE
-	u |= kpf_copy_bit(k, KPF_HWPOISON,	PG_hwpoison);
+	u |= kpf_copy_bit(p, KPF_HWPOISON,	PG_hwpoison);
 #endif
 
 #ifdef CONFIG_ARCH_USES_PG_UNCACHED
@@ -216,11 +218,14 @@  u64 stable_page_flags(struct page *page)
 	u |= kpf_copy_bit(k, KPF_MAPPEDTODISK,	PG_mappedtodisk);
 	u |= kpf_copy_bit(k, KPF_PRIVATE,	PG_private);
 	u |= kpf_copy_bit(k, KPF_PRIVATE_2,	PG_private_2);
+	u |= kpf_copy_bit(p, KPF_PRIVATE,	PG_private);
+	u |= kpf_copy_bit(p, KPF_PRIVATE_2,	PG_private_2);
 	u |= kpf_copy_bit(k, KPF_OWNER_PRIVATE,	PG_owner_priv_1);
 	u |= kpf_copy_bit(k, KPF_ARCH,		PG_arch_1);
+	u |= kpf_copy_bit(p, KPF_ARCH,		PG_arch_1);
 #ifdef CONFIG_ARCH_USES_PG_ARCH_X
-	u |= kpf_copy_bit(k, KPF_ARCH_2,	PG_arch_2);
-	u |= kpf_copy_bit(k, KPF_ARCH_3,	PG_arch_3);
+	u |= kpf_copy_bit(p, KPF_ARCH_2,	PG_arch_2);
+	u |= kpf_copy_bit(p, KPF_ARCH_3,	PG_arch_3);
 #endif
 
 	return u;