* Re: [PATCH 2/2] fs: stable_page_flags(): use folio_test_*() helpers
[not found] ` <20260702110614.2176986-3-tujinjiang@huawei.com>
@ 2026-07-10 3:52 ` Jinjiang Tu
2026-07-10 11:29 ` David Hildenbrand (Arm)
1 sibling, 0 replies; 3+ messages in thread
From: Jinjiang Tu @ 2026-07-10 3:52 UTC (permalink / raw)
To: akpm, ziy, david, luizcap, willy, linmiaohe, svetly.todorov,
xu.xin16, chengming.zhou, linux-fsdevel, linux-mm
Cc: wangkefeng.wang, sunnanyong
在 2026/7/2 19:06, Jinjiang Tu 写道:
> stable_page_flags() currently accesses folio->flags.f directly. Since
> commit 476d87d6a061 ("fs: stable_page_flags(): use snapshot_page()"), we
> can replace these raw flag accesses with the existing folio_test_*()
> helpers, thereby cleaning up the open‑coded flag handling.
>
> For KPF_IDLE, set_ps_flags() already handles both 64‑bit and 32‑bit
> cases, so the duplicate code in stable_page_flags() that deals with
> CONFIG_PAGE_IDLE_FLAG for the 64‑bit case is removed.
>
> The flag bits that has no folio_test_*() helpers are left unchanged,
> including PG_owner_priv_1, PG_arch_*.
>
> No functional change is intended.
I update commit message to explain the history and motivation. The updated
commit message is as follows:
Since commit 304daa8132a9 ("maps4: add /proc/kpageflags interface"),
/proc/kpageflags directly operates on page->flags to determine page status.
Later, commit 177975495914 ("proc: export more page flags in
/proc/kpageflags") started using page helper functions when exposing new
flags, leading to a mix of both approaches.
For tail pages, the original code did not return corresponding status.
commit 0a71649cb724 ("/proc/kpageflags: return KPF_SLAB for slab tail
pages") and commit 832fc1de01ae ("/proc/kpageflags: return KPF_BUDDY for
"tail" buddy pages") made tail slab/buddy pages also return corresponding
status. Then commit dee3d0bef2b0 ("proc: rewrite stable_page_flags()")
made all tail pages return the same status as their head page, except for
hwpoison and mapped flags. It also cached the folio's flags and operate on
the flags directly to avoid concurrency issues if using folio_test_*()
helpers. At this point, folio helpers and direct flag operations are mixed
together in stable_page_flags().
Since commit 476d87d6a061 ("fs: stable_page_flags(): use snapshot_page()"),
the folio has already been snapshotted, we can now safely switch to
folio_test_*() helpers instead of directly operating on flags, which is
more readable and consistent with the rest of the kernel.
For KPF_IDLE, set_ps_flags() already handles both 64-bit and 32-bit cases,
so the duplicate code in stable_page_flags() that deals with
CONFIG_PAGE_IDLE_FLAG for the 64-bit case is removed.
The flag bits that have no folio_test_*() helpers are left unchanged,
including PG_owner_priv_1, PG_arch_*.
No functional change is intended.
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
> fs/proc/page.c | 94 ++++++++++++++++++++++++++------------------------
> 1 file changed, 49 insertions(+), 45 deletions(-)
>
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 2717dbb0431b..0dbd46fa84ea 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -148,8 +148,6 @@ u64 stable_page_flags(const struct page *page)
> const struct folio *folio;
> struct page_snapshot ps;
> unsigned long k;
> - unsigned long mapping;
> - bool is_anon;
> u64 u = 0;
>
> /*
> @@ -163,17 +161,14 @@ u64 stable_page_flags(const struct page *page)
> folio = &ps.folio_snapshot;
>
> k = folio->flags.f;
> - mapping = (unsigned long)folio->mapping;
> - is_anon = mapping & FOLIO_MAPPING_ANON;
> -
> /*
> * pseudo flags for the well known (anonymous) memory mapped pages
> */
> if (folio_mapped(folio))
> u |= BIT_ULL(KPF_MMAP);
> - if (is_anon) {
> + if (folio_test_anon(folio)) {
> u |= BIT_ULL(KPF_ANON);
> - if ((mapping & FOLIO_MAPPING_FLAGS) == FOLIO_MAPPING_KSM)
> + if (folio_test_ksm(folio))
> u |= BIT_ULL(KPF_KSM);
> }
>
> @@ -181,10 +176,12 @@ u64 stable_page_flags(const struct page *page)
> * compound pages: export both head/tail info
> * they together define a compound page's start/end pos and order
> */
> - if (ps.idx == 0)
> - u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
> - else
> + if (ps.idx == 0) {
> + if (folio_test_head(folio))
> + u |= BIT_ULL(KPF_COMPOUND_HEAD);
> + } else {
> u |= BIT_ULL(KPF_COMPOUND_TAIL);
> + }
> if (folio_test_hugetlb(folio))
> u |= BIT_ULL(KPF_HUGE);
> else if (folio_test_large(folio) &&
> @@ -201,6 +198,9 @@ u64 stable_page_flags(const struct page *page)
> if (ps.flags & PAGE_SNAPSHOT_PG_BUDDY)
> u |= BIT_ULL(KPF_BUDDY);
>
> + if (ps.flags & PAGE_SNAPSHOT_PG_IDLE)
> + u |= BIT_ULL(KPF_IDLE);
> +
> if (folio_test_offline(folio))
> u |= BIT_ULL(KPF_OFFLINE);
> if (folio_test_pgtable(folio))
> @@ -208,42 +208,46 @@ u64 stable_page_flags(const struct page *page)
> if (folio_test_slab(folio))
> u |= BIT_ULL(KPF_SLAB);
>
> -#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
> - u |= kpf_copy_bit(k, KPF_IDLE, PG_idle);
> -#else
> - if (ps.flags & PAGE_SNAPSHOT_PG_IDLE)
> - u |= BIT_ULL(KPF_IDLE);
> -#endif
> -
> - u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
> - u |= kpf_copy_bit(k, KPF_DIRTY, PG_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);
> - u |= kpf_copy_bit(k, KPF_RECLAIM, PG_reclaim);
> -
> -#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);
> -
> - u |= kpf_copy_bit(k, KPF_UNEVICTABLE, PG_unevictable);
> - u |= kpf_copy_bit(k, KPF_MLOCKED, PG_mlocked);
> -
> -#ifdef CONFIG_MEMORY_FAILURE
> - if (u & (1 << KPF_HUGE))
> - u |= kpf_copy_bit(k, KPF_HWPOISON, PG_hwpoison);
> - else
> - u |= kpf_copy_bit(ps.page_snapshot.flags.f, KPF_HWPOISON, PG_hwpoison);
> -#endif
> + if (folio_test_locked(folio))
> + u |= BIT_ULL(KPF_LOCKED);
> + if (folio_test_dirty(folio))
> + u |= BIT_ULL(KPF_DIRTY);
> + if (folio_test_uptodate(folio))
> + u |= BIT_ULL(KPF_UPTODATE);
> + if (folio_test_writeback(folio))
> + u |= BIT_ULL(KPF_WRITEBACK);
> +
> + if (folio_test_lru(folio))
> + u |= BIT_ULL(KPF_LRU);
> + if (folio_test_referenced(folio))
> + u |= BIT_ULL(KPF_REFERENCED);
> + if (folio_test_active(folio))
> + u |= BIT_ULL(KPF_ACTIVE);
> + if (folio_test_reclaim(folio))
> + u |= BIT_ULL(KPF_RECLAIM);
> +
> + if (folio_test_swapcache(folio))
> + u |= BIT_ULL(KPF_SWAPCACHE);
> + if (folio_test_swapbacked(folio))
> + u |= BIT_ULL(KPF_SWAPBACKED);
> + if (folio_test_unevictable(folio))
> + u |= BIT_ULL(KPF_UNEVICTABLE);
> + if (folio_test_mlocked(folio))
> + u |= BIT_ULL(KPF_MLOCKED);
> +
> + if ((folio_test_hugetlb(folio) && folio_test_hwpoison(folio))
> + || PageHWPoison(&ps.page_snapshot))
> + u |= BIT_ULL(KPF_HWPOISON);
> +
> + if (folio_test_reserved(folio))
> + u |= BIT_ULL(KPF_RESERVED);
> + if (folio_test_owner_2(folio))
> + u |= BIT_ULL(KPF_OWNER_2);
> + if (folio_test_private(folio))
> + u |= BIT_ULL(KPF_PRIVATE);
> + if (folio_test_private_2(folio))
> + u |= BIT_ULL(KPF_PRIVATE_2);
>
> - u |= kpf_copy_bit(k, KPF_RESERVED, PG_reserved);
> - u |= kpf_copy_bit(k, KPF_OWNER_2, PG_owner_2);
> - 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(k, KPF_ARCH, PG_arch_1);
> #ifdef CONFIG_ARCH_USES_PG_ARCH_2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 2/2] fs: stable_page_flags(): use folio_test_*() helpers
[not found] ` <20260702110614.2176986-3-tujinjiang@huawei.com>
2026-07-10 3:52 ` [PATCH 2/2] " Jinjiang Tu
@ 2026-07-10 11:29 ` David Hildenbrand (Arm)
1 sibling, 0 replies; 3+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-10 11:29 UTC (permalink / raw)
To: Jinjiang Tu, akpm, ziy, luizcap, willy, linmiaohe, svetly.todorov,
xu.xin16, chengming.zhou, linux-fsdevel, linux-mm
Cc: wangkefeng.wang, sunnanyong
On 7/2/26 13:06, Jinjiang Tu wrote:
> stable_page_flags() currently accesses folio->flags.f directly. Since
> commit 476d87d6a061 ("fs: stable_page_flags(): use snapshot_page()"), we
> can replace these raw flag accesses with the existing folio_test_*()
> helpers, thereby cleaning up the open‑coded flag handling.
>
> For KPF_IDLE, set_ps_flags() already handles both 64‑bit and 32‑bit
> cases, so the duplicate code in stable_page_flags() that deals with
> CONFIG_PAGE_IDLE_FLAG for the 64‑bit case is removed.
>
> The flag bits that has no folio_test_*() helpers are left unchanged,
> including PG_owner_priv_1, PG_arch_*.
>
> No functional change is intended.
>
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
> fs/proc/page.c | 94 ++++++++++++++++++++++++++------------------------
> 1 file changed, 49 insertions(+), 45 deletions(-)
>
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 2717dbb0431b..0dbd46fa84ea 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -148,8 +148,6 @@ u64 stable_page_flags(const struct page *page)
> const struct folio *folio;
> struct page_snapshot ps;
> unsigned long k;
> - unsigned long mapping;
> - bool is_anon;
> u64 u = 0;
>
> /*
> @@ -163,17 +161,14 @@ u64 stable_page_flags(const struct page *page)
> folio = &ps.folio_snapshot;
>
> k = folio->flags.f;
> - mapping = (unsigned long)folio->mapping;
> - is_anon = mapping & FOLIO_MAPPING_ANON;
> -
> /*
> * pseudo flags for the well known (anonymous) memory mapped pages
> */
> if (folio_mapped(folio))
> u |= BIT_ULL(KPF_MMAP);
> - if (is_anon) {
> + if (folio_test_anon(folio)) {
> u |= BIT_ULL(KPF_ANON);
> - if ((mapping & FOLIO_MAPPING_FLAGS) == FOLIO_MAPPING_KSM)
> + if (folio_test_ksm(folio))
> u |= BIT_ULL(KPF_KSM);
> }
These make sense.
>
> @@ -181,10 +176,12 @@ u64 stable_page_flags(const struct page *page)
> * compound pages: export both head/tail info
> * they together define a compound page's start/end pos and order
> */
> - if (ps.idx == 0)
> - u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
> - else
> + if (ps.idx == 0) {
> + if (folio_test_head(folio))
> + u |= BIT_ULL(KPF_COMPOUND_HEAD);
Once we decouple folios from pages, we will have compound pages that are not
folios. PG_head is not folio specific.
But then, some of the flags below are used in other context outside of folios.
I would focus here only on converting things to use folios that will actually be
folio-specific: like the folio_test_anon() above.
> + } else {
> u |= BIT_ULL(KPF_COMPOUND_TAIL);
> + }
> if (folio_test_hugetlb(folio))
> u |= BIT_ULL(KPF_HUGE);
> else if (folio_test_large(folio) &&
> @@ -201,6 +198,9 @@ u64 stable_page_flags(const struct page *page)
> if (ps.flags & PAGE_SNAPSHOT_PG_BUDDY)
> u |= BIT_ULL(KPF_BUDDY);
>
> + if (ps.flags & PAGE_SNAPSHOT_PG_IDLE)
> + u |= BIT_ULL(KPF_IDLE);
> +
That looks like an independent cleanup, right? Should probably go in a separate
patch then.
> if (folio_test_offline(folio))
> u |= BIT_ULL(KPF_OFFLINE);
> if (folio_test_pgtable(folio))
> @@ -208,42 +208,46 @@ u64 stable_page_flags(const struct page *page)
> if (folio_test_slab(folio))
> u |= BIT_ULL(KPF_SLAB);
>
> -#if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
> - u |= kpf_copy_bit(k, KPF_IDLE, PG_idle);
> -#else
> - if (ps.flags & PAGE_SNAPSHOT_PG_IDLE)
> - u |= BIT_ULL(KPF_IDLE);
> -#endif
> -
> - u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
> - u |= kpf_copy_bit(k, KPF_DIRTY, PG_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);
> - u |= kpf_copy_bit(k, KPF_RECLAIM, PG_reclaim);
> -
> -#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);
> -
> - u |= kpf_copy_bit(k, KPF_UNEVICTABLE, PG_unevictable);
> - u |= kpf_copy_bit(k, KPF_MLOCKED, PG_mlocked);
> -
> -#ifdef CONFIG_MEMORY_FAILURE
> - if (u & (1 << KPF_HUGE))
> - u |= kpf_copy_bit(k, KPF_HWPOISON, PG_hwpoison);
> - else
> - u |= kpf_copy_bit(ps.page_snapshot.flags.f, KPF_HWPOISON, PG_hwpoison);
> -#endif
> + if (folio_test_locked(folio))
> + u |= BIT_ULL(KPF_LOCKED);
> + if (folio_test_dirty(folio))
> + u |= BIT_ULL(KPF_DIRTY);
> + if (folio_test_uptodate(folio))
> + u |= BIT_ULL(KPF_UPTODATE);
> + if (folio_test_writeback(folio))
> + u |= BIT_ULL(KPF_WRITEBACK);
> +
> + if (folio_test_lru(folio))
> + u |= BIT_ULL(KPF_LRU);
> + if (folio_test_referenced(folio))
> + u |= BIT_ULL(KPF_REFERENCED);
> + if (folio_test_active(folio))
> + u |= BIT_ULL(KPF_ACTIVE);
> + if (folio_test_reclaim(folio))
> + u |= BIT_ULL(KPF_RECLAIM);
> +
> + if (folio_test_swapcache(folio))
> + u |= BIT_ULL(KPF_SWAPCACHE);
> + if (folio_test_swapbacked(folio))
> + u |= BIT_ULL(KPF_SWAPBACKED);
> + if (folio_test_unevictable(folio))
> + u |= BIT_ULL(KPF_UNEVICTABLE);
> + if (folio_test_mlocked(folio))
> + u |= BIT_ULL(KPF_MLOCKED);
> +
> + if ((folio_test_hugetlb(folio) && folio_test_hwpoison(folio))
> + || PageHWPoison(&ps.page_snapshot))
> + u |= BIT_ULL(KPF_HWPOISON);
> +
> + if (folio_test_reserved(folio))
> + u |= BIT_ULL(KPF_RESERVED);
That is not a pure folio flag.
> + if (folio_test_owner_2(folio))
> + u |= BIT_ULL(KPF_OWNER_2);
> + if (folio_test_private(folio))
> + u |= BIT_ULL(KPF_PRIVATE);
> + if (folio_test_private_2(folio))
> + u |= BIT_ULL(KPF_PRIVATE_2);
These as well.
So I'd probably leave the page bit checking alone for now and focus on the
mapping only.
--
Cheers,
David
^ permalink raw reply [flat|nested] 3+ messages in thread