mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] mm/mglru: cacheline follow-ups on the reclaim hot paths
@ 2026-11-05 13:22 Ridong Chen
  2026-11-05 13:22 ` [RFC PATCH 1/3] mm/mglru: skip redundant bitmap store in lru_gen_use_mm() Ridong Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Ridong Chen @ 2026-11-05 13:22 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton, Johannes Weiner
  Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Baoquan He, Baolin Wang, Joanne Koong, linux-mm, linux-kernel,
	Ridong Chen, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

Joanne's series to reduce cacheline contention around struct
mem_cgroup_per_node [1], now in linux-next, made me look at the MGLRU
side of the same structures, and at the stores MGLRU does on hot paths,
to see whether any of them write a line for no reason. This RFC has the
three candidates that showed a measurable difference. They are
independent of each other.

1. lru_gen_use_mm() writes mm->lru_gen.bitmap on every context switch
   to a user task, although the bitmap is almost always already set.
   For threads of one process spread over several CPUs, the line
   bounces between them. Read first and skip the store: +2-3% context
   switches per CPU for a shared mm, no change for private mms.

2. folio_update_gen() and folio_set_lru_refs() always end with a lock
   cmpxchg, even when the flags do not change. During aging, a folio
   mapped by N processes is visited N times and only the first visit
   changes anything. Skip the no-op cmpxchg: an aging pass over a 2GiB
   file shared by 8-64 processes gets 18-22% faster.

3. lrugen->evicted[] and ->refaulted[] are updated locklessly on
   every eviction and refault, and the file type counters share a line
   with lrugen->enabled/gen/seg/list, which are read on every LRU add
   and by shrink_many(). Move the read-mostly fields up and give the
   counters their own lines: +4.3% fio IOPS with 24 jobs refaulting
   file pages in one memcg, no change at 8 jobs or fewer. This one
   costs a cacheline per memcg per node, and its gain needs many CPUs
   refaulting from a fast backing store, so I am not sure it is worth
   it on its own; feedback welcome.

All numbers are from an i7-13700HX (8P+8E, 24 threads, one socket) with
turbo disabled and the performance governor, on next-20261023 with the
same config for base and patched kernels. Details are in each patch.
The series is rebased onto next-20261104, which includes [1], without
code changes; the struct sizes in patch 3 are updated for it.

Limitations of the testing, where input would be welcome:

- Patch 1 does not show up in will-it-scale context_switch1 or
  sched_yield (explained in the patch), and perf bench sched messaging
  and schbench are too noisy here to resolve 2-3%. The numbers come
  from a small test that pins two sched_yield()ing threads per CPU,
  with a one-process-per-CPU control.

- Patch 2 has no standard benchmark either. In fio with many mmap()
  readers under memcg pressure, reclaim time goes to rmap walks on the
  eviction side and aging barely runs. The numbers come from triggering
  aging through debugfs with force_scan, which is the upper bound. On
  real systems the gain scales with how many processes in the same
  memcg share the hot file pages, e.g. preforked servers or Android
  without per-app memcgs.

- Patch 3 depends on the layout of the containing structs, which
  depends on the config (LRU_GEN_STATS changes the array sizes). I
  checked LRU_GEN_STATS=n only.

The test programs are small C files; I can post them if that helps.

[1] https://lore.kernel.org/20261023195417.1391216-1-joannelkoong@gmail.com


Ridong Chen (3):
  mm/mglru: skip redundant bitmap store in lru_gen_use_mm()
  mm/mglru: skip the flags cmpxchg when nothing changes
  mm/mglru: move lrugen atomic stats off the read-mostly lines

 include/linux/mm_inline.h |  2 ++
 include/linux/mm_types.h  |  3 ++-
 include/linux/mmzone.h    | 18 ++++++++++--------
 mm/vmscan.c               |  2 ++
 4 files changed, 16 insertions(+), 9 deletions(-)


base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH 1/3] mm/mglru: skip redundant bitmap store in lru_gen_use_mm()
  2026-11-05 13:22 [RFC PATCH 0/3] mm/mglru: cacheline follow-ups on the reclaim hot paths Ridong Chen
@ 2026-11-05 13:22 ` Ridong Chen
  2026-11-05 13:22 ` [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes Ridong Chen
  2026-11-05 13:22 ` [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines Ridong Chen
  2 siblings, 0 replies; 12+ messages in thread
From: Ridong Chen @ 2026-11-05 13:22 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton, Johannes Weiner
  Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Baoquan He, Baolin Wang, Joanne Koong, linux-mm, linux-kernel,
	Ridong Chen, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

lru_gen_use_mm() is called from context_switch() on every switch to a
user task and unconditionally sets mm->lru_gen.bitmap to -1. The bitmap
is only cleared by the page table walkers in get_next_mm() during aging,
so between aging passes it is already all ones and the store is
redundant.

The store is not free when the mm_struct is shared: every CPU running a
thread of the same process must take the cacheline in exclusive state,
so the line bounces between CPUs on every context switch. On x86,
switch_mm_irqs_off() returns early when switching between threads of
the same mm, which can make this the only shared write on that path.

Read the bitmap first and only write it when it is not already set.
The semantics are unchanged: the plain WRITE_ONCE() had no ordering
guarantees, and a walker clearing a bit concurrently can be missed in
either version.

Tested on an i7-13700HX (8P+8E cores, 24 threads), with turbo disabled
and the performance governor. On each CPU two tasks sched_yield() to
each other, either as threads of one process (shared mm) or as one
process per CPU (private mm, control). Context switches per second per
CPU, median of 5 runs, averaged over 2 boots of each kernel:

  CPUs   shared mm              private mm
         base     patched       base     patched
     1   1114766  1106870 -0.7% 1104512  1109284 +0.4%
     2   1086513  1110204 +2.2% 1110334  1112209 +0.2%
     4   1085872  1112258 +2.4% 1113146  1112467 -0.1%
     8   1085945  1111371 +2.3% 1111662  1111876 +0.0%
    16    660870   666748 +0.9%  665470   667058 +0.2%
    24    579382   596472 +2.9%  596177   598500 +0.4%

The 16 CPU case uses both SMT siblings of each P-core, which share the
L1/L2, so less of the traffic crosses cores. With the patch, the shared
mm case is within noise of the private mm case.

will-it-scale context_switch1 and sched_yield do not show a difference
(within 1% for both threads and processes): sched_yield runs one task
per CPU and never switches, and context_switch1 leaves the pipe peer
unpinned, so most switches stay on one CPU and the line does not move.

Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 include/linux/mm_types.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 1ed35d258d2e..3680cc0fd714 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1553,7 +1553,8 @@ static inline void lru_gen_use_mm(struct mm_struct *mm)
 	 * used since the last time it cleared the bitmap. So it might be worth
 	 * walking the page tables of this mm_struct to clear the accessed bit.
 	 */
-	WRITE_ONCE(mm->lru_gen.bitmap, -1);
+	if (READ_ONCE(mm->lru_gen.bitmap) != -1UL)
+		WRITE_ONCE(mm->lru_gen.bitmap, -1UL);
 }
 
 #else /* !CONFIG_LRU_GEN_WALKS_MMU */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-05 13:22 [RFC PATCH 0/3] mm/mglru: cacheline follow-ups on the reclaim hot paths Ridong Chen
  2026-11-05 13:22 ` [RFC PATCH 1/3] mm/mglru: skip redundant bitmap store in lru_gen_use_mm() Ridong Chen
@ 2026-11-05 13:22 ` Ridong Chen
  2026-11-05 16:21   ` Kairui Song
  2026-11-08  8:50   ` Barry Song
  2026-11-05 13:22 ` [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines Ridong Chen
  2 siblings, 2 replies; 12+ messages in thread
From: Ridong Chen @ 2026-11-05 13:22 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton, Johannes Weiner
  Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Baoquan He, Baolin Wang, Joanne Koong, linux-mm, linux-kernel,
	Ridong Chen, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

folio_update_gen() and folio_set_lru_refs() always end with a
try_cmpxchg() on folio->flags, even when the new value equals the old
one. The locked instruction still takes the struct page cacheline in
exclusive state.

That is common in practice: a folio mapped by many processes is visited
once per mm during a page table walk, and after the first visit it is
already in the youngest generation with refs cleared and PG_workingset
set. isolate_folio() and the eviction paths also call
folio_set_lru_refs(folio, 0) on folios whose refs are already zero.

Break out of the loop when the flags would not change. The result is
the same as a successful no-op cmpxchg. The only difference is that the
full barrier implied by the locked instruction is gone on that path.
No caller relies on it: the eviction and isolation callers run under
the LRU lock, and the others (page table walks, folio_mark_accessed(),
lru_gen_clear_refs()) only care about the resulting flags value, which
is already in place. Note that lru_gen_inc_refs() already returns
without any atomic when refs is at LRU_REFS_MAX and the folio is in
the workingset.

Tested on an i7-13700HX with turbo disabled and the performance
governor. N processes map the same 2GiB tmpfs file (huge=never) and
read every page, then one aging pass is triggered on their memcg
through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
over 2 boots of each kernel:

  processes    base   patched
          1    32.9      31.7   -3.5%
          8   195.1     160.2  -17.9%
         32   755.8     592.2  -21.7%
         64  1597.2    1260.6  -21.1%

With one process, every visit changes the generation, so there is
nothing to skip. Before the patch, perf annotate attributes 53-58% of
the walk_update_folio() samples to the lock cmpxchg in the inlined
folio_update_gen() in this test.

Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 include/linux/mm_inline.h | 2 ++
 mm/vmscan.c               | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index b3d85fe91385..4ba9079f4dc8 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
 	do {
 		new_flags = old_flags;
 		lru_set_refs_flags(&new_flags, refs);
+		if (new_flags == old_flags)
+			break;
 	} while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
 }
 
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 50c352e9ffef..3fa76715382d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
 		lru_set_gen_flags(&new_flags, new_gen);
 		lru_set_refs_flags(&new_flags, 0);
 		new_flags |= BIT(PG_workingset);
+		if (new_flags == old_flags)
+			break;
 	} while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
 
 	*type = folio_flags_is_file_lru(&old_flags);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines
  2026-11-05 13:22 [RFC PATCH 0/3] mm/mglru: cacheline follow-ups on the reclaim hot paths Ridong Chen
  2026-11-05 13:22 ` [RFC PATCH 1/3] mm/mglru: skip redundant bitmap store in lru_gen_use_mm() Ridong Chen
  2026-11-05 13:22 ` [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes Ridong Chen
@ 2026-11-05 13:22 ` Ridong Chen
  2026-11-08  9:14   ` Barry Song
  2 siblings, 1 reply; 12+ messages in thread
From: Ridong Chen @ 2026-11-05 13:22 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton, Johannes Weiner
  Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu,
	Baoquan He, Baolin Wang, Joanne Koong, linux-mm, linux-kernel,
	Ridong Chen, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

lru_gen_eviction() and lru_gen_refault() update lrugen->evicted[] and
lrugen->refaulted[] locklessly with atomic_long_add(). The arrays sit at
the tail of struct lru_gen_folio, right before enabled/gen/seg/list:
with LRU_GEN_STATS=n the file type counters share a cacheline with them.

Those fields are read far more often than they are written:
lru_gen_add_folio() reads ->enabled on every LRU add, and shrink_many()
walks ->list and reads ->gen/->seg under RCU. With frequent file refaults
from several CPUs, every refault invalidates that line on the CPUs doing
LRU adds.

Move enabled/gen/seg/list next to max_seq/min_seq/timestamps, which are
also mostly read, and put evicted[]/refaulted[] in their own cacheline
group at the end of the struct. This grows struct lru_gen_folio from
1360 to 1408 bytes and struct mem_cgroup_per_node from 2176 to 2240
bytes on x86_64 (one more cacheline per memcg per node).

Tested on an i7-13700HX (24 threads) with turbo disabled and the
performance governor. 24 files of 128MiB on ext4 on brd, read with
fio (psync, randread, bs=4k, fadvise_hint=random, buffered), all jobs
in one memcg with memory.max=1G, so nearly every read refaults.
IOPS, median of 5 runs:

  jobs      base   patched
     1    270537    274303   +1.4%  (sd 3.7%)
     8   1461025   1466293   +0.4%
    16   2455629   2508001   +2.1%
    24   2707720   2823776   +4.3%

A custom random pread() test of the same setup, with this patch alone
against base over two boots of each kernel, shows +2.4% at 16 threads
and +5.9% at 24 threads, and no change at 8 threads or fewer. With
anonymous memory on zram, the refaults mostly hit the tier 0 counters
on a different line, and there is no measurable change.

The gain grows with the number of CPUs refaulting into the same lruvec
and needs a fast backing store; with real IO it is likely to be lost in
the noise. The patch is mostly meant to keep the lockless counters away
from the read-mostly fields regardless of where they end up.

Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 include/linux/mmzone.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 6442e662aa4f..eb84af674e23 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -551,6 +551,14 @@ struct lru_gen_folio {
 	unsigned long min_seq[ANON_AND_FILE];
 	/* the birth time of each generation in jiffies */
 	unsigned long timestamps[MAX_NR_GENS];
+	/* whether the multi-gen LRU is enabled */
+	bool enabled;
+	/* the memcg generation this lru_gen_folio belongs to */
+	u8 gen;
+	/* the list segment this lru_gen_folio belongs to */
+	u8 seg;
+	/* per-node lru_gen_folio list for global reclaim */
+	struct hlist_nulls_node list;
 	/* the multi-gen LRU lists, lazily sorted on eviction */
 	struct list_head folios[MAX_NR_GENS][ANON_AND_FILE][MAX_NR_ZONES];
 	/* the multi-gen LRU sizes, eventually consistent */
@@ -562,16 +570,10 @@ struct lru_gen_folio {
 	/* can only be modified under the LRU lock */
 	unsigned long protected[NR_HIST_GENS][ANON_AND_FILE][MAX_NR_TIERS];
 	/* can be modified without holding the LRU lock */
+	__cacheline_group_begin_aligned(lrugen_stats);
 	atomic_long_t evicted[NR_HIST_GENS][ANON_AND_FILE][MAX_NR_TIERS];
 	atomic_long_t refaulted[NR_HIST_GENS][ANON_AND_FILE][MAX_NR_TIERS];
-	/* whether the multi-gen LRU is enabled */
-	bool enabled;
-	/* the memcg generation this lru_gen_folio belongs to */
-	u8 gen;
-	/* the list segment this lru_gen_folio belongs to */
-	u8 seg;
-	/* per-node lru_gen_folio list for global reclaim */
-	struct hlist_nulls_node list;
+	__cacheline_group_end_aligned(lrugen_stats);
 };
 
 enum {
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-05 13:22 ` [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes Ridong Chen
@ 2026-11-05 16:21   ` Kairui Song
  2026-11-06  1:43     ` Ridong Chen
  2026-11-08  8:50   ` Barry Song
  1 sibling, 1 reply; 12+ messages in thread
From: Kairui Song @ 2026-11-05 16:21 UTC (permalink / raw)
  To: Ridong Chen
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Qi Zheng, Shakeel Butt,
	Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen

On Thu, Nov 5, 2026 at 10:11 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>
> From: Ridong Chen <chenridong@xiaomi.com>
>
> folio_update_gen() and folio_set_lru_refs() always end with a
> try_cmpxchg() on folio->flags, even when the new value equals the old
> one. The locked instruction still takes the struct page cacheline in
> exclusive state.
>
> That is common in practice: a folio mapped by many processes is visited
> once per mm during a page table walk, and after the first visit it is
> already in the youngest generation with refs cleared and PG_workingset
> set. isolate_folio() and the eviction paths also call
> folio_set_lru_refs(folio, 0) on folios whose refs are already zero.
>
> Break out of the loop when the flags would not change. The result is
> the same as a successful no-op cmpxchg. The only difference is that the
> full barrier implied by the locked instruction is gone on that path.
> No caller relies on it: the eviction and isolation callers run under
> the LRU lock, and the others (page table walks, folio_mark_accessed(),
> lru_gen_clear_refs()) only care about the resulting flags value, which
> is already in place. Note that lru_gen_inc_refs() already returns
> without any atomic when refs is at LRU_REFS_MAX and the folio is in
> the workingset.
>
> Tested on an i7-13700HX with turbo disabled and the performance
> governor. N processes map the same 2GiB tmpfs file (huge=never) and
> read every page, then one aging pass is triggered on their memcg
> through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
> Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
> over 2 boots of each kernel:
>
>   processes    base   patched
>           1    32.9      31.7   -3.5%
>           8   195.1     160.2  -17.9%
>          32   755.8     592.2  -21.7%
>          64  1597.2    1260.6  -21.1%
>
> With one process, every visit changes the generation, so there is
> nothing to skip. Before the patch, perf annotate attributes 53-58% of
> the walk_update_folio() samples to the lock cmpxchg in the inlined
> folio_update_gen() in this test.
>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>  include/linux/mm_inline.h | 2 ++
>  mm/vmscan.c               | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index b3d85fe91385..4ba9079f4dc8 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
>         do {
>                 new_flags = old_flags;
>                 lru_set_refs_flags(&new_flags, refs);
> +               if (new_flags == old_flags)
> +                       break;
>         } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>  }
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 50c352e9ffef..3fa76715382d 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
>                 lru_set_gen_flags(&new_flags, new_gen);
>                 lru_set_refs_flags(&new_flags, 0);
>                 new_flags |= BIT(PG_workingset);
> +               if (new_flags == old_flags)
> +                       break;
>         } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));

Hi Ridong,

I had the same idea in V4 of MGLRU-FG which is posted a few weeks ago,
with the same code:
https://lore.kernel.org/linux-mm/20261016-mglru-fg-v4-4-2106750c0bfa@tencent.com/

+ if (new_flags == old_flags)
+ break;

I didn't have time to benchmark that one function but the e2e test
showed very trivial improvement, it's a good improvement though. Feel
free to add:

Reviewed-by: Kairui Song <kasong@tencent.com>

BTW there are a lot of concurrent activity is happening for MGLRU at
the moment so conflicts occur very frequently...

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-05 16:21   ` Kairui Song
@ 2026-11-06  1:43     ` Ridong Chen
  2026-11-07 10:39       ` Kairui Song
  0 siblings, 1 reply; 12+ messages in thread
From: Ridong Chen @ 2026-11-06  1:43 UTC (permalink / raw)
  To: Kairui Song
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Qi Zheng, Shakeel Butt,
	Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen



On 11/6/2026 12:21 AM, Kairui Song wrote:
> On Thu, Nov 5, 2026 at 10:11 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> folio_update_gen() and folio_set_lru_refs() always end with a
>> try_cmpxchg() on folio->flags, even when the new value equals the old
>> one. The locked instruction still takes the struct page cacheline in
>> exclusive state.
>>
>> That is common in practice: a folio mapped by many processes is visited
>> once per mm during a page table walk, and after the first visit it is
>> already in the youngest generation with refs cleared and PG_workingset
>> set. isolate_folio() and the eviction paths also call
>> folio_set_lru_refs(folio, 0) on folios whose refs are already zero.
>>
>> Break out of the loop when the flags would not change. The result is
>> the same as a successful no-op cmpxchg. The only difference is that the
>> full barrier implied by the locked instruction is gone on that path.
>> No caller relies on it: the eviction and isolation callers run under
>> the LRU lock, and the others (page table walks, folio_mark_accessed(),
>> lru_gen_clear_refs()) only care about the resulting flags value, which
>> is already in place. Note that lru_gen_inc_refs() already returns
>> without any atomic when refs is at LRU_REFS_MAX and the folio is in
>> the workingset.
>>
>> Tested on an i7-13700HX with turbo disabled and the performance
>> governor. N processes map the same 2GiB tmpfs file (huge=never) and
>> read every page, then one aging pass is triggered on their memcg
>> through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
>> Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
>> over 2 boots of each kernel:
>>
>>    processes    base   patched
>>            1    32.9      31.7   -3.5%
>>            8   195.1     160.2  -17.9%
>>           32   755.8     592.2  -21.7%
>>           64  1597.2    1260.6  -21.1%
>>
>> With one process, every visit changes the generation, so there is
>> nothing to skip. Before the patch, perf annotate attributes 53-58% of
>> the walk_update_folio() samples to the lock cmpxchg in the inlined
>> folio_update_gen() in this test.
>>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   include/linux/mm_inline.h | 2 ++
>>   mm/vmscan.c               | 2 ++
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
>> index b3d85fe91385..4ba9079f4dc8 100644
>> --- a/include/linux/mm_inline.h
>> +++ b/include/linux/mm_inline.h
>> @@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
>>          do {
>>                  new_flags = old_flags;
>>                  lru_set_refs_flags(&new_flags, refs);
>> +               if (new_flags == old_flags)
>> +                       break;
>>          } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>>   }
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 50c352e9ffef..3fa76715382d 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
>>                  lru_set_gen_flags(&new_flags, new_gen);
>>                  lru_set_refs_flags(&new_flags, 0);
>>                  new_flags |= BIT(PG_workingset);
>> +               if (new_flags == old_flags)
>> +                       break;
>>          } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
> 
> Hi Ridong,
> 
> I had the same idea in V4 of MGLRU-FG which is posted a few weeks ago,
> with the same code:
> https://lore.kernel.org/linux-mm/20261016-mglru-fg-v4-4-2106750c0bfa@tencent.com/
> 
> + if (new_flags == old_flags)
> + break;
> 

