linux-kernel - Re: [PATCH v3 26/30] mm: vmscan: prepare for reparenting MGLRU folios

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7cd937c3-148c-43ed-ac51-18d90cb9a5cc@linux.dev>
Date: Mon, 19 Jan 2026 11:39:37 +0800
From: Qi Zheng <qi.zheng@...ux.dev>
To: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: hannes@...xchg.org, hughd@...gle.com, mhocko@...e.com,
 roman.gushchin@...ux.dev, muchun.song@...ux.dev, david@...nel.org,
 lorenzo.stoakes@...cle.com, ziy@...dia.com, harry.yoo@...cle.com,
 yosry.ahmed@...ux.dev, imran.f.khan@...cle.com, kamalesh.babulal@...cle.com,
 axelrasmussen@...gle.com, yuanchu@...gle.com, weixugc@...gle.com,
 chenridong@...weicloud.com, mkoutny@...e.com, akpm@...ux-foundation.org,
 hamzamahfooz@...ux.microsoft.com, apais@...ux.microsoft.com,
 lance.yang@...ux.dev, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 cgroups@...r.kernel.org, Qi Zheng <zhengqi.arch@...edance.com>
Subject: Re: [PATCH v3 26/30] mm: vmscan: prepare for reparenting MGLRU folios



On 1/18/26 11:29 AM, Shakeel Butt wrote:
> I asked AI to review this patch and it find following potential issues:

Thanks.

> 
> Author: Qi Zheng <zhengqi.arch@...edance.com>
> 
> mm: vmscan: prepare for reparenting MGLRU folios
> 
> This commit adds infrastructure for reparenting MGLRU folios from dying
> memcg to parent memcg, including functions to ensure parent has MAX_NR_GENS
> generations before reparenting.
> 
> Link: https://lkml.kernel.org/r/92e0728fed3d68855173352416cf8077670610f0.1768389889.git.zhengqi.arch@bytedance.com
> 
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index f1a45c94c2fc..284292653d14 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
> 
> [ ... ]
> 
>> +static void try_to_inc_max_seq_nowalk(struct mem_cgroup *memcg,
>> +				      struct lruvec *lruvec)
>> +{
>> +	struct lru_gen_mm_list *mm_list = get_mm_list(memcg);
>> +	struct lru_gen_mm_state *mm_state = get_mm_state(lruvec);
>> +	int swappiness = mem_cgroup_swappiness(memcg);
>> +	DEFINE_MAX_SEQ(lruvec);
>> +	bool success = false;
>> +
>> +	/*
>> +	 * We are not iterating the mm_list here, updating mm_state->seq is just
>> +	 * to make mm walkers work properly.
>> +	 */
>> +	if (mm_state) {
>> +		spin_lock(&mm_list->lock);
>> +		VM_WARN_ON_ONCE(mm_state->seq + 1 < max_seq);
>> +		if (max_seq > mm_state->seq) {
>                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> When mm_state->seq equals max_seq (which can happen when mm walkers have
> completed their iteration), this condition is false, success remains false,
> and inc_max_seq() is never called.

I expect that once mm walkers complete their iterations, the number of
generations will have increased, so it won't cause an infinite loop.

> 
> The caller max_lru_gen_memcg() loops with:
> 
>      while (get_nr_gens(lruvec, type) < MAX_NR_GENS) {
>          try_to_inc_max_seq_nowalk(memcg, lruvec);
>          cond_resched();
>      }
> 
> If try_to_inc_max_seq_nowalk() fails to increment max_seq, get_nr_gens()
> returns the same value, and the while loop continues indefinitely.
> 
> Can this cause an infinite loop during memcg offline when mm_state->seq has
> caught up to max_seq?
> 
> Suggested fix: Remove the success variable and always call inc_max_seq().
> The mm_state->seq update is only for coordination with mm walkers - it should
> not gate whether we increment the generation. inc_max_seq() has its own
> staleness check (seq < READ_ONCE(lrugen->max_seq)) so calling it unconditionally
> is safe:
> 
>      if (mm_state) {
>          spin_lock(&mm_list->lock);
>          VM_WARN_ON_ONCE(mm_state->seq + 1 < max_seq);
>          if (max_seq > mm_state->seq)
>              WRITE_ONCE(mm_state->seq, mm_state->seq + 1);
>          spin_unlock(&mm_list->lock);
>      }
> 
>      inc_max_seq(lruvec, max_seq, swappiness);
> 
>> +			WRITE_ONCE(mm_state->seq, mm_state->seq + 1);
>> +			success = true;
>> +		}
>> +		spin_unlock(&mm_list->lock);
>> +	} else {
>> +		success = true;
>> +	}
>> +
>> +	if (success)
>> +		inc_max_seq(lruvec, max_seq, swappiness);
>> +}
> 
> [ ... ]
> 
>> +static void __lru_gen_reparent_memcg(struct lruvec *child_lruvec, struct lruvec *parent_lruvec,
>> +				     int zone, int type)
>> +{
>> +	struct lru_gen_folio *child_lrugen, *parent_lrugen;
>> +	enum lru_list lru = type * LRU_INACTIVE_FILE;
>> +	int i;
> 
> [ ... ]
> 
>> +		WRITE_ONCE(child_lrugen->nr_pages[gen][type][zone], 0);
>> +		WRITE_ONCE(parent_lrugen->nr_pages[gen][type][zone],
>> +			   parent_lrugen->nr_pages[gen][type][zone] + nr_pages);
>> +
>> +		update_lru_size(parent_lruvec, lru + dst_lru_active, zone, nr_pages);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> This update_lru_size() call updates both __update_lru_size() counters and
> mem_cgroup_update_lru_size(). However, lru_gen_reparent_memcg() also calls
> mem_cgroup_update_lru_size() for LRU_UNEVICTABLE. The combination appears
> to cause incorrect accounting.
> 
> Note: This issue was fixed in later commit ("mm: mglru: do not call
> update_lru_size() during reparenting").

Right.

> 
>> +	}
>> +}
>