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

[PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Gregory Price 2 years, 2 months ago

kpageflags reads page-flags directly from the page, even when the
respective flag is only updated on the headpage of a folio.

Update bitchecks to use PAGEFLAG() interfaces to check folio for the
referenced, dirty, lru, active, and unevictable bits.

Signed-off-by: Gregory Price <gregory.price@memverge.com>
---
 fs/proc/page.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 195b077c0fac..1dceecb8018a 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -111,6 +111,7 @@ u64 stable_page_flags(struct page *page)
 {
 	u64 k;
 	u64 u;
+	struct folio *folio;
 
 	/*
 	 * pseudo flag: KPF_NOPAGE
@@ -119,6 +120,8 @@ u64 stable_page_flags(struct page *page)
 	if (!page)
 		return 1 << KPF_NOPAGE;
 
+	folio = page_folio(page);
+
 	k = page->flags;
 	u = 0;
 
@@ -188,20 +191,31 @@ u64 stable_page_flags(struct page *page)
 		u |= 1 << KPF_SLAB;
 
 	u |= kpf_copy_bit(k, KPF_ERROR,		PG_error);
-	u |= kpf_copy_bit(k, KPF_DIRTY,		PG_dirty);
+
+	if (folio_test_dirty(folio))
+		u |= 1 << KPF_DIRTY;
+
 	u |= kpf_copy_bit(k, KPF_UPTODATE,	PG_uptodate);
 	u |= kpf_copy_bit(k, KPF_WRITEBACK,	PG_writeback);
 
-	u |= kpf_copy_bit(k, KPF_LRU,		PG_lru);
-	u |= kpf_copy_bit(k, KPF_REFERENCED,	PG_referenced);
-	u |= kpf_copy_bit(k, KPF_ACTIVE,	PG_active);
+	if (folio_test_lru(folio))
+		u |= 1 << KPF_LRU;
+
+	if (folio_test_referenced(folio))
+		u |= 1 << KPF_REFERENCED;
+
+	if (folio_test_active(folio))
+		u |= 1 << KPF_ACTIVE;
+
 	u |= kpf_copy_bit(k, KPF_RECLAIM,	PG_reclaim);
 
 	if (PageSwapCache(page))
 		u |= 1 << KPF_SWAPCACHE;
 	u |= kpf_copy_bit(k, KPF_SWAPBACKED,	PG_swapbacked);
 
-	u |= kpf_copy_bit(k, KPF_UNEVICTABLE,	PG_unevictable);
+	if (folio_test_unevictable(folio))
+		u |= 1 << KPF_UNEVICTABLE;
+
 	u |= kpf_copy_bit(k, KPF_MLOCKED,	PG_mlocked);
 
 #ifdef CONFIG_MEMORY_FAILURE
-- 
2.39.1

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Matthew Wilcox 2 years, 2 months ago

On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
> kpageflags reads page-flags directly from the page, even when the
> respective flag is only updated on the headpage of a folio.
> 
> Update bitchecks to use PAGEFLAG() interfaces to check folio for the
> referenced, dirty, lru, active, and unevictable bits.

But uptodate, writeback and reclaim (amongst others) are also defined
only on the head page.

>  	u |= kpf_copy_bit(k, KPF_UPTODATE,	PG_uptodate);
>  	u |= kpf_copy_bit(k, KPF_WRITEBACK,	PG_writeback);
>  
>  	u |= kpf_copy_bit(k, KPF_RECLAIM,	PG_reclaim);
>  
>  	if (PageSwapCache(page))
>  		u |= 1 << KPF_SWAPCACHE;
>  	u |= kpf_copy_bit(k, KPF_SWAPBACKED,	PG_swapbacked);
>  
>  	u |= kpf_copy_bit(k, KPF_MLOCKED,	PG_mlocked);

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Gregory Price 2 years, 2 months ago

On Mon, Oct 30, 2023 at 11:22:18PM +0000, Matthew Wilcox wrote:
> On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
> > kpageflags reads page-flags directly from the page, even when the
> > respective flag is only updated on the headpage of a folio.
> > 
> > Update bitchecks to use PAGEFLAG() interfaces to check folio for the
> > referenced, dirty, lru, active, and unevictable bits.
> 
> But uptodate, writeback and reclaim (amongst others) are also defined
> only on the head page.
> 

Ah yes i was only looking at the things defined w/ PAGEFLAG defines in
page-flags.h. I'll give it full once over can collect them all, my bad.

(also i forgot to update my commit message)

Quick question here since i have your attention: any recommendation on
what to do for ONLY_HEAD flags?  If the provided page is not the head,
should the flag report 0... or whatever the head says?

> >  	u |= kpf_copy_bit(k, KPF_UPTODATE,	PG_uptodate);
> >  	u |= kpf_copy_bit(k, KPF_WRITEBACK,	PG_writeback);
> >  
> >  	u |= kpf_copy_bit(k, KPF_RECLAIM,	PG_reclaim);
> >  
> >  	if (PageSwapCache(page))
> >  		u |= 1 << KPF_SWAPCACHE;
> >  	u |= kpf_copy_bit(k, KPF_SWAPBACKED,	PG_swapbacked);
> >  
> >  	u |= kpf_copy_bit(k, KPF_MLOCKED,	PG_mlocked);
>

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Matthew Wilcox 2 years, 2 months ago

On Mon, Oct 30, 2023 at 07:41:23PM -0400, Gregory Price wrote:
> On Mon, Oct 30, 2023 at 11:22:18PM +0000, Matthew Wilcox wrote:
> > On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
> > > kpageflags reads page-flags directly from the page, even when the
> > > respective flag is only updated on the headpage of a folio.
> > > 
> > > Update bitchecks to use PAGEFLAG() interfaces to check folio for the
> > > referenced, dirty, lru, active, and unevictable bits.
> > 
> > But uptodate, writeback and reclaim (amongst others) are also defined
> > only on the head page.
> > 
> 
> Ah yes i was only looking at the things defined w/ PAGEFLAG defines in
> page-flags.h. I'll give it full once over can collect them all, my bad.
> 
> (also i forgot to update my commit message)
> 
> Quick question here since i have your attention: any recommendation on
> what to do for ONLY_HEAD flags?  If the provided page is not the head,
> should the flag report 0... or whatever the head says?

Thinking about it some more, really almost all flags are per-folio, not
per-page.  The only exceptions are HWPoison and AnonExclusive.  So
probably the right way to do this is to make k = folio->flags, and
then just change a few places rather than changing all the places that
test 'k'.

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Gregory Price 2 years, 2 months ago

On Tue, Oct 31, 2023 at 09:13:44AM +0000, Matthew Wilcox wrote:
> On Mon, Oct 30, 2023 at 07:41:23PM -0400, Gregory Price wrote:
> > On Mon, Oct 30, 2023 at 11:22:18PM +0000, Matthew Wilcox wrote:
> > > On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
> > > > kpageflags reads page-flags directly from the page, even when the
> > > > respective flag is only updated on the headpage of a folio.
> > > > 
> > > > Update bitchecks to use PAGEFLAG() interfaces to check folio for the
> > > > referenced, dirty, lru, active, and unevictable bits.
> > > 
> > > But uptodate, writeback and reclaim (amongst others) are also defined
> > > only on the head page.
> > > 
> > 
> > Ah yes i was only looking at the things defined w/ PAGEFLAG defines in
> > page-flags.h. I'll give it full once over can collect them all, my bad.
> > 
> > (also i forgot to update my commit message)
> > 
> > Quick question here since i have your attention: any recommendation on
> > what to do for ONLY_HEAD flags?  If the provided page is not the head,
> > should the flag report 0... or whatever the head says?
> 
> Thinking about it some more, really almost all flags are per-folio, not
> per-page.  The only exceptions are HWPoison and AnonExclusive.  So
> probably the right way to do this is to make k = folio->flags, and
> then just change a few places rather than changing all the places that
> test 'k'.

Funny enough that's what i originally did but was confident it was
correct so walked it back.  I'll take another crack at it.

~Gregory

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Kefeng Wang 2 years, 1 month ago


On 2023/10/31 12:34, Gregory Price wrote:
> On Tue, Oct 31, 2023 at 09:13:44AM +0000, Matthew Wilcox wrote:
>> On Mon, Oct 30, 2023 at 07:41:23PM -0400, Gregory Price wrote:
>>> On Mon, Oct 30, 2023 at 11:22:18PM +0000, Matthew Wilcox wrote:
>>>> On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
>>>>> kpageflags reads page-flags directly from the page, even when the
>>>>> respective flag is only updated on the headpage of a folio.
>>>>>
>>>>> Update bitchecks to use PAGEFLAG() interfaces to check folio for the
>>>>> referenced, dirty, lru, active, and unevictable bits.
>>>>
>>>> But uptodate, writeback and reclaim (amongst others) are also defined
>>>> only on the head page.
>>>>
>>>
>>> Ah yes i was only looking at the things defined w/ PAGEFLAG defines in
>>> page-flags.h. I'll give it full once over can collect them all, my bad.
>>>
>>> (also i forgot to update my commit message)
>>>
>>> Quick question here since i have your attention: any recommendation on
>>> what to do for ONLY_HEAD flags?  If the provided page is not the head,
>>> should the flag report 0... or whatever the head says?
>>
>> Thinking about it some more, really almost all flags are per-folio, not
>> per-page.  The only exceptions are HWPoison and AnonExclusive.  So
>> probably the right way to do this is to make k = folio->flags, and
>> then just change a few places rather than changing all the places that
>> test 'k'.
> 
> Funny enough that's what i originally did but was confident it was
> correct so walked it back.  I'll take another crack at it.

Hi Gregory, any update?
I changed stable_page_flags[1] when try to remove page idle wrapper,
Matthew pointed it will conflict with this, I could redo my patch
based on your new version:)

[1]https://lore.kernel.org/linux-mm/20231103072906.2000381-5-wangkefeng.wang@huawei.com/

Thanks.
> 
> ~Gregory
>

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Gregory Price 2 years, 1 month ago

On Tue, Nov 07, 2023 at 09:03:53AM +0800, Kefeng Wang wrote:
> 
> 
> On 2023/10/31 12:34, Gregory Price wrote:
> > On Tue, Oct 31, 2023 at 09:13:44AM +0000, Matthew Wilcox wrote:
> > > On Mon, Oct 30, 2023 at 07:41:23PM -0400, Gregory Price wrote:
> > > > On Mon, Oct 30, 2023 at 11:22:18PM +0000, Matthew Wilcox wrote:
> > > > > On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
> > > > > > kpageflags reads page-flags directly from the page, even when the
> > > > > > respective flag is only updated on the headpage of a folio.
> > > > > > 
> > > > > > Update bitchecks to use PAGEFLAG() interfaces to check folio for the
> > > > > > referenced, dirty, lru, active, and unevictable bits.
> > > > > 
> > > > > But uptodate, writeback and reclaim (amongst others) are also defined
> > > > > only on the head page.
> > > > > 
> > > > 
> > > > Ah yes i was only looking at the things defined w/ PAGEFLAG defines in
> > > > page-flags.h. I'll give it full once over can collect them all, my bad.
> > > > 
> > > > (also i forgot to update my commit message)
> > > > 
> > > > Quick question here since i have your attention: any recommendation on
> > > > what to do for ONLY_HEAD flags?  If the provided page is not the head,
> > > > should the flag report 0... or whatever the head says?
> > > 
> > > Thinking about it some more, really almost all flags are per-folio, not
> > > per-page.  The only exceptions are HWPoison and AnonExclusive.  So
> > > probably the right way to do this is to make k = folio->flags, and
> > > then just change a few places rather than changing all the places that
> > > test 'k'.
> > 
> > Funny enough that's what i originally did but was confident it was
> > correct so walked it back.  I'll take another crack at it.
> 
> Hi Gregory, any update?
> I changed stable_page_flags[1] when try to remove page idle wrapper,
> Matthew pointed it will conflict with this, I could redo my patch
> based on your new version:)
> 
> [1]https://lore.kernel.org/linux-mm/20231103072906.2000381-5-wangkefeng.wang@huawei.com/
> 
> Thanks.

looks like we were noticing the same thing.  I haven't done any further
work, got caught up in another project.

Matthew last pointed out:

"probably the right way to do this is to make k = folio->flags, and then
just change a few places rather than changing all the places that test
'k'."

https://lore.kernel.org/linux-mm/ZUDFSEvpxxoGWmdG@casper.infradead.org/

I took a quick look, and the only thing I'm not confident about is that
some flags are stored in the head page, and some are stored on the
second page.

/* Which page is the flag stored in */
#define FOLIO_PF_ANY            0
#define FOLIO_PF_HEAD           0
#define FOLIO_PF_ONLY_HEAD      0
#define FOLIO_PF_NO_TAIL        0
#define FOLIO_PF_NO_COMPOUND    0
#define FOLIO_PF_SECOND         1

There's only a handful, so yeah the best way is probably to go ahead and
swap k = page->flags for k = *folio_flags(folio, 0) and then handle the
couple of outliars.

~Gregory

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Kefeng Wang 2 years, 1 month ago


On 2023/11/7 23:34, Gregory Price wrote:
> On Tue, Nov 07, 2023 at 09:03:53AM +0800, Kefeng Wang wrote:
>>
>>
>> On 2023/10/31 12:34, Gregory Price wrote:
>>> On Tue, Oct 31, 2023 at 09:13:44AM +0000, Matthew Wilcox wrote:
>>>> On Mon, Oct 30, 2023 at 07:41:23PM -0400, Gregory Price wrote:
>>>>> On Mon, Oct 30, 2023 at 11:22:18PM +0000, Matthew Wilcox wrote:
>>>>>> On Mon, Oct 30, 2023 at 02:00:05PM -0400, Gregory Price wrote:
>>>>>>> kpageflags reads page-flags directly from the page, even when the
>>>>>>> respective flag is only updated on the headpage of a folio.
>>>>>>>
>>>>>>> Update bitchecks to use PAGEFLAG() interfaces to check folio for the
>>>>>>> referenced, dirty, lru, active, and unevictable bits.
>>>>>>
>>>>>> But uptodate, writeback and reclaim (amongst others) are also defined
>>>>>> only on the head page.
>>>>>>
>>>>>
>>>>> Ah yes i was only looking at the things defined w/ PAGEFLAG defines in
>>>>> page-flags.h. I'll give it full once over can collect them all, my bad.
>>>>>
>>>>> (also i forgot to update my commit message)
>>>>>
>>>>> Quick question here since i have your attention: any recommendation on
>>>>> what to do for ONLY_HEAD flags?  If the provided page is not the head,
>>>>> should the flag report 0... or whatever the head says?
>>>>
>>>> Thinking about it some more, really almost all flags are per-folio, not
>>>> per-page.  The only exceptions are HWPoison and AnonExclusive.  So
>>>> probably the right way to do this is to make k = folio->flags, and
>>>> then just change a few places rather than changing all the places that
>>>> test 'k'.
>>>
>>> Funny enough that's what i originally did but was confident it was
>>> correct so walked it back.  I'll take another crack at it.
>>
>> Hi Gregory, any update?
>> I changed stable_page_flags[1] when try to remove page idle wrapper,
>> Matthew pointed it will conflict with this, I could redo my patch
>> based on your new version:)
>>
>> [1]https://lore.kernel.org/linux-mm/20231103072906.2000381-5-wangkefeng.wang@huawei.com/
>>
>> Thanks.
> 
> looks like we were noticing the same thing.  I haven't done any further
> work, got caught up in another project.

Yes, I see your "Node Weights and Weighted Interleave", this is an 
interesting topic, we need some easy and efficient way to use tiered
memory.

> 
> Matthew last pointed out:
> 
> "probably the right way to do this is to make k = folio->flags, and then
> just change a few places rather than changing all the places that test
> 'k'."
> 
> https://lore.kernel.org/linux-mm/ZUDFSEvpxxoGWmdG@casper.infradead.org/
> 
> I took a quick look, and the only thing I'm not confident about is that
> some flags are stored in the head page, and some are stored on the
> second page.
> 
> /* Which page is the flag stored in */
> #define FOLIO_PF_ANY            0
> #define FOLIO_PF_HEAD           0
> #define FOLIO_PF_ONLY_HEAD      0
> #define FOLIO_PF_NO_TAIL        0
> #define FOLIO_PF_NO_COMPOUND    0
> #define FOLIO_PF_SECOND         1
> 
> There's only a handful, so yeah the best way is probably to go ahead and
> swap k = page->flags for k = *folio_flags(folio, 0) and then handle the
> couple of outliars.

If you don't mind, I maybe try to convert it in my changes.

Thanks.
> 
> ~Gregory

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Gregory Price 2 years, 1 month ago

On Wed, Nov 08, 2023 at 10:01:19AM +0800, Kefeng Wang wrote:
> 
> Yes, I see your "Node Weights and Weighted Interleave", this is an
> interesting topic, we need some easy and efficient way to use tiered
> memory.
> 

There will be an LPC talk on it next week in the CXL track, presented
by another group that's been testing it.  Please tune in!

> > 
> > There's only a handful, so yeah the best way is probably to go ahead and
> > swap k = page->flags for k = *folio_flags(folio, 0) and then handle the
> > couple of outliars.
> 
> If you don't mind, I maybe try to convert it in my changes.
> 

Please feel free! I am happy to help test, CC me on subsequent patches
please!

~Gregory

Re: [PATCH v2] kpageflags: respect folio head-page flag placement

Posted by Kefeng Wang 2 years, 1 month ago


On 2023/11/9 7:33, Gregory Price wrote:
> On Wed, Nov 08, 2023 at 10:01:19AM +0800, Kefeng Wang wrote:
>>
>> Yes, I see your "Node Weights and Weighted Interleave", this is an
>> interesting topic, we need some easy and efficient way to use tiered
>> memory.
>>
> 
> There will be an LPC talk on it next week in the CXL track, presented
> by another group that's been testing it.  Please tune in!

Sure.

> 
>>>
>>> There's only a handful, so yeah the best way is probably to go ahead and
>>> swap k = page->flags for k = *folio_flags(folio, 0) and then handle the
>>> couple of outliars.
>>
>> If you don't mind, I maybe try to convert it in my changes.
>>
> 
> Please feel free! I am happy to help test, CC me on subsequent patches
> please!

Will do, thanks.
> 
> ~Gregory