Hi Kairui,

Oh. Sorry about that. I read the series a few weeks ago, but as it is large
I missed this change. I should have checked it again before sending.

> I didn't have time to benchmark that one function but the e2e test
> showed very trivial improvement, it's a good improvement though. Feel
> free to add:
> 
> Reviewed-by: Kairui Song <kasong@tencent.com>
> 

Thanks for the Reviewed-by. If you don't mind, I'd suggest merging it as
a separate patch, as the test shows the benefit of this small change,
which makes the code easier to trace. I will also mention in the commit
message that it is a small part of your series. Does that sound OK to
you?

> BTW there are a lot of concurrent activity is happening for MGLRU at
> the moment so conflicts occur very frequently...

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-06  1:43     ` Ridong Chen
@ 2026-11-07 10:39       ` Kairui Song
  2026-11-07 12:04         ` Ridong Chen
  0 siblings, 1 reply; 12+ messages in thread
From: Kairui Song @ 2026-11-07 10:39 UTC (permalink / raw)
  To: Ridong Chen
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Qi Zheng, Shakeel Butt,
	Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen

On Fri, Nov 6, 2026 at 9:44 AM Ridong Chen <ridong.chen@linux.dev> wrote:
>
> On 11/6/2026 12:21 AM, Kairui Song wrote:
> > On Thu, Nov 5, 2026 at 10:11 PM Ridong Chen <ridong.chen@linux.dev> wrote:
> >>
> >> From: Ridong Chen <chenridong@xiaomi.com>
> >>
> >> folio_update_gen() and folio_set_lru_refs() always end with a
> >> try_cmpxchg() on folio->flags, even when the new value equals the old
> >> one. The locked instruction still takes the struct page cacheline in
> >> exclusive state.
> >>
> >> That is common in practice: a folio mapped by many processes is visited
> >> once per mm during a page table walk, and after the first visit it is
> >> already in the youngest generation with refs cleared and PG_workingset
> >> set. isolate_folio() and the eviction paths also call
> >> folio_set_lru_refs(folio, 0) on folios whose refs are already zero.
> >>
> >> Break out of the loop when the flags would not change. The result is
> >> the same as a successful no-op cmpxchg. The only difference is that the
> >> full barrier implied by the locked instruction is gone on that path.
> >> No caller relies on it: the eviction and isolation callers run under
> >> the LRU lock, and the others (page table walks, folio_mark_accessed(),
> >> lru_gen_clear_refs()) only care about the resulting flags value, which
> >> is already in place. Note that lru_gen_inc_refs() already returns
> >> without any atomic when refs is at LRU_REFS_MAX and the folio is in
> >> the workingset.
> >>
> >> Tested on an i7-13700HX with turbo disabled and the performance
> >> governor. N processes map the same 2GiB tmpfs file (huge=never) and
> >> read every page, then one aging pass is triggered on their memcg
> >> through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
> >> Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
> >> over 2 boots of each kernel:
> >>
> >>    processes    base   patched
> >>            1    32.9      31.7   -3.5%
> >>            8   195.1     160.2  -17.9%
> >>           32   755.8     592.2  -21.7%
> >>           64  1597.2    1260.6  -21.1%
> >>
> >> With one process, every visit changes the generation, so there is
> >> nothing to skip. Before the patch, perf annotate attributes 53-58% of
> >> the walk_update_folio() samples to the lock cmpxchg in the inlined
> >> folio_update_gen() in this test.
> >>
> >> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> >> ---
> >>   include/linux/mm_inline.h | 2 ++
> >>   mm/vmscan.c               | 2 ++
> >>   2 files changed, 4 insertions(+)
> >>
> >> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> >> index b3d85fe91385..4ba9079f4dc8 100644
> >> --- a/include/linux/mm_inline.h
> >> +++ b/include/linux/mm_inline.h
> >> @@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
> >>          do {
> >>                  new_flags = old_flags;
> >>                  lru_set_refs_flags(&new_flags, refs);
> >> +               if (new_flags == old_flags)
> >> +                       break;
> >>          } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
> >>   }
> >>
> >> diff --git a/mm/vmscan.c b/mm/vmscan.c
> >> index 50c352e9ffef..3fa76715382d 100644
> >> --- a/mm/vmscan.c
> >> +++ b/mm/vmscan.c
> >> @@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
> >>                  lru_set_gen_flags(&new_flags, new_gen);
> >>                  lru_set_refs_flags(&new_flags, 0);
> >>                  new_flags |= BIT(PG_workingset);
> >> +               if (new_flags == old_flags)
> >> +                       break;
> >>          } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
> >
> > Hi Ridong,
> >
> > I had the same idea in V4 of MGLRU-FG which is posted a few weeks ago,
> > with the same code:
> > https://lore.kernel.org/linux-mm/20261016-mglru-fg-v4-4-2106750c0bfa@tencent.com/
> >
> > + if (new_flags == old_flags)
> > + break;
> >
>
> Hi Kairui,
>
> Oh. Sorry about that. I read the series a few weeks ago, but as it is large
> I missed this change. I should have checked it again before sending.
>
> > I didn't have time to benchmark that one function but the e2e test
> > showed very trivial improvement, it's a good improvement though. Feel
> > free to add:
> >
> > Reviewed-by: Kairui Song <kasong@tencent.com>
> >
>
> Thanks for the Reviewed-by. If you don't mind, I'd suggest merging it as
> a separate patch, as the test shows the benefit of this small change,
> which makes the code easier to trace. I will also mention in the commit
> message that it is a small part of your series. Does that sound OK to
> you?

Sounds good to me! No need to mention that, though; I believe you came
up with this idea by yourself. And that will make the change more
traceable indeed.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-07 10:39       ` Kairui Song
@ 2026-11-07 12:04         ` Ridong Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Ridong Chen @ 2026-11-07 12:04 UTC (permalink / raw)
  To: Kairui Song
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Qi Zheng, Shakeel Butt,
	Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen



On 11/7/2026 6:39 PM, Kairui Song wrote:
> On Fri, Nov 6, 2026 at 9:44 AM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> On 11/6/2026 12:21 AM, Kairui Song wrote:
>>> On Thu, Nov 5, 2026 at 10:11 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>>>
>>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>>
>>>> folio_update_gen() and folio_set_lru_refs() always end with a
>>>> try_cmpxchg() on folio->flags, even when the new value equals the old
>>>> one. The locked instruction still takes the struct page cacheline in
>>>> exclusive state.
>>>>
>>>> That is common in practice: a folio mapped by many processes is visited
>>>> once per mm during a page table walk, and after the first visit it is
>>>> already in the youngest generation with refs cleared and PG_workingset
>>>> set. isolate_folio() and the eviction paths also call
>>>> folio_set_lru_refs(folio, 0) on folios whose refs are already zero.
>>>>
>>>> Break out of the loop when the flags would not change. The result is
>>>> the same as a successful no-op cmpxchg. The only difference is that the
>>>> full barrier implied by the locked instruction is gone on that path.
>>>> No caller relies on it: the eviction and isolation callers run under
>>>> the LRU lock, and the others (page table walks, folio_mark_accessed(),
>>>> lru_gen_clear_refs()) only care about the resulting flags value, which
>>>> is already in place. Note that lru_gen_inc_refs() already returns
>>>> without any atomic when refs is at LRU_REFS_MAX and the folio is in
>>>> the workingset.
>>>>
>>>> Tested on an i7-13700HX with turbo disabled and the performance
>>>> governor. N processes map the same 2GiB tmpfs file (huge=never) and
>>>> read every page, then one aging pass is triggered on their memcg
>>>> through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
>>>> Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
>>>> over 2 boots of each kernel:
>>>>
>>>>     processes    base   patched
>>>>             1    32.9      31.7   -3.5%
>>>>             8   195.1     160.2  -17.9%
>>>>            32   755.8     592.2  -21.7%
>>>>            64  1597.2    1260.6  -21.1%
>>>>
>>>> With one process, every visit changes the generation, so there is
>>>> nothing to skip. Before the patch, perf annotate attributes 53-58% of
>>>> the walk_update_folio() samples to the lock cmpxchg in the inlined
>>>> folio_update_gen() in this test.
>>>>
>>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>>> ---
>>>>    include/linux/mm_inline.h | 2 ++
>>>>    mm/vmscan.c               | 2 ++
>>>>    2 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
>>>> index b3d85fe91385..4ba9079f4dc8 100644
>>>> --- a/include/linux/mm_inline.h
>>>> +++ b/include/linux/mm_inline.h
>>>> @@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
>>>>           do {
>>>>                   new_flags = old_flags;
>>>>                   lru_set_refs_flags(&new_flags, refs);
>>>> +               if (new_flags == old_flags)
>>>> +                       break;
>>>>           } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>>>>    }
>>>>
>>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>>> index 50c352e9ffef..3fa76715382d 100644
>>>> --- a/mm/vmscan.c
>>>> +++ b/mm/vmscan.c
>>>> @@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
>>>>                   lru_set_gen_flags(&new_flags, new_gen);
>>>>                   lru_set_refs_flags(&new_flags, 0);
>>>>                   new_flags |= BIT(PG_workingset);
>>>> +               if (new_flags == old_flags)
>>>> +                       break;
>>>>           } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>>>
>>> Hi Ridong,
>>>
>>> I had the same idea in V4 of MGLRU-FG which is posted a few weeks ago,
>>> with the same code:
>>> https://lore.kernel.org/linux-mm/20261016-mglru-fg-v4-4-2106750c0bfa@tencent.com/
>>>
>>> + if (new_flags == old_flags)
>>> + break;
>>>
>>
>> Hi Kairui,
>>
>> Oh. Sorry about that. I read the series a few weeks ago, but as it is large
>> I missed this change. I should have checked it again before sending.
>>
>>> I didn't have time to benchmark that one function but the e2e test
>>> showed very trivial improvement, it's a good improvement though. Feel
>>> free to add:
>>>
>>> Reviewed-by: Kairui Song <kasong@tencent.com>
>>>
>>
>> Thanks for the Reviewed-by. If you don't mind, I'd suggest merging it as
>> a separate patch, as the test shows the benefit of this small change,
>> which makes the code easier to trace. I will also mention in the commit
>> message that it is a small part of your series. Does that sound OK to
>> you?
> 
> Sounds good to me! No need to mention that, though; I believe you came
> up with this idea by yourself. And that will make the change more
> traceable indeed.

That's very kind of you. Thanks.

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-05 13:22 ` [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes Ridong Chen
  2026-11-05 16:21   ` Kairui Song
@ 2026-11-08  8:50   ` Barry Song
  2026-11-08 11:49     ` Ridong Chen
  1 sibling, 1 reply; 12+ messages in thread
From: Barry Song @ 2026-11-08  8:50 UTC (permalink / raw)
  To: Ridong Chen
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen

On Thu, Nov 5, 2026 at 9:23 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>
> From: Ridong Chen <chenridong@xiaomi.com>
>
> folio_update_gen() and folio_set_lru_refs() always end with a
> try_cmpxchg() on folio->flags, even when the new value equals the old
> one. The locked instruction still takes the struct page cacheline in
> exclusive state.
>
> That is common in practice: a folio mapped by many processes is visited
> once per mm during a page table walk, and after the first visit it is
> already in the youngest generation with refs cleared and PG_workingset
> set. isolate_folio() and the eviction paths also call
> folio_set_lru_refs(folio, 0) on folios whose refs are already zero.
>
> Break out of the loop when the flags would not change. The result is
> the same as a successful no-op cmpxchg. The only difference is that the
> full barrier implied by the locked instruction is gone on that path.

Hi Ridong,

I'm not quite sure this is an accurate description. The
atomic operation checks whether folio_flags(folio, 0)
has changed. If it has, we need to recompute new_flags
based on the updated old_flags.

For example:

CPU 0                          CPU 1
-----                          -----
read old_flags = A
                               change flags A -> B
new_flags = A
new_flags == old_flags
break

This is how the CAS loop works. Whether we strictly
need the guarantees provided by CAS is a separate
question :-)

But skipping the CAS logic might slightly underestimate a
folio's hotness if it gets promoted twice? Perhaps the
race window is so small that the reduced CAS overhead
outweighs the impact on hotness tracking.

> No caller relies on it: the eviction and isolation callers run under
> the LRU lock, and the others (page table walks, folio_mark_accessed(),
> lru_gen_clear_refs()) only care about the resulting flags value, which
> is already in place. Note that lru_gen_inc_refs() already returns
> without any atomic when refs is at LRU_REFS_MAX and the folio is in
> the workingset.
>
> Tested on an i7-13700HX with turbo disabled and the performance
> governor. N processes map the same 2GiB tmpfs file (huge=never) and
> read every page, then one aging pass is triggered on their memcg
> through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
> Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
> over 2 boots of each kernel:
>
>   processes    base   patched
>           1    32.9      31.7   -3.5%
>           8   195.1     160.2  -17.9%
>          32   755.8     592.2  -21.7%
>          64  1597.2    1260.6  -21.1%

Could we post the microbenchmark code here if it's
short enough?

>
> With one process, every visit changes the generation, so there is
> nothing to skip. Before the patch, perf annotate attributes 53-58% of
> the walk_update_folio() samples to the lock cmpxchg in the inlined
> folio_update_gen() in this test.
>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>  include/linux/mm_inline.h | 2 ++
>  mm/vmscan.c               | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index b3d85fe91385..4ba9079f4dc8 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
>         do {
>                 new_flags = old_flags;
>                 lru_set_refs_flags(&new_flags, refs);
> +               if (new_flags == old_flags)
> +                       break;
>         } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>  }
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 50c352e9ffef..3fa76715382d 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
>                 lru_set_gen_flags(&new_flags, new_gen);
>                 lru_set_refs_flags(&new_flags, 0);
>                 new_flags |= BIT(PG_workingset);
> +               if (new_flags == old_flags)
> +                       break;
>         } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>
>         *type = folio_flags_is_file_lru(&old_flags);
> --
> 2.34.1
>

Thanks
Barry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines
  2026-11-05 13:22 ` [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines Ridong Chen
@ 2026-11-08  9:14   ` Barry Song
  2026-11-08 12:49     ` Ridong Chen
  0 siblings, 1 reply; 12+ messages in thread
From: Barry Song @ 2026-11-08  9:14 UTC (permalink / raw)
  To: Ridong Chen
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen

On Thu, Nov 5, 2026 at 9:23 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>
> From: Ridong Chen <chenridong@xiaomi.com>
>
> lru_gen_eviction() and lru_gen_refault() update lrugen->evicted[] and
> lrugen->refaulted[] locklessly with atomic_long_add(). The arrays sit at
> the tail of struct lru_gen_folio, right before enabled/gen/seg/list:
> with LRU_GEN_STATS=n the file type counters share a cacheline with them.

So basically, this is causing false sharing, right?

I'd prefer a text diagram showing how the false sharing
occurs, specifically how freq read and freq write end up
in the same cacheline.

This will help clarify why rearranging the struct is the
right approach and how to rearrange it properly.

>
> Those fields are read far more often than they are written:
> lru_gen_add_folio() reads ->enabled on every LRU add, and shrink_many()
> walks ->list and reads ->gen/->seg under RCU. With frequent file refaults
> from several CPUs, every refault invalidates that line on the CPUs doing
> LRU adds.
>
> Move enabled/gen/seg/list next to max_seq/min_seq/timestamps, which are
> also mostly read, and put evicted[]/refaulted[] in their own cacheline
> group at the end of the struct. This grows struct lru_gen_folio from
> 1360 to 1408 bytes and struct mem_cgroup_per_node from 2176 to 2240

Why is the difference exactly 2240 - 2176 = 64 bytes,
which is one cacheline? I was expecting a value smaller
than one cacheline :-)

What I'm not convinced about is that nr_pages should be
updated even more frequently than evicted and refaulted.
Why aren't they causing a false sharing?

long nr_pages[MAX_NR_GENS][ANON_AND_FILE][MAX_NR_ZONES];

It would be better to have a text diagram showing the
potential false sharing, so we can consider the issue
more comprehensively.

> bytes on x86_64 (one more cacheline per memcg per node).
>
> Tested on an i7-13700HX (24 threads) with turbo disabled and the
> performance governor. 24 files of 128MiB on ext4 on brd, read with
> fio (psync, randread, bs=4k, fadvise_hint=random, buffered), all jobs
> in one memcg with memory.max=1G, so nearly every read refaults.
> IOPS, median of 5 runs:
>
>   jobs      base   patched
>      1    270537    274303   +1.4%  (sd 3.7%)
>      8   1461025   1466293   +0.4%
>     16   2455629   2508001   +2.1%
>     24   2707720   2823776   +4.3%
>
> A custom random pread() test of the same setup, with this patch alone
> against base over two boots of each kernel, shows +2.4% at 16 threads
> and +5.9% at 24 threads, and no change at 8 threads or fewer. With
> anonymous memory on zram, the refaults mostly hit the tier 0 counters
> on a different line, and there is no measurable change.
>
> The gain grows with the number of CPUs refaulting into the same lruvec
> and needs a fast backing store; with real IO it is likely to be lost in
> the noise. The patch is mostly meant to keep the lockless counters away
> from the read-mostly fields regardless of where they end up.
>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>  include/linux/mmzone.h | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>

Thanks,
Barry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes
  2026-11-08  8:50   ` Barry Song
@ 2026-11-08 11:49     ` Ridong Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Ridong Chen @ 2026-11-08 11:49 UTC (permalink / raw)
  To: Barry Song
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen



On 11/8/2026 4:50 PM, Barry Song wrote:
> On Thu, Nov 5, 2026 at 9:23 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> folio_update_gen() and folio_set_lru_refs() always end with a
>> try_cmpxchg() on folio->flags, even when the new value equals the old
>> one. The locked instruction still takes the struct page cacheline in
>> exclusive state.
>>
>> That is common in practice: a folio mapped by many processes is visited
>> once per mm during a page table walk, and after the first visit it is
>> already in the youngest generation with refs cleared and PG_workingset
>> set. isolate_folio() and the eviction paths also call
>> folio_set_lru_refs(folio, 0) on folios whose refs are already zero.
>>
>> Break out of the loop when the flags would not change. The result is
>> the same as a successful no-op cmpxchg. The only difference is that the
>> full barrier implied by the locked instruction is gone on that path.
> 
> Hi Ridong,
> 
> I'm not quite sure this is an accurate description. The

You are right, "the same as a successful no-op cmpxchg" is not
accurate. A better way to put it: when we break, the flags we read
are already the target value, so the update can be considered done
at the time of that read, and any concurrent change is ordered after
it. The current code gives the same result whenever its cmpxchg wins
the race, so the patch makes no new state reachable. What it drops is
only the retry, which recomputes from the new value and overwrites
the concurrent change with what this CPU wanted.

> atomic operation checks whether folio_flags(folio, 0)
> has changed. If it has, we need to recompute new_flags
> based on the updated old_flags.
> 
> For example:
> 
> CPU 0                          CPU 1
> -----                          -----
> read old_flags = A
>                                 change flags A -> B
> new_flags = A
> new_flags == old_flags
> break
> 
> This is how the CAS loop works. Whether we strictly
> need the guarantees provided by CAS is a separate
> question :-)
> 
> But skipping the CAS logic might slightly underestimate a
> folio's hotness if it gets promoted twice? Perhaps the
> race window is so small that the reduced CAS overhead
> outweighs the impact on hotness tracking.
> 

