[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 11 Nov 2023 17:49:38 +0800
From: Kefeng Wang <wangkefeng.wang@...wei.com>
To: Matthew Wilcox <willy@...radead.org>
CC: Andrew Morton <akpm@...ux-foundation.org>,
<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
David Hildenbrand <david@...hat.com>,
Gregory Price <gregory.price@...verge.com>
Subject: Re: [PATCH v2 3/7] fs/proc/page: respect folio head-page flag
placement
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@...verge.com>
>> Suggested-by: Matthew Wilcox <willy@...radead.org>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@...wei.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.
Powered by blists - more mailing lists