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

[Ridong Chen <ridong.chen@linux.dev>]

On 8/30/2026 3:53 PM, Barry Song wrote:
> On Fri, Aug 28, 2026 at 7:10 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> memory.min/low is silently bypassed for MGLRU during global proactive
>> reclaim (writing to the root memory.reclaim) and global direct reclaim.
> 
> I guess nobody is silently bypassing anything. It's just that the
> effective min is stale data. If kswapd has run at least once, should
> the protection have been updated already?
> I guess we need to update the changelog a bit?
> 

The children's emin/elow are derived from the parent's min/low settings and 
children_min_usage, both of which can change over time. As a result, emin/elow 
may become stale, even if kswapd has already run once.

>> 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 remain stale or zero.
>>
>> Introduce mem_cgroup_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 | 11 ++++++++++
>>   mm/memcontrol.c            | 45 ++++++++++++++++++++++++++++++++++++++
>>   mm/vmscan.c                |  8 ++++++-
>>   3 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index 7d1c0ce189a8..8066b798a759 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -605,6 +605,10 @@ static inline void mem_cgroup_protection(struct mem_cgroup *root,
>>
>>   void mem_cgroup_calculate_protection(struct mem_cgroup *root,
>>                                       struct mem_cgroup *memcg);
>> +#ifdef CONFIG_LRU_GEN
>> +void mem_cgroup_protection_path(struct mem_cgroup *root,
>> +                               struct mem_cgroup *memcg);
>> +#endif
>>
>>   static inline bool mem_cgroup_unprotected(struct mem_cgroup *target,
>>                                            struct mem_cgroup *memcg)
>> @@ -1133,6 +1137,13 @@ static inline void mem_cgroup_calculate_protection(struct mem_cgroup *root,
>>   {
>>   }
>>
>> +#ifdef CONFIG_LRU_GEN
>> +static inline void mem_cgroup_protection_path(struct mem_cgroup *root,
>> +                                             struct mem_cgroup *memcg)
>> +{
>> +}
>> +#endif
>> +
> 
> I wonder if we could follow the zswap pattern?
> 
> #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);
> void obj_cgroup_uncharge_zswap(struct obj_cgroup *objcg, size_t size);
> bool mem_cgroup_zswap_writeback_enabled(struct mem_cgroup *memcg);
> #else
> static inline bool obj_cgroup_may_zswap(struct obj_cgroup *objcg)
> {
>          return true;
> }

That is fine for me, I will update it.

> ...
> #endif
> 
>>   static inline bool mem_cgroup_unprotected(struct mem_cgroup *target,
>>                                            struct mem_cgroup *memcg)
>>   {
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 1271d390b617..095050d4296a 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -5198,6 +5198,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_protection_path - compute protection along root->memcg 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_protection_path(struct mem_cgroup *root,
>> +                               struct mem_cgroup *memcg)
> 
> Can we rename it to `mem_cgroup_calculate_protection_path()`?
> 
> BTW, I see that the only caller is in vmscan and it passes NULL as
> `root`. Do we need to keep the `root` argument if the new helper is
> only used for global reclaim?
> 
I'd suggest keeping it as is. This function updates protection along the path 
from root to memcg, and could be reused later. Note that 
mem_cgroup_calculate_protection() assumes the caller has already performed the 
top-down walk, each level's calculation depends on its parent being updated first.

For mem_cgroup_calculate_protection_path(), it can be called in any context 
without such a precondition.

-- 
Best regards
Ridong