I think it goes the other way. For folio_update_gen(), take two
walkers where CPU 0 read an older max_seq:

CPU 0 (new_gen = N)            CPU 1 (new_gen = N+1)
read flags: gen N
				set gen N+1
current: cmpxchg fails, retry writes gen N back
patched: break, folio stays in gen N+1

So the retry moves the folio back to the older generation, and the
patch keeps the second promotion. Similarly for folio_set_lru_refs():
if CPU 1 increments refs from 0 to 1 while CPU 0 is setting it to 0,
the retry would clear that access, and the patch keeps it.

I will reword the changelog in v2 along these lines.

>> No caller relies on it: the eviction and isolation callers run under
>> the LRU lock, and the others (page table walks, folio_mark_accessed(),
>> lru_gen_clear_refs()) only care about the resulting flags value, which
>> is already in place. Note that lru_gen_inc_refs() already returns
>> without any atomic when refs is at LRU_REFS_MAX and the folio is in
>> the workingset.
>>
>> Tested on an i7-13700HX with turbo disabled and the performance
>> governor. N processes map the same 2GiB tmpfs file (huge=never) and
>> read every page, then one aging pass is triggered on their memcg
>> through /sys/kernel/debug/lru_gen ("+ <memcg_id> 0 <max_seq> 200 1").
>> Time per aging pass in ms, median of 5 runs of 10 passes each, averaged
>> over 2 boots of each kernel:
>>
>>    processes    base   patched
>>            1    32.9      31.7   -3.5%
>>            8   195.1     160.2  -17.9%
>>           32   755.8     592.2  -21.7%
>>           64  1597.2    1260.6  -21.1%
> 
> Could we post the microbenchmark code here if it's
> short enough?
> 

