Re: [PATCH v2 3/7] fs/proc/page: respect folio head-page flag placement

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2023/11/11 2:19, Matthew Wilcox wrote:
> On Fri, Nov 10, 2023 at 11:33:20AM +0800, Kefeng Wang wrote:
>> kpageflags reads page-flags directly from the page, even when the
>> respective flag is only updated on the headpage of a folio.
>>
>> Since most flags are stored in head flags, make k = folio->flags,
>> and add new p = page->flags used for per-page flags.
> 
> You'd do better to steal Greg's commit message.

Sure

> 
>> Originally-from: Gregory Price <gregory.price@memverge.com>
>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> 
>> @@ -202,7 +202,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);
> 
> This is correct.
> 
>> @@ -211,13 +211,13 @@ u64 stable_page_flags(struct page *page)
>>   
>>   	u |= kpf_copy_bit(k, KPF_RESERVED,	PG_reserved);
>>   	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(k, KPF_OWNER_PRIVATE,	PG_owner_priv_1);
>> +	u |= kpf_copy_bit(p, KPF_PRIVATE,	PG_private);
>> +	u |= kpf_copy_bit(p, KPF_PRIVATE_2,	PG_private_2);
>> +	u |= kpf_copy_bit(p, KPF_OWNER_PRIVATE,	PG_owner_priv_1);
> 
> This is not.  PG_private is not, I believe, set on tail pages.
> Ditto the other two.  If you know differently ... ?

k = folio->flags, p = page->flags
as PG_private/private_2/owner_priv_1 use PF_ANY page policy,
so they should per-page check, confused...

See checked page flags,

PG_error        PF_NO_TAIL
PG_dirty        PF_HEAD
PG_uptodate     PF_NO_TAIL
PG_writeback    PF_NO_TAIL
PG_lru          PF_HEAD
PG_referenced 	PF_HEAD
PG_active       PF_HEAD
PG_reclaim      PF_NO_TAIL
PG_swapbacked   PF_NO_TAIL
PG_unevictable  PF_HEAD
PG_mlocked      PF_NO_TAIL
PG_hwpoison     PF_ANY
PG_uncached     PF_NO_COMPOUND	
PG_reserved     PF_NO_COMPOUND
PG_mappedtodisk PF_NO_TAIL
PG_private      PF_ANY
PG_private_2    PF_ANY
PG_owner_priv_1 PF_ANY

above part has 4 types,

1) PF_HEAD        - should use k

2) PF_ANY         - should use p

3) PF_NO_TAIL
    - PageXXX will check head page flags, so suppose to use k,
      but here use bit check, I wonder it should use p?

4) PF_NO_COMPOUND
   - should per-page check
> 
>>   #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
> 
> I also don't think this is correct, but there are many uses of
> PG_arch* and I may have missed something.
> 

And 3 arch page flags,

PG_arch_1
  - PG_dcache_clean, from achetlb.rst, I think it is per-folio, use k
	
PG_arch_2
  - only arm64 mte, PG_mte_tagged
PG_arch_3
  - only arm64 mte, PG_mte_lock

the two PG_arch only used by arm64 mte, they are per-page flag, use p

Correct me if I am wrong, thanks.