[PATCH v3 0/3] mm: workingset: fix the shadow node budget under MGLRU

[PATCH v3 0/3] mm: workingset: fix the shadow node budget under MGLRU

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

Commit 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the
number of lru pages") broke the workingset shadow node budget under
MGLRU: lruvec_lru_size() reads mz->lru_zone_size, which MGLRU never
maintains, so count_shadow_nodes() sees the evictable LRU lists as
empty and the shadow shrinker reclaims eviction tokens almost as fast
as they are created, losing thrashing protection.

Patch 1 extends the dying-mcg stat redirection (previously cgroup v1
only) to all hierarchies, addressing the reparenting race that motivated
7404bd37cfbe.

Patch 2 then switches count_shadow_nodes() back to
lruvec_page_state_local(), which both classic LRU and MGLRU maintain.

Patch 3 recovers the performance.  Patch 1 added an unconditional
rcu_read_lock() to the stat update fast path; patch 3 checks
memcg_is_dying() first and takes the RCU lock only on the rare dying
path.

Changes since v2:
 - Reorder the series: the dying-memcg redirection now comes first and
   the switch back to lruvec_page_state_local() follows it, as
   requested.
 - Patch 1: add the Fixes and Cc stable tags and describe the
   user-visible impact of the bug, as requested.
 - Collect Shakeel's Acked-by.

Performance testing
===================

The test script and the raw results are available at [1].

Environment: 10-vCPU QEMU guest, 8 GiB RAM, cgroup v2; 7 runs per
configuration, medians reported.  Workloads:

  w1-anon-churn: single-threaded anon fault/charge loop in a memcg
                 (MADV_DONTNEED + re-fault, no reclaim).  Every touch
                 is a real fault with charge and memcg stat updates,
                 so it stresses exactly the fast path patch 1 changes.
  w2-file-churn: file read loop under memory.high pressure
                 (reclaim-bound, noisier).
  w3-reparent:   reparent accounting sanity check.

w1-anon-churn (pages/s):

                 classic LRU          MGLRU
base             4393028              4377122
patches 1-2      4385996   (-0.2%)    4352887   (-0.6%)
patches 1-3      4381832   (-0.3%)    4377053   (+0.0%)

w2-file-churn (MB/s):

                 classic LRU          MGLRU
base             8277                 8226
patches 1-2      8226      (-0.6%)    8123      (-1.3%)
patches 1-3      8157      (-1.4%)    8294      (+0.8%)

w3-reparent passed on all kernels.

The small overhead visible with patches 1-2 comes from the redirection
added by patch 1; patch 3 brings w1 back to the base level in both LRU
configurations.  The remaining differences are within run-to-run noise.

[1] https://gist.github.com/teawater/32f373ec41d185d840455eb167321a5a

Changelog:
v3:
According to the comments of Shakeel, reorder the series per review,
add Fixes/Cc stable and the user-visible impact to patch 1.

Hui Zhu (3):
  mm: memcg: redirect stats updates of dying memcgs for all hierarchies
  mm: workingset: use lruvec_page_state_local() to count lru pages
  mm: memcg: skip the RCU lock when the memcg is not dying

 mm/memcontrol.c | 30 ++++++++++++------------------
 mm/workingset.c |  5 ++---
 2 files changed, 14 insertions(+), 21 deletions(-)

-- 
2.53.0

[PATCH v3 1/3] mm: memcg: redirect stats updates of dying memcgs for all hierarchies

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

get_non_dying_memcg_start() redirects the stat updates of a dying memcg to
its closest non-dying ancestor, but only on cgroup v1; on cgroup v2 the
stats keep being accounted to the dying memcg itself.

A later patch in this series restores lruvec_page_state_local() in
count_shadow_nodes() to fix the broken workingset shadow node budget
under MGLRU.  count_shadow_nodes() is the only reader of those
non-hierarchical state_locals on cgroup v2: when a memcg is offlined,
its pages are reparented to the ancestor but their stat updates keep
being accounted to the dying memcg, so count_shadow_nodes() computes a
wrong shadow node budget and workingset thrashing protection is lost.
This is user visible as premature reclaim of hot page cache and
degraded performance under memory pressure.  Apply the redirection to
all hierarchies to fix this.