Sure, it is below. Run it as root in a dedicated cgroup v2 memcg,
on a tmpfs mounted with huge=never:

./p4age /mnt/tmpfs/f 2048 <nproc> 10

```
/*
  * P4 load: MGLRU aging over a file mapped by many processes.
  *
  * Creates <MiB> tmpfs file at <path> (mount it with huge=never so every
  * folio is a single page), forks <nproc> children that each map it
  * MAP_SHARED, then for <rounds> rounds:
  *   1. every child reads one byte per page (sets the accessed bit in its
  *      own page tables)
  *   2. the parent triggers one aging pass on its memcg through
  *      /sys/kernel/debug/lru_gen ("+ memcg_id nid max_seq 200 1") and times
  *      the write()
  *
  * The page table walk visits every page once per mm. The first visit moves
  * the folio to the youngest generation; the other nproc-1 visits find the
  * flags already at the target value, which is the no-op cmpxchg P4 skips.
  *
  * Run inside a dedicated cgroup v2 memcg, as root, with debugfs mounted.
  * Usage: p4age <path> <MiB> <nproc> <rounds>
  * Output: one "round=R ms=T" line per round, then a summary.
  */
#define _GNU_SOURCE
#include <fcntl.h>
#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/resource.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>

#define PAGE	4096UL
#define LRU_GEN	"/sys/kernel/debug/lru_gen"

struct shared {
	atomic_int round;	/* parent bumps to start a touch round */
	atomic_int done;	/* children finished the current round */
	atomic_int quit;
};

static struct shared *sh;

static double now(void)
{
	struct timespec t;

	clock_gettime(CLOCK_MONOTONIC, &t);
	return t.tv_sec + t.tv_nsec / 1e9;
}

static void child(int fd, size_t len)
{
	volatile char sink;
	int seen = 0;
	size_t i;
	char *p;

	p = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0);
	if (p == MAP_FAILED) {
		perror("mmap");
		_exit(1);
	}
	for (;;) {
		while (atomic_load(&sh->round) == seen && !atomic_load(&sh->quit))
			usleep(100);
		if (atomic_load(&sh->quit))
			break;
		seen = atomic_load(&sh->round);
		for (i = 0; i < len; i += PAGE)
			sink = p[i];
		atomic_fetch_add(&sh->done, 1);
	}
	(void)sink;
	_exit(0);
}

/* find "memcg <id> <path>" for our cgroup, then max_seq of node 0 */
static int lookup(const char *cg, unsigned long long *id, unsigned long *max_seq)
{
	char line[4096], path[4096];
	int found = 0, in_node = 0;
	unsigned long long mid;
	unsigned long seq;
	FILE *f = fopen(LRU_GEN, "r");

	if (!f) {
		perror(LRU_GEN);
		return -1;
	}
	while (fgets(line, sizeof(line), f)) {
		if (sscanf(line, "memcg %llu %4095s", &mid, path) == 2) {
			if (found)
				break;
			found = !strcmp(path, cg);
			if (found)
				*id = mid;
			in_node = 0;
			continue;
		}
		if (!found)
			continue;
		if (sscanf(line, " node %d", &in_node) == 1) {
			in_node = in_node == 0 ? 1 : -1;
			continue;
		}
		if (in_node == 1 && sscanf(line, " %lu", &seq) == 1)
			*max_seq = seq;
	}
	fclose(f);
	return found ? 0 : -1;
}

static void own_cgroup(char *buf, size_t sz)
{
	FILE *f = fopen("/proc/self/cgroup", "r");

	/* cgroup v2: "0::/path" */
	if (!f || !fgets(buf, sz, f) || strncmp(buf, "0::", 3)) {
		fprintf(stderr, "need cgroup v2\n");
		exit(1);
	}
	fclose(f);
	memmove(buf, buf + 3, strlen(buf + 3) + 1);
	buf[strcspn(buf, "\n")] = 0;
}

int main(int argc, char **argv)
{
	double t, sum = 0, mn = 1e30, mx = 0, tw;
	unsigned long long id;
	unsigned long max_seq;
	int nproc, rounds, fd, r, i, dfd;
	char cg[4096], cmd[128];
	size_t len, off;
	char *buf;

	if (argc < 5) {
		fprintf(stderr, "usage: %s <path> <MiB> <nproc> <rounds>\n", argv[0]);
		return 1;
	}
	len = strtoul(argv[2], NULL, 0) << 20;
	nproc = atoi(argv[3]);
	rounds = atoi(argv[4]);
	own_cgroup(cg, sizeof(cg));

	fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
	if (fd < 0 || ftruncate(fd, len)) {
		perror(argv[1]);
		return 1;
	}
	/* populate the page cache (charged to our memcg) */
	buf = calloc(1, 1 << 20);
	memset(buf, 0x5a, 1 << 20);
	for (off = 0; off < len; off += 1 << 20)
		if (pwrite(fd, buf, 1 << 20, off) != 1 << 20) {
			perror("pwrite");
			return 1;
		}

	sh = mmap(NULL, sizeof(*sh), PROT_READ | PROT_WRITE,
		  MAP_SHARED | MAP_ANONYMOUS, -1, 0);
	for (i = 0; i < nproc; i++)
		if (fork() == 0)
			child(fd, len);

	dfd = open(LRU_GEN, O_WRONLY);
	if (dfd < 0) {
		perror(LRU_GEN);
		return 1;
	}

	/* round 0 is warm-up: first touch, folios gain PG_workingset */
	for (r = 0; r <= rounds; r++) {
		atomic_store(&sh->done, 0);
		atomic_fetch_add(&sh->round, 1);
		while (atomic_load(&sh->done) < nproc)
			usleep(100);

		if (lookup(cg, &id, &max_seq)) {
			fprintf(stderr, "memcg %s not in %s\n", cg, LRU_GEN);
			return 1;
		}
		/* swappiness 200: walk anon (tmpfs) too; force_scan=1 */
		snprintf(cmd, sizeof(cmd), "+ %llu 0 %lu 200 1\n", id, max_seq);
		tw = now();
		if (write(dfd, cmd, strlen(cmd)) < 0) {
			perror("lru_gen write");
			return 1;
		}
		t = (now() - tw) * 1000;
		if (!r)
			continue;
		printf("round=%d max_seq=%lu ms=%.2f\n", r, max_seq, t);
		sum += t;
		mn = t < mn ? t : mn;
		mx = t > mx ? t : mx;
	}

	atomic_store(&sh->quit, 1);
	while (wait(NULL) > 0)
		;
	close(fd);
	unlink(argv[1]);
	printf("p4age nproc=%d MiB=%zu rounds=%d avg_ms=%.2f min_ms=%.2f max_ms=%.2f\n",
	       nproc, len >> 20, rounds, sum / rounds, mn, mx);
	return 0;
}

```

