[PATCH v3 0/2] mm/mglru: fix ineffective memory protection for non-kswapd reclaim

[PATCH v3 0/2] mm/mglru: fix ineffective memory protection for non-kswapd reclaim

[Ridong Chen]

From: Ridong Chen <chenridong@xiaomi.com>

For MGLRU, memory.min/low is not honored during non-kswapd global
reclaim (global direct reclaim and root-level memory.reclaim), because
these paths shrink memcgs using stale effective protection (emin/elow).

Patch 2 is the actual fix. Patch 1 is a prerequisite: an integer overflow
in effective_protection(), spotted by the sashiko review tool, which
patch 2's new caller would also be exposed to.

---

v3:
- Per Barry Song's review comments

v2:
- Fix an issue caused by non-atomic read races in patch 1 [1]

[1] https://sashiko.dev/#/patchset/20260828092432.1257917-1-ridong.chen@linux.dev?part=1

Ridong Chen (2):
  mm/page_counter: avoid integer overflow in effective_protection()
  mm/mglru: fix ineffective memory protection for non-kswapd reclaim

 include/linux/memcontrol.h | 10 +++++++++
 mm/memcontrol.c            | 45 ++++++++++++++++++++++++++++++++++++++
 mm/page_counter.c          | 19 +++++++++++-----
 mm/vmscan.c                |  8 ++++++-
 4 files changed, 75 insertions(+), 7 deletions(-)


base-commit: 32b6ef9a5d0eca44f9cd91f52f4faa89f145a0de
--
2.34.1

[PATCH v3 1/2] mm/page_counter: avoid integer overflow in effective_protection()

[Ridong Chen]

From: Ridong Chen <chenridong@xiaomi.com>

effective_protection() scales a parent's protection by a ratio of page
counts, e.g. for recursive protection:

	(parent_effective - siblings_protected) * (usage - protected)
		/ (parent_usage - siblings_protected)

The multiply is done at unsigned long width before dividing. On systems
with >= 16TB RAM the product can exceed 2^64 and wrap, giving a bogus
protection value and silently breaking memory.min/low enforcement.