Offlining is rare, so the added cost on the stat update fast path is
limited to an rcu_read_lock() and a css_is_dying() check; the upward
walk happens only while a memcg is dying.

Fixes: 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the number of lru pages")
Cc: stable@vger.kernel.org
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 mm/memcontrol.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8319ad8c5c23..b3d1ac3fe0aa 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -805,20 +805,14 @@ static long memcg_state_val_in_pages(int idx, long val)
 	return val < 0 ? -res : res;
 }
 
-#ifdef CONFIG_MEMCG_V1
 /*
- * Used in mod_memcg_state() and mod_memcg_lruvec_state() to avoid race with
- * reparenting of non-hierarchical state_locals.
+ * Used in mod_memcg_state() and mod_memcg_lruvec_state() to avoid race
+ * with reparenting of non-hierarchical state_locals.  Offlining a
+ * memcg is rare, so do the redirection for all cgroup hierarchies.
  */
-static inline struct mem_cgroup *get_non_dying_memcg_start(struct mem_cgroup *memcg,
-							   bool *rcu_locked)
+static inline struct mem_cgroup *
+get_non_dying_memcg_start(struct mem_cgroup *memcg, bool *rcu_locked)
 {
-	/* Rebinding can cause this value to be changed at runtime */
-	if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
-		*rcu_locked = false;
-		return memcg;
-	}
-
 	rcu_read_lock();
 	*rcu_locked = true;
 