>>
>> With one process, every visit changes the generation, so there is
>> nothing to skip. Before the patch, perf annotate attributes 53-58% of
>> the walk_update_folio() samples to the lock cmpxchg in the inlined
>> folio_update_gen() in this test.
>>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   include/linux/mm_inline.h | 2 ++
>>   mm/vmscan.c               | 2 ++
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
>> index b3d85fe91385..4ba9079f4dc8 100644
>> --- a/include/linux/mm_inline.h
>> +++ b/include/linux/mm_inline.h
>> @@ -244,6 +244,8 @@ static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs)
>>          do {
>>                  new_flags = old_flags;
>>                  lru_set_refs_flags(&new_flags, refs);
>> +               if (new_flags == old_flags)
>> +                       break;
>>          } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>>   }
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 50c352e9ffef..3fa76715382d 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -3398,6 +3398,8 @@ static int folio_update_gen(struct folio *folio, int new_gen, int *type,
>>                  lru_set_gen_flags(&new_flags, new_gen);
>>                  lru_set_refs_flags(&new_flags, 0);
>>                  new_flags |= BIT(PG_workingset);
>> +               if (new_flags == old_flags)
>> +                       break;
>>          } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>>
>>          *type = folio_flags_is_file_lru(&old_flags);
>> --
>> 2.34.1
>>
> 
> Thanks
> Barry

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines
  2026-11-08  9:14   ` Barry Song
@ 2026-11-08 12:49     ` Ridong Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Ridong Chen @ 2026-11-08 12:49 UTC (permalink / raw)
  To: Barry Song
  Cc: David Hildenbrand, Andrew Morton, Johannes Weiner,
	Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Kairui Song, Qi Zheng,
	Shakeel Butt, Axel Rasmussen, Yuanchu Xie, Wei Xu, Baoquan He,
	Baolin Wang, Joanne Koong, linux-mm, linux-kernel, Ridong Chen