Use mul_u64_u64_div_u64() to multiply in a 128-bit intermediate. Because
usage and parent_usage are not read atomically (a child is charged
before its parent), usage - protected can briefly exceed the divisor,
making the quotient overflow 64 bits and trap (#DE on x86). Cap it so
the ratio stays <= 1.

Reported by the sashiko review tool [1].

[1] https://sashiko.dev/#/patchset/20260826133054.88529-1-ridong.chen@linux.dev?part=1

Fixes: bc50bcc6e00b ("mm: memcontrol: clean up and document effective low/min calculations")
Fixes: 8a931f801340 ("mm: memcontrol: recursive memory.low protection")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Reviewed-by: Barry Song <baohua@kernel.org>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 mm/page_counter.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/mm/page_counter.c b/mm/page_counter.c
index 661e0f2a5127..e8bd512069c5 100644
--- a/mm/page_counter.c
+++ b/mm/page_counter.c
@@ -8,6 +8,7 @@
 #include <linux/page_counter.h>
 #include <linux/atomic.h>
 #include <linux/kernel.h>
+#include <linux/math64.h>
 #include <linux/string.h>
 #include <linux/sched.h>
 #include <linux/bug.h>
@@ -356,7 +357,8 @@ static unsigned long effective_protection(unsigned long usage,
 	 * otherwise get a smaller chunk than what they claimed.
 	 */
 	if (siblings_protected > parent_effective)
-		return protected * parent_effective / siblings_protected;
+		return mul_u64_u64_div_u64(protected, parent_effective,
+					   siblings_protected);
 
 	/*
 	 * Ok, utilized protection of all children is within what the
@@ -397,13 +399,18 @@ static unsigned long effective_protection(unsigned long usage,
 	if (parent_effective > siblings_protected &&
 	    parent_usage > siblings_protected &&
 	    usage > protected) {
-		unsigned long unclaimed;
+		unsigned long unclaimed = parent_effective - siblings_protected;
+		unsigned long unprotected = usage - protected;
+		unsigned long parent_unprotected = parent_usage - siblings_protected;
 
-		unclaimed = parent_effective - siblings_protected;
-		unclaimed *= usage - protected;
-		unclaimed /= parent_usage - siblings_protected;
+		/*
+		 * The usages aren't read atomically, so a child can transiently
+		 * appear to use more than its parent, making the ratio exceed 1
+		 * and the quotient overflow 64 bits (#DE on x86).  Cap it.
+		 */
+		unprotected = min(unprotected, parent_unprotected);
 
-		ep += unclaimed;
+		ep += mul_u64_u64_div_u64(unclaimed, unprotected, parent_unprotected);
 	}
 
 	return ep;
-- 
2.34.1

Re: [PATCH v3 1/2] mm/page_counter: avoid integer overflow in effective_protection()

[Johannes Weiner]

On Thu, Sep 03, 2026 at 11:19:51AM +0800, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> effective_protection() scales a parent's protection by a ratio of page
> counts, e.g. for recursive protection:
> 
> 	(parent_effective - siblings_protected) * (usage - protected)
> 		/ (parent_usage - siblings_protected)
> 
> The multiply is done at unsigned long width before dividing. On systems
> with >= 16TB RAM the product can exceed 2^64 and wrap, giving a bogus
> protection value and silently breaking memory.min/low enforcement.
> 
> Use mul_u64_u64_div_u64() to multiply in a 128-bit intermediate. Because
> usage and parent_usage are not read atomically (a child is charged
> before its parent), usage - protected can briefly exceed the divisor,
> making the quotient overflow 64 bits and trap (#DE on x86). Cap it so
> the ratio stays <= 1.
> 
> Reported by the sashiko review tool [1].
> 
> [1] https://sashiko.dev/#/patchset/20260826133054.88529-1-ridong.chen@linux.dev?part=1
> 
> Fixes: bc50bcc6e00b ("mm: memcontrol: clean up and document effective low/min calculations")
> Fixes: 8a931f801340 ("mm: memcontrol: recursive memory.low protection")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Reviewed-by: Barry Song <baohua@kernel.org>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>  mm/page_counter.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_counter.c b/mm/page_counter.c
> index 661e0f2a5127..e8bd512069c5 100644
> --- a/mm/page_counter.c
> +++ b/mm/page_counter.c
> @@ -8,6 +8,7 @@
>  #include <linux/page_counter.h>
>  #include <linux/atomic.h>
>  #include <linux/kernel.h>
> +#include <linux/math64.h>
>  #include <linux/string.h>
>  #include <linux/sched.h>
>  #include <linux/bug.h>
> @@ -356,7 +357,8 @@ static unsigned long effective_protection(unsigned long usage,
>  	 * otherwise get a smaller chunk than what they claimed.
>  	 */
>  	if (siblings_protected > parent_effective)
> -		return protected * parent_effective / siblings_protected;
> +		return mul_u64_u64_div_u64(protected, parent_effective,
> +					   siblings_protected);
>  
>  	/*
>  	 * Ok, utilized protection of all children is within what the
> @@ -397,13 +399,18 @@ static unsigned long effective_protection(unsigned long usage,
>  	if (parent_effective > siblings_protected &&
>  	    parent_usage > siblings_protected &&
>  	    usage > protected) {
> -		unsigned long unclaimed;
> +		unsigned long unclaimed = parent_effective - siblings_protected;
> +		unsigned long unprotected = usage - protected;
> +		unsigned long parent_unprotected = parent_usage - siblings_protected;
>  
> -		unclaimed = parent_effective - siblings_protected;
> -		unclaimed *= usage - protected;
> -		unclaimed /= parent_usage - siblings_protected;
> +		/*
> +		 * The usages aren't read atomically, so a child can transiently
> +		 * appear to use more than its parent, making the ratio exceed 1
> +		 * and the quotient overflow 64 bits (#DE on x86).  Cap it.
> +		 */
> +		unprotected = min(unprotected, parent_unprotected);

Looks correct to me. But a few nits on readability, since this code
already is quite painfully complicated.

Please don't do math in the declaration block.

`unclaimed` made a bit more sense when it held *this group's* final
share of the unclaimed protection. As an intermediate, it's *the
parent's* unclaimed protection.

Put together, it should look something like this:

		unsigned long parent_unclaimed, parent_unprotected, unprotected;

		parent_unclaimed = parent_effective - siblings_protected;
		parent_unprotected = parent_usage - siblings_protected;
		unprotected = usage - protected;

		/* overflow comment */
		unprotected = min(usage - protected, parent_unprotected);
		ep += mul_u64_u64_div_u64(parent_unclaimed, unprotected, parent_unprotected);

With that,

Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH v3 1/2] mm/page_counter: avoid integer overflow in effective_protection()

[Ridong Chen]

On 9/3/2026 10:00 PM, Johannes Weiner wrote:
> On Thu, Sep 03, 2026 at 11:19:51AM +0800, Ridong Chen wrote:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> effective_protection() scales a parent's protection by a ratio of page
>> counts, e.g. for recursive protection:
>>
>> 	(parent_effective - siblings_protected) * (usage - protected)
>> 		/ (parent_usage - siblings_protected)
>>
>> The multiply is done at unsigned long width before dividing. On systems
>> with >= 16TB RAM the product can exceed 2^64 and wrap, giving a bogus
>> protection value and silently breaking memory.min/low enforcement.
>>
>> Use mul_u64_u64_div_u64() to multiply in a 128-bit intermediate. Because
>> usage and parent_usage are not read atomically (a child is charged
>> before its parent), usage - protected can briefly exceed the divisor,
>> making the quotient overflow 64 bits and trap (#DE on x86). Cap it so
>> the ratio stays <= 1.
>>
>> Reported by the sashiko review tool [1].
>>
>> [1] https://sashiko.dev/#/patchset/20260826133054.88529-1-ridong.chen@linux.dev?part=1
>>
>> Fixes: bc50bcc6e00b ("mm: memcontrol: clean up and document effective low/min calculations")
>> Fixes: 8a931f801340 ("mm: memcontrol: recursive memory.low protection")
>> Cc: stable@vger.kernel.org
>> Assisted-by: Claude:claude-opus-4-8
>> Reviewed-by: Barry Song <baohua@kernel.org>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   mm/page_counter.c | 19 +++++++++++++------
>>   1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/page_counter.c b/mm/page_counter.c
>> index 661e0f2a5127..e8bd512069c5 100644
>> --- a/mm/page_counter.c
>> +++ b/mm/page_counter.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/page_counter.h>
>>   #include <linux/atomic.h>
>>   #include <linux/kernel.h>
>> +#include <linux/math64.h>
>>   #include <linux/string.h>
>>   #include <linux/sched.h>
>>   #include <linux/bug.h>
>> @@ -356,7 +357,8 @@ static unsigned long effective_protection(unsigned long usage,
>>   	 * otherwise get a smaller chunk than what they claimed.
>>   	 */
>>   	if (siblings_protected > parent_effective)
>> -		return protected * parent_effective / siblings_protected;
>> +		return mul_u64_u64_div_u64(protected, parent_effective,
>> +					   siblings_protected);
>>   
>>   	/*
>>   	 * Ok, utilized protection of all children is within what the
>> @@ -397,13 +399,18 @@ static unsigned long effective_protection(unsigned long usage,
>>   	if (parent_effective > siblings_protected &&
>>   	    parent_usage > siblings_protected &&
>>   	    usage > protected) {
>> -		unsigned long unclaimed;
>> +		unsigned long unclaimed = parent_effective - siblings_protected;
>> +		unsigned long unprotected = usage - protected;
>> +		unsigned long parent_unprotected = parent_usage - siblings_protected;
>>   
>> -		unclaimed = parent_effective - siblings_protected;
>> -		unclaimed *= usage - protected;
>> -		unclaimed /= parent_usage - siblings_protected;
>> +		/*
>> +		 * The usages aren't read atomically, so a child can transiently
>> +		 * appear to use more than its parent, making the ratio exceed 1
>> +		 * and the quotient overflow 64 bits (#DE on x86).  Cap it.
>> +		 */
>> +		unprotected = min(unprotected, parent_unprotected);
> 
> Looks correct to me. But a few nits on readability, since this code
> already is quite painfully complicated.
> 
> Please don't do math in the declaration block.
> 
> `unclaimed` made a bit more sense when it held *this group's* final
> share of the unclaimed protection. As an intermediate, it's *the
> parent's* unclaimed protection.
> 
> Put together, it should look something like this:
> 
> 		unsigned long parent_unclaimed, parent_unprotected, unprotected;
> 
> 		parent_unclaimed = parent_effective - siblings_protected;
> 		parent_unprotected = parent_usage - siblings_protected;
> 		unprotected = usage - protected;
> 
> 		/* overflow comment */
> 		unprotected = min(usage - protected, parent_unprotected);
> 		ep += mul_u64_u64_div_u64(parent_unclaimed, unprotected, parent_unprotected);
> 
> With that,
> 
> Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

Thank you for your suggestion. Will update.

-- 
Best regards
Ridong

Re: [PATCH v3 1/2] mm/page_counter: avoid integer overflow in effective_protection()

[David Laight]

On Thu,  3 Sep 2026 11:19:51 +0800
Ridong Chen <ridong.chen@linux.dev> wrote:

> From: Ridong Chen <chenridong@xiaomi.com>
> 
> effective_protection() scales a parent's protection by a ratio of page
> counts, e.g. for recursive protection:
> 
> 	(parent_effective - siblings_protected) * (usage - protected)
> 		/ (parent_usage - siblings_protected)
> 
> The multiply is done at unsigned long width before dividing. On systems
> with >= 16TB RAM the product can exceed 2^64 and wrap, giving a bogus
> protection value and silently breaking memory.min/low enforcement.
> 
> Use mul_u64_u64_div_u64() to multiply in a 128-bit intermediate. Because
> usage and parent_usage are not read atomically (a child is charged
> before its parent), usage - protected can briefly exceed the divisor,
> making the quotient overflow 64 bits and trap (#DE on x86). Cap it so
> the ratio stays <= 1.
> 
> Reported by the sashiko review tool [1].
> 
> [1] https://sashiko.dev/#/patchset/20260826133054.88529-1-ridong.chen@linux.dev?part=1
> 
> Fixes: bc50bcc6e00b ("mm: memcontrol: clean up and document effective low/min calculations")
> Fixes: 8a931f801340 ("mm: memcontrol: recursive memory.low protection")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Reviewed-by: Barry Song <baohua@kernel.org>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>  mm/page_counter.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_counter.c b/mm/page_counter.c
> index 661e0f2a5127..e8bd512069c5 100644
> --- a/mm/page_counter.c
> +++ b/mm/page_counter.c
> @@ -8,6 +8,7 @@
>  #include <linux/page_counter.h>
>  #include <linux/atomic.h>
>  #include <linux/kernel.h>
> +#include <linux/math64.h>
>  #include <linux/string.h>
>  #include <linux/sched.h>
>  #include <linux/bug.h>
> @@ -356,7 +357,8 @@ static unsigned long effective_protection(unsigned long usage,
>  	 * otherwise get a smaller chunk than what they claimed.
>  	 */
>  	if (siblings_protected > parent_effective)
> -		return protected * parent_effective / siblings_protected;
> +		return mul_u64_u64_div_u64(protected, parent_effective,
> +					   siblings_protected);

On 32bit it is only necessary to use a 64bit intermediary.
mul_u64_u64_div_u64() will drop back to the (probably faster) 64 by 64
divide (and then maybe to a 64 by 32 one).
But there is a lot of extra code before that happens.

>  
>  	/*
>  	 * Ok, utilized protection of all children is within what the
> @@ -397,13 +399,18 @@ static unsigned long effective_protection(unsigned long usage,
>  	if (parent_effective > siblings_protected &&
>  	    parent_usage > siblings_protected &&
>  	    usage > protected) {
> -		unsigned long unclaimed;
> +		unsigned long unclaimed = parent_effective - siblings_protected;
> +		unsigned long unprotected = usage - protected;
> +		unsigned long parent_unprotected = parent_usage - siblings_protected;
>  
> -		unclaimed = parent_effective - siblings_protected;
> -		unclaimed *= usage - protected;
> -		unclaimed /= parent_usage - siblings_protected;
> +		/*
> +		 * The usages aren't read atomically, so a child can transiently
> +		 * appear to use more than its parent, making the ratio exceed 1
> +		 * and the quotient overflow 64 bits (#DE on x86).  Cap it.
> +		 */
> +		unprotected = min(unprotected, parent_unprotected);
>  
> -		ep += unclaimed;
> +		ep += mul_u64_u64_div_u64(unclaimed, unprotected, parent_unprotected);

If the ratio is forced to 1 there is no point doing the scaling.
So maybe:
		if (likely(parent_unprotected > unprotected))
			unclaimed = mul_u64_u64_div_u64(unclaimed, unprotected,
							parent_unprotected);
		ep += unclaimed;
OTOH if the min() generates a cmov rather than a conditional branch
then you don't get a statically mispredicted branch in the normal case
(which is very likely with the empty 'else' branch).

David

>  	}
>  
>  	return ep;

[PATCH v3 2/2] mm/mglru: fix ineffective memory protection for non-kswapd reclaim

[Ridong Chen]

From: Ridong Chen <chenridong@xiaomi.com>

For MGLRU, memory.min/low is not honored during global proactive reclaim
(writing to the root memory.reclaim) and global direct reclaim, because
these paths shrink memcgs using stale or effective protection (emin/elow).
It can be reproduced as follows:

  # echo 7 > /sys/kernel/mm/lru_gen/enabled
  # cd /sys/fs/cgroup
  # mkdir -p a/b
  # echo 100M > a/memory.min
  # echo +memory > a/cgroup.subtree_control
  # echo 100M > a/b/memory.min
  # echo $$ > a/b/cgroup.procs
  # dd if=/dev/zero of=/tmp/testfile bs=1M count=200
  # cat a/b/memory.current
  222650368
  # echo 500M > memory.reclaim
  -bash: echo: write error: Resource temporarily unavailable
  # cat a/b/memory.current
  6070272

memory.min is 100M, yet reclaim drops a/b down to 6M, breaking the
protection. The traditional LRU path is not affected because
shrink_node() calls mem_cgroup_calculate_protection() for each memcg it
visits during a top-down tree walk.

Commit 30d77b7eef01 ("mm/mglru: fix ineffective protection calculation")
moved the protection computation into lru_gen_age_node(), which only
runs for kswapd. Non-kswapd global reclaim reaches shrink_one() through
lru_gen_shrink_node() -> shrink_many() without any protection
computation, so emin/elow are whatever a previous kswapd run left behind
- or zero if kswapd never ran on this node. Relying on a prior kswapd
pass is not correct either: a memcg's emin/elow are derived from its
ancestors' memory.min/low settings and from children_min_usage, both of
which change over time, so emin/elow go stale even after kswapd has run
and must be recomputed at the point of reclaim.

Introduce mem_cgroup_calculate_protection_path() which computes emin/elow
along the root-to-target path only, by iterating through the cgroup
ancestors array top-down. This avoids the full tree traversal that would
be needed with mem_cgroup_calculate_protection(), limiting the cost to
O(depth) per memcg - typically 3-5 levels.

Call it from shrink_one() for the non-kswapd path so that each memcg
about to be shrunk has correct protection values.

Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 include/linux/memcontrol.h | 10 +++++++++
 mm/memcontrol.c            | 45 ++++++++++++++++++++++++++++++++++++++
 mm/vmscan.c                |  8 ++++++-
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index f227348a3f24..a65a516adc66 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1885,6 +1885,16 @@ static inline bool memcg_is_dying(struct mem_cgroup *memcg)
 }
 #endif /* CONFIG_MEMCG */
 
+#if defined(CONFIG_MEMCG) && defined(CONFIG_LRU_GEN)
+void mem_cgroup_calculate_protection_path(struct mem_cgroup *root,
+					  struct mem_cgroup *memcg);
+#else
+static inline void mem_cgroup_calculate_protection_path(struct mem_cgroup *root,
+							struct mem_cgroup *memcg)
+{
+}
+#endif
+
 #if defined(CONFIG_MEMCG) && defined(CONFIG_ZSWAP)
 bool obj_cgroup_may_zswap(struct obj_cgroup *objcg);
 void obj_cgroup_charge_zswap(struct obj_cgroup *objcg, size_t size);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 256b68ffca70..ae568fc68813 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5214,6 +5214,51 @@ void mem_cgroup_calculate_protection(struct mem_cgroup *root,
 	page_counter_calculate_protection(&root->memory, &memcg->memory, recursive_protection);
 }
 
+#ifdef CONFIG_LRU_GEN
+/**
+ * mem_cgroup_calculate_protection_path - compute protection along a path
+ * @root: the top ancestor of the sub-tree being checked (NULL for root_mem_cgroup)
+ * @memcg: the target memory cgroup
+ *
+ * Walk the ancestor path from @root down to @memcg and compute the effective
+ * protection at each level.  This is safe for isolated queries because it
+ * ensures parents are computed before children.
+ */
+void mem_cgroup_calculate_protection_path(struct mem_cgroup *root,
+					  struct mem_cgroup *memcg)
+{
+	bool recursive_protection =
+		cgrp_dfl_root.flags & CGRP_ROOT_MEMORY_RECURSIVE_PROT;
+	struct cgroup *cg;
+	int root_level, i;
+
+	if (mem_cgroup_disabled())
+		return;
+
+	if (!root)
+		root = root_mem_cgroup;
+
+	if (memcg == root)
+		return;
+
+	root_level = root->css.cgroup->level;
+	cg = memcg->css.cgroup;
+
+	rcu_read_lock();
+	for (i = root_level + 1; i <= cg->level; i++) {
+		struct mem_cgroup *cur;
+
+		cur = mem_cgroup_from_css(cgroup_css(cg->ancestors[i],
+						     &memory_cgrp_subsys));
+		if (cur)
+			page_counter_calculate_protection(&root->memory,
+							  &cur->memory,
+							  recursive_protection);
+	}
+	rcu_read_unlock();
+}
+#endif /* CONFIG_LRU_GEN */
+
 static int charge_memcg(struct folio *folio, struct mem_cgroup *memcg,
 			gfp_t gfp)
 {
diff --git a/mm/vmscan.c b/mm/vmscan.c
index b4c9b8f3dfe9..500cc2051d13 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -5111,7 +5111,13 @@ static int shrink_one(struct lruvec *lruvec, struct scan_control *sc)
 	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 
-	/* lru_gen_age_node() called mem_cgroup_calculate_protection() */
+	/*
+	 * For kswapd, lru_gen_age_node() has already called
+	 * mem_cgroup_calculate_protection()
+	 */
+	if (!current_is_kswapd())
+		mem_cgroup_calculate_protection_path(NULL, memcg);
+
 	if (mem_cgroup_below_min(NULL, memcg))
 		return MEMCG_LRU_YOUNG;
 
-- 
2.34.1

Re: [PATCH v3 2/2] mm/mglru: fix ineffective memory protection for non-kswapd reclaim

[Barry Song]

On Thu, Sep 3, 2026 at 11:20 AM Ridong Chen <ridong.chen@linux.dev> wrote:
>
> From: Ridong Chen <chenridong@xiaomi.com>
>
> For MGLRU, memory.min/low is not honored during global proactive reclaim
> (writing to the root memory.reclaim) and global direct reclaim, because
> these paths shrink memcgs using stale or effective protection (emin/elow).

Are you sure it should be “stale or effective”?
My gut feeling is that we don't need the “or” here.

[...]
>
> Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---

With a few nits:

Reviewed-by: Barry Song <baohua@kernel.org>

[...]
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index b4c9b8f3dfe9..500cc2051d13 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -5111,7 +5111,13 @@ static int shrink_one(struct lruvec *lruvec, struct scan_control *sc)
>         struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>         struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>
> -       /* lru_gen_age_node() called mem_cgroup_calculate_protection() */
> +       /*
> +        * For kswapd, lru_gen_age_node() has already called
> +        * mem_cgroup_calculate_protection()
> +        */

Could be clearer:

/*
 * For kswapd, mem_cgroup_calculate_protection() has already
 * been called during the top-down cgroup traversal.
 */

> +       if (!current_is_kswapd())
> +               mem_cgroup_calculate_protection_path(NULL, memcg);

Best Regards
Barry

Re: [PATCH v3 2/2] mm/mglru: fix ineffective memory protection for non-kswapd reclaim

[Ridong Chen]

On 9/3/2026 2:30 PM, Barry Song wrote:
> On Thu, Sep 3, 2026 at 11:20 AM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> For MGLRU, memory.min/low is not honored during global proactive reclaim
>> (writing to the root memory.reclaim) and global direct reclaim, because
>> these paths shrink memcgs using stale or effective protection (emin/elow).
> 
> Are you sure it should be “stale or effective”?
> My gut feeling is that we don't need the “or” here.
> 

My bad. It was a typo. Will remove "or".

> [...]
>>
>> Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
>> Cc: stable@vger.kernel.org
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
> 
> With a few nits:
> 
> Reviewed-by: Barry Song <baohua@kernel.org>
> 
> [...]
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index b4c9b8f3dfe9..500cc2051d13 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -5111,7 +5111,13 @@ static int shrink_one(struct lruvec *lruvec, struct scan_control *sc)
>>          struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>>          struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>>
>> -       /* lru_gen_age_node() called mem_cgroup_calculate_protection() */
>> +       /*
>> +        * For kswapd, lru_gen_age_node() has already called
>> +        * mem_cgroup_calculate_protection()
>> +        */
> 
> Could be clearer:
> 
> /*
>   * For kswapd, mem_cgroup_calculate_protection() has already
>   * been called during the top-down cgroup traversal.
>   */
> 

Thank you, will update.

>> +       if (!current_is_kswapd())
>> +               mem_cgroup_calculate_protection_path(NULL, memcg);
> 
> Best Regards
> Barry

-- 
Best regards
Ridong

Re: [PATCH v3 2/2] mm/mglru: fix ineffective memory protection for non-kswapd reclaim

[Johannes Weiner]

On Thu, Sep 03, 2026 at 11:19:52AM +0800, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> For MGLRU, memory.min/low is not honored during global proactive reclaim
> (writing to the root memory.reclaim) and global direct reclaim, because
> these paths shrink memcgs using stale or effective protection (emin/elow).
> It can be reproduced as follows:
> 
>   # echo 7 > /sys/kernel/mm/lru_gen/enabled
>   # cd /sys/fs/cgroup
>   # mkdir -p a/b
>   # echo 100M > a/memory.min
>   # echo +memory > a/cgroup.subtree_control
>   # echo 100M > a/b/memory.min
>   # echo $$ > a/b/cgroup.procs
>   # dd if=/dev/zero of=/tmp/testfile bs=1M count=200
>   # cat a/b/memory.current
>   222650368
>   # echo 500M > memory.reclaim
>   -bash: echo: write error: Resource temporarily unavailable
>   # cat a/b/memory.current
>   6070272
> 
> memory.min is 100M, yet reclaim drops a/b down to 6M, breaking the
> protection. The traditional LRU path is not affected because
> shrink_node() calls mem_cgroup_calculate_protection() for each memcg it
> visits during a top-down tree walk.
> 
> Commit 30d77b7eef01 ("mm/mglru: fix ineffective protection calculation")
> moved the protection computation into lru_gen_age_node(), which only
> runs for kswapd. Non-kswapd global reclaim reaches shrink_one() through
> lru_gen_shrink_node() -> shrink_many() without any protection
> computation, so emin/elow are whatever a previous kswapd run left behind
> - or zero if kswapd never ran on this node. Relying on a prior kswapd
> pass is not correct either: a memcg's emin/elow are derived from its
> ancestors' memory.min/low settings and from children_min_usage, both of
> which change over time, so emin/elow go stale even after kswapd has run
> and must be recomputed at the point of reclaim.
> 
> Introduce mem_cgroup_calculate_protection_path() which computes emin/elow
> along the root-to-target path only, by iterating through the cgroup
> ancestors array top-down. This avoids the full tree traversal that would
> be needed with mem_cgroup_calculate_protection(), limiting the cost to
> O(depth) per memcg - typically 3-5 levels.
> 
> Call it from shrink_one() for the non-kswapd path so that each memcg
> about to be shrunk has correct protection values.
> 
> Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>

With Barry's two points of feedback (changelog and comment),

Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>