@@ -830,22 +824,8 @@ static inline struct mem_cgroup *get_non_dying_memcg_start(struct mem_cgroup *me
 
 static inline void get_non_dying_memcg_end(bool rcu_locked)
 {
-	if (!rcu_locked)
-		return;
-
 	rcu_read_unlock();
 }
-#else
-static inline struct mem_cgroup *get_non_dying_memcg_start(struct mem_cgroup *memcg,
-							   bool *rcu_locked)
-{
-	return memcg;
-}
-
-static inline void get_non_dying_memcg_end(bool rcu_locked)
-{
-}
-#endif
 
 static void __mod_memcg_state(struct mem_cgroup *memcg,
 			      enum memcg_stat_item idx, long val)
-- 
2.53.0

Re: [PATCH v3 1/3] mm: memcg: redirect stats updates of dying memcgs for all hierarchies

[Hui Zhu]

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Redirecting dying memcg stat updates to the parent on cgroup v2 without reparenting the base stats causes permanent negative stat leaks and underflows.
> --
>
> commit 106b19246ac20191c0af6a41927414095cb5d019
> Author: Hui Zhu <zhuhui@kylinos.cn>
>
> mm: memcg: redirect stats updates of dying memcgs for all hierarchies
>
> This patch removes the cgroup v1 restriction on redirecting dying memcg stat
> updates to the closest non-dying ancestor. It unconditionally applies this
> redirection across all hierarchies in an effort to fix workingset shadow node
> budget calculations under MGLRU.
>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 9a65d7148c227..64eb59bba61cf 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
> [ ... ]
>
>> -static inline struct mem_cgroup *get_non_dying_memcg_start(struct mem_cgroup *memcg,
>> -							   bool *rcu_locked)
>> +static inline struct mem_cgroup *
>> +get_non_dying_memcg_start(struct mem_cgroup *memcg, bool *rcu_locked)
>>   {
>> -	/* Rebinding can cause this value to be changed at runtime */
>> -	if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) {
>> -		*rcu_locked = false;
>> -		return memcg;
>> -	}
>> -
>>   	rcu_read_lock();
>>   	*rcu_locked = true;
>>   
>> @@ -870,22 +864,8 @@ static inline struct mem_cgroup *get_non_dying_memcg_start(struct mem_cgroup *me
> [Severity: High]
> Does this redirection cause permanent stat underflows in the ancestor under
> cgroup v2?
>
> In cgroup v2, base local stats are explicitly not reparented during offlining.
> Looking at reparent_state_local():
>
>      if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
>          return;
>
> Because of this early return, the dying memcg retains its positive base counts.
>
> However, with this patch, when resources previously charged to the dying memcg
> are freed later on, the negative delta applied in mod_memcg_state() is
> redirected up to the parent.
>
> Since the non-dying ancestor never received the positive base counts but now
> receives the negative uncharge updates, its local stats (like NR_FILE_PAGES,
> MEMCG_SOCK, and LRU counts) will permanently underflow.
>
> Because lruvec_page_state_local() clamps negative values to zero, wouldn't
> this severe underflow mask the ancestor's own legitimate positive stats,
> ultimately breaking the workingset shadow node budget calculation and causing
> the exact premature reclaim and thrashing this patch aims to resolve?
>
The observation is correct: on cgroup v2 reparent_state_local() returns
early, so the dying memcg keeps its positive base counts while the
negative uncharge deltas land on the ancestor, and the ancestor's
state_local can permanently underflow and get clamped to zero.

One clarification on attribution, though: this underflow is not
introduced by the redirection.  Once memcg_reparent_objcgs() rewrites
objcg->memcg to the parent, folio_memcg() of the reparented folios
already returns the parent, so the freeing path applies the negative
deltas to the parent's lruvec directly, with or without this patch.
The redirection only matters during the short window between
css_offline() and the objcg reparenting.  So the missing base reparent
is a pre-existing gap that this series re-exposes rather than creates.

That said, I agree it needs to be handled.  I'm preparing a follow-up
patch that also reparents the non-hierarchical lruvec state_locals
(the ones count_shadow_nodes() reads) on cgroup v2, mirroring what v1
already does.  Will fold it into the next version.

Best,
Hui

[PATCH v3 2/3] mm: workingset: use lruvec_page_state_local() to count lru pages

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

Commit 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the
number of lru pages") switched count_shadow_nodes() to lruvec_lru_size().
With CONFIG_MEMCG enabled, lruvec_lru_size() reads mz->lru_zone_size,
which only the classic LRU paths maintain.  MGLRU accounts its pages
through __update_lru_size(), which skips that array, so with MGLRU on the
four evictable LRU lists are always seen as empty.  The shadow node budget
(pages >> 3) then collapses to slab plus unevictable pages, and the
workingset shadow shrinker reclaims eviction tokens almost as fast as they
are created, losing thrashing protection.

lruvec_page_state_local() reads lruvec_stats->state_local instead, which
both classic LRU and MGLRU maintain.  Switch back to it.  The reparenting
race this re-exposes on cgroup v2 is closed by the follow-up patch that
redirects dying-memcg stat updates for all hierarchies.

Fixes: 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the number of lru pages")
Cc: stable@vger.kernel.org
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 mm/workingset.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/workingset.c b/mm/workingset.c
index f351798e723a..85a4e14e95d5 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -693,10 +693,9 @@ static unsigned long count_shadow_nodes(struct shrinker *shrinker,
 
 		mem_cgroup_flush_stats_ratelimited(sc->memcg);
 		lruvec = mem_cgroup_lruvec(sc->memcg, NODE_DATA(sc->nid));
-
 		for (pages = 0, i = 0; i < NR_LRU_LISTS; i++)
-			pages += lruvec_lru_size(lruvec, i, MAX_NR_ZONES - 1);
-
+			pages += lruvec_page_state_local(lruvec,
+							 NR_LRU_BASE + i);
 		pages += lruvec_page_state_local(
 			lruvec, NR_SLAB_RECLAIMABLE_B) >> PAGE_SHIFT;
 		pages += lruvec_page_state_local(
-- 
2.53.0

Re: [PATCH v3 2/3] mm: workingset: use lruvec_page_state_local() to count lru pages

[Andrew Morton]

On Fri,  4 Sep 2026 17:45:55 +0800 "Hui Zhu" <hui.zhu@linux.dev> wrote:

> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> Commit 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the
> number of lru pages") switched count_shadow_nodes() to lruvec_lru_size().
> With CONFIG_MEMCG enabled, lruvec_lru_size() reads mz->lru_zone_size,
> which only the classic LRU paths maintain.  MGLRU accounts its pages
> through __update_lru_size(), which skips that array, so with MGLRU on the
> four evictable LRU lists are always seen as empty.  The shadow node budget
> (pages >> 3) then collapses to slab plus unevictable pages, and the
> workingset shadow shrinker reclaims eviction tokens almost as fast as they
> are created, losing thrashing protection.
> 
> lruvec_page_state_local() reads lruvec_stats->state_local instead, which
> both classic LRU and MGLRU maintain.  Switch back to it.  The reparenting
> race this re-exposes on cgroup v2 is closed by the follow-up patch that
> redirects dying-memcg stat updates for all hierarchies.

The follow-up patch is "mm: memcg: skip the RCU lock when the memcg is
not dying"?  But that's an optimization so I'm confused.

If we're re-exposing a race, the fix for that race should have the same
Fixes: and cc:stable as the commit which did the reexposure?

It isn't clear why any of these patches is cc:stable.  The overall
effect is a tiny performance improvement?  Very clear descriptions of
end-user effects are always helpful.

So at this time I'll schedule the whole series for 7.4-rc1.  If there's
some reason why some/all of these should be backported then please lmk.

Re: [PATCH v3 2/3] mm: workingset: use lruvec_page_state_local() to count lru pages

[Hui Zhu]

> On Fri,  4 Sep 2026 17:45:55 +0800 "Hui Zhu" <hui.zhu@linux.dev> wrote:
>
>> From: Hui Zhu <zhuhui@kylinos.cn>
>>
>> Commit 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the
>> number of lru pages") switched count_shadow_nodes() to lruvec_lru_size().
>> With CONFIG_MEMCG enabled, lruvec_lru_size() reads mz->lru_zone_size,
>> which only the classic LRU paths maintain.  MGLRU accounts its pages
>> through __update_lru_size(), which skips that array, so with MGLRU on the
>> four evictable LRU lists are always seen as empty.  The shadow node budget
>> (pages >> 3) then collapses to slab plus unevictable pages, and the
>> workingset shadow shrinker reclaims eviction tokens almost as fast as they
>> are created, losing thrashing protection.
>>
>> lruvec_page_state_local() reads lruvec_stats->state_local instead, which
>> both classic LRU and MGLRU maintain.  Switch back to it.  The reparenting
>> race this re-exposes on cgroup v2 is closed by the follow-up patch that
>> redirects dying-memcg stat updates for all hierarchies.
> The follow-up patch is "mm: memcg: skip the RCU lock when the memcg is
> not dying"?  But that's an optimization so I'm confused.

Sorry for the confusion - the commit message still says "follow-up
patch" because that was the position in v2.  In v3 the series was
reordered as requested, so the patch that closes the race, "mm: memcg:
redirect stats updates of dying memcgs for all hierarchies", is now
patch 1 and comes before this one.  Patch 3 is indeed only an
optimization.  v4 will reword this sentence to:

The reparenting race this re-exposes on cgroup v2 is closed by the
preceding patch that redirects dying-memcg stat updates for all
hierarchies.

> If we're re-exposing a race, the fix for that race should have the same
> Fixes: and cc:stable as the commit which did the reexposure?

Yes, and it does.  Patch 1 carries exactly the same tags as this
patch:
Fixes: 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get the 
number of lru pages")
Cc: stable@vger.kernel.org
Patch 3 has no tags because it only recovers the fast-path overhead
that patch 1 adds; it fixes no bug by itself.

> It isn't clear why any of these patches is cc:stable.  The overall
> effect is a tiny performance improvement?  Very clear descriptions of
> end-user effects are always helpful.
The performance table in the cover letter measures the overhead that
the patches themselves add to the stat update fast path, not the
impact of the bug - sorry if that was misleading.

The end-user effect of the bug is the loss of thrashing protection.
Since 7404bd37cfbe, with MGLRU enabled count_shadow_nodes() sees the
four evictable LRU lists as always empty, so the workingset shadow
node budget collapses to slab plus unevictable pages: for a memcg
holding 1 GiB of page cache and 64 MiB of slab the budget drops from
~35k nodes to ~2k.  The shadow shrinker then reclaims eviction tokens
almost as fast as they are created, so a refault finds a live page
instead of a shadow entry and lru_gen_refault() cannot restore the
workingset state of the refaulting page.  Hot file pages that should
be protected are evicted again and re-read from disk.  In other words,
workloads that refault page cache under memory pressure thrash, with
visibly increased IO and latency.  This hits every memcg reclaim and,
since the root memcg is iterated as well, global reclaim too.
> So at this time I'll schedule the whole series for 7.4-rc1.  If there's
> some reason why some/all of these should be backported then please lmk.
>
I would suggest backporting all three patches together to the trees
that contain 7404bd37cfbe: it went into v7.1, so 7.1.y and 7.2.y
carry the regression.  Taking patch 3 along with patches 1 and 2
avoids leaving the ~0.6% fast-path overhead of patch 1 in the stable
trees.

So v4 will reword the reference in this patch's commit message ("the
preceding patch" instead of "the follow-up patch").  No code changes.

One question: should patch 3 ("mm: memcg: skip the RCU lock when the
memcg is not dying") also carry the same

   Fixes: 7404bd37cfbe ("mm: workingset: use lruvec_lru_size() to get 
the number of lru pages")
   Cc: stable@vger.kernel.org

tags?  That would make the stable trees pick up the whole series
together, but since patch 3 only recovers the overhead of patch 1 and
fixes no bug by itself, a Fixes: tag on it may look odd.  Please let
me know your preference.

Best,
Hui

[PATCH v3 3/3] mm: memcg: skip the RCU lock when the memcg is not dying

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

get_non_dying_memcg_start() takes rcu_read_lock() on every stat update, but
the lock only protects the upward walk to a non-dying ancestor, which
happens solely while a memcg is being offlined.  The dying check itself
reads the CSS_DYING flag of a memcg the caller already holds a reference
to, so it is safe without the lock.

Check memcg_is_dying() first and return immediately when the memcg is
alive, taking the RCU lock only on the rare dying path.  On an anon
fault/charge churn workload in a memcg this recovers the ~0.6% overhead
added by the previous patch (4368077 vs 4343159 pages/s before, back to
~4377000 pages/s after).

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 mm/memcontrol.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b3d1ac3fe0aa..f454d02746e9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -813,6 +813,17 @@ static long memcg_state_val_in_pages(int idx, long val)
 static inline struct mem_cgroup *
 get_non_dying_memcg_start(struct mem_cgroup *memcg, bool *rcu_locked)
 {
+	/*
+	 * Fast path: the caller holds a reference to @memcg, so reading
+	 * its CSS_DYING flag without the RCU lock is safe.  The RCU lock
+	 * is only needed to walk up to a non-dying ancestor, which
+	 * happens only while a memcg is actually being offlined.
+	 */
+	if (!memcg_is_dying(memcg)) {
+		*rcu_locked = false;
+		return memcg;
+	}
+
 	rcu_read_lock();
 	*rcu_locked = true;
 
@@ -824,6 +835,9 @@ get_non_dying_memcg_start(struct mem_cgroup *memcg, bool *rcu_locked)
 
 static inline void get_non_dying_memcg_end(bool rcu_locked)
 {
+	if (!rcu_locked)
+		return;
+
 	rcu_read_unlock();
 }
 
-- 
2.53.0