On 11/8/2026 5:14 PM, Barry Song wrote:
> On Thu, Nov 5, 2026 at 9:23 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> lru_gen_eviction() and lru_gen_refault() update lrugen->evicted[] and
>> lrugen->refaulted[] locklessly with atomic_long_add(). The arrays sit at
>> the tail of struct lru_gen_folio, right before enabled/gen/seg/list:
>> with LRU_GEN_STATS=n the file type counters share a cacheline with them.
> 

Hi Barry,

Thanks for the review. Note that the sizes in the changelog were
measured with lockdep enabled.

Offsets below are from pahole on x86_64, LRU_GEN_STATS=n, counted
from the start of struct mem_cgroup_per_node (cacheline aligned), so
line N = bytes N*64 .. N*64+63.

The nesting is:
     mem_cgroup_per_node
       lruvec                at 128
         lrugen              at 128 + 176 = 304  (before the patch)
                             at 128 + 192 = 320  (after the patch)

> So basically, this is causing false sharing, right?
> 

Yes. Before the patch:

field                    offset       line
refaulted[0][file][*]    1608-1639    25    atomic, no lru_lock
enabled/gen/seg          1640-1642    25    read on every LRU add
list                     1648-1663    25    read by shrink_many()

line 25 (1600-1663)
+------------------------+-----------------+------+------------+
| refaulted[0][file][*]  | enabled,gen,seg | pad  | list       |
| 1608-1639              | 1640-1642       |      | 1648-1663  |
+------------------------+-----------------+------+------------+

CPU A (file refault)		CPU B (LRU add)
atomic_long_add(&refaulted[..])
				read lrugen->enabled -> miss
atomic_long_add(&refaulted[..])
				read lrugen->enabled -> miss

CPU B doesn't need refaulted[], but it keeps missing on line 25.

After the patch:

field                    offset       line
max_seq/min_seq/ts       320-375      5
enabled/gen/seg          376-378      5     read-mostly
list                     384-399      6
nr_pages[]               1040-1359    16-21 under lru_lock
evicted[]                1600-1663    25    own line
refaulted[]              1664-1727    26    own line

> I'd prefer a text diagram showing how the false sharing
> occurs, specifically how freq read and freq write end up
> in the same cacheline.
> 
> This will help clarify why rearranging the struct is the
> right approach and how to rearrange it properly.
> 
>>
>> Those fields are read far more often than they are written:
>> lru_gen_add_folio() reads ->enabled on every LRU add, and shrink_many()
>> walks ->list and reads ->gen/->seg under RCU. With frequent file refaults
>> from several CPUs, every refault invalidates that line on the CPUs doing
>> LRU adds.
>>
>> Move enabled/gen/seg/list next to max_seq/min_seq/timestamps, which are
>> also mostly read, and put evicted[]/refaulted[] in their own cacheline
>> group at the end of the struct. This grows struct lru_gen_folio from
>> 1360 to 1408 bytes and struct mem_cgroup_per_node from 2176 to 2240
> 
> Why is the difference exactly 2240 - 2176 = 64 bytes,
> which is one cacheline? I was expecting a value smaller
> than one cacheline :-)
> 

The aligned group makes lru_gen_folio cacheline aligned, and the
padding adds up on the way out:

lru_gen_folio         1360 -> 1408  (+48, hole before evicted[])
lruvec                1624 -> 1728  (+104, lrugen moves from offset
				     176 to 192, plus tail padding to 64)
mem_cgroup_per_node   2048 -> 2112  (+64)

mem_cgroup_per_node is cacheline aligned, so its size only changes
in whole lines. It had 40 bytes of padding after lruvec, which
absorbs part of the 104 bytes, and the rest takes one more line.


> What I'm not convinced about is that nr_pages should be
> updated even more frequently than evicted and refaulted.
> Why aren't they causing a false sharing?
> 

nr_pages[] (lines 16-21) is read and written only under lru_lock.
Its lines move between CPUs only when the lock does, and the new
lock holder needs the updated values anyway, so that is true
sharing. refaulted[] is written by CPUs that don't hold the lock,
so it can steal the line from the lock holder at any time, and the
lock holder doesn't even need it.

> long nr_pages[MAX_NR_GENS][ANON_AND_FILE][MAX_NR_ZONES];
> 
> It would be better to have a text diagram showing the
> potential false sharing, so we can consider the issue
> more comprehensively.
> 
>> bytes on x86_64 (one more cacheline per memcg per node).
>>
>> Tested on an i7-13700HX (24 threads) with turbo disabled and the
>> performance governor. 24 files of 128MiB on ext4 on brd, read with
>> fio (psync, randread, bs=4k, fadvise_hint=random, buffered), all jobs
>> in one memcg with memory.max=1G, so nearly every read refaults.
>> IOPS, median of 5 runs:
>>
>>    jobs      base   patched
>>       1    270537    274303   +1.4%  (sd 3.7%)
>>       8   1461025   1466293   +0.4%
>>      16   2455629   2508001   +2.1%
>>      24   2707720   2823776   +4.3%
>>
>> A custom random pread() test of the same setup, with this patch alone
>> against base over two boots of each kernel, shows +2.4% at 16 threads
>> and +5.9% at 24 threads, and no change at 8 threads or fewer. With
>> anonymous memory on zram, the refaults mostly hit the tier 0 counters
>> on a different line, and there is no measurable change.
>>
>> The gain grows with the number of CPUs refaulting into the same lruvec
>> and needs a fast backing store; with real IO it is likely to be lost in
>> the noise. The patch is mostly meant to keep the lockless counters away
>> from the read-mostly fields regardless of where they end up.
>>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   include/linux/mmzone.h | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
> 
> Thanks,
> Barry

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-11-08 12:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-05 13:22 [RFC PATCH 0/3] mm/mglru: cacheline follow-ups on the reclaim hot paths Ridong Chen
2026-11-05 13:22 ` [RFC PATCH 1/3] mm/mglru: skip redundant bitmap store in lru_gen_use_mm() Ridong Chen
2026-11-05 13:22 ` [RFC PATCH 2/3] mm/mglru: skip the flags cmpxchg when nothing changes Ridong Chen
2026-11-05 16:21   ` Kairui Song
2026-11-06  1:43     ` Ridong Chen
2026-11-07 10:39       ` Kairui Song
2026-11-07 12:04         ` Ridong Chen
2026-11-08  8:50   ` Barry Song
2026-11-08 11:49     ` Ridong Chen
2026-11-05 13:22 ` [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines Ridong Chen
2026-11-08  9:14   ` Barry Song
2026-11-08 12:49     ` Ridong Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®