* Re: [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling
2026-08-12 12:22 [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling Kairui Song via B4 Relay
@ 2026-08-12 20:59 ` Andrew Morton
2026-08-13 1:20 ` Baolin Wang
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2026-08-12 20:59 UTC (permalink / raw)
To: kasong
Cc: Kairui Song via B4 Relay, linux-mm, Johannes Weiner,
David Hildenbrand, Michal Hocko, Qi Zheng, Shakeel Butt,
Lorenzo Stoakes, Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Oleksandr Natalenko, Suleiman Souhlal,
Jan Alexander Steffens (heftig),
Yu Zhao, Steven Barrett, Brian Geffon, Baolin Wang, Kairui Song,
linux-kernel
On Wed, 12 Aug 2026 20:22:39 +0800 Kairui Song via B4 Relay <devnull+kasong.tencent.com@kernel.org> wrote:
> From: Kairui Song <kasong@tencent.com>
>
> sort_folio() has a shortcut for moving folios that are no longer
> evictable but are still sitting on a generation list. However, this
> shortcut is buggy. It does not follow the PG_lru usage convention,
> and it has a more serious issue.
>
> Unevictable folios are not threaded on lists[LRU_UNEVICTABLE], so that
> folio->lru can be reused to hold folio->mlock_count (see the comment in
> lruvec_init()). Hence lruvec_add_folio() skips the list_add() for them,
> and every other place that turns a folio unevictable initialises
> mlock_count explicitly: lru_add() sets it to 0, __mlock_folio() and
> __mlock_new_folio() set it to !!folio_test_mlocked(folio).
> sort_folio() sets nothing, and the lru_gen_del_folio() right above it
> may have already poisoned folio->lru via list_del(), so mlock_count
> ends up aliasing LIST_POISON2, which reads as 0x122, i.e. 290. The
> result is user visible. On munlock, __munlock_folio() decrements that
> bogus count, finds it still non-zero and bails out before clearing
> PG_mlocked, so the folio remains unevictable and the Mlocked
> accounting stays inflated until the folio is freed.
>
> The shortcut also touches the LRU flags in the wrong order. It calls
> lru_gen_del_folio() while PG_lru is still set, so a concurrent
> folio_test_clear_lru() (e.g. compaction, folio_isolate_lru()) can
> succeed on a folio that has already been taken off the generation list,
> which may lead to unexpected behavior.
>
> So fix it by isolating them as common folios and letting the generic
> shrink path cull them. This matches the classical LRU behavior, and
> there should be no visible effect on the generic eviction or isolation
> behavior.
>
> There is no performance concern either, such a folio goes through this
> once, and then it is off the generation lists for good.
Great, thanks.
> Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
The above description makes me think -stable wants this.
That ac35a4902374 is four years old makes me think that we can feed
this in for 7.3-rc1.
OK?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling
2026-08-12 12:22 [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling Kairui Song via B4 Relay
2026-08-12 20:59 ` Andrew Morton
@ 2026-08-13 1:20 ` Baolin Wang
2026-08-13 8:10 ` Barry Song
2026-08-26 18:07 ` Ketan Kishore
3 siblings, 0 replies; 5+ messages in thread
From: Baolin Wang @ 2026-08-13 1:20 UTC (permalink / raw)
To: kasong, linux-mm
Cc: Andrew Morton, Johannes Weiner, David Hildenbrand, Michal Hocko,
Qi Zheng, Shakeel Butt, Lorenzo Stoakes, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Oleksandr Natalenko,
Suleiman Souhlal, Jan Alexander Steffens (heftig),
Yu Zhao, Steven Barrett, Brian Geffon, Kairui Song, linux-kernel
On 8/12/26 8:22 PM, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@tencent.com>
>
> sort_folio() has a shortcut for moving folios that are no longer
> evictable but are still sitting on a generation list. However, this
> shortcut is buggy. It does not follow the PG_lru usage convention,
> and it has a more serious issue.
>
> Unevictable folios are not threaded on lists[LRU_UNEVICTABLE], so that
> folio->lru can be reused to hold folio->mlock_count (see the comment in
> lruvec_init()). Hence lruvec_add_folio() skips the list_add() for them,
> and every other place that turns a folio unevictable initialises
> mlock_count explicitly: lru_add() sets it to 0, __mlock_folio() and
> __mlock_new_folio() set it to !!folio_test_mlocked(folio).
> sort_folio() sets nothing, and the lru_gen_del_folio() right above it
> may have already poisoned folio->lru via list_del(), so mlock_count
> ends up aliasing LIST_POISON2, which reads as 0x122, i.e. 290. The
> result is user visible. On munlock, __munlock_folio() decrements that
> bogus count, finds it still non-zero and bails out before clearing
> PG_mlocked, so the folio remains unevictable and the Mlocked
> accounting stays inflated until the folio is freed.
>
> The shortcut also touches the LRU flags in the wrong order. It calls
> lru_gen_del_folio() while PG_lru is still set, so a concurrent
> folio_test_clear_lru() (e.g. compaction, folio_isolate_lru()) can
> succeed on a folio that has already been taken off the generation list,
> which may lead to unexpected behavior.
>
> So fix it by isolating them as common folios and letting the generic
> shrink path cull them. This matches the classical LRU behavior, and
> there should be no visible effect on the generic eviction or isolation
> behavior.
>
> There is no performance concern either, such a folio goes through this
> once, and then it is off the generation lists for good.
>
> Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
> Changes in v2:
> - Proactively bypass MGLRU pid protection and lazy promotion to avoid
> hot unevcitable folios staying on list for a long time.
> - Link to v1: https://patch.msgid.link/20260811-mglru-mlock-fix-v1-1-8b2321d0e1d3@tencent.com
> ---
> mm/vmscan.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 3194da7dcc79..ca2b926520ea 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4648,7 +4648,6 @@ void lru_gen_reparent_memcg(struct mem_cgroup *memcg, struct mem_cgroup *parent,
> static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_control *sc,
> int tier_idx)
> {
> - bool success;
> int gen = folio_lru_gen(folio);
> int type = folio_is_file_lru(folio);
> int zone = folio_zonenum(folio);
> @@ -4660,15 +4659,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
>
> VM_WARN_ON_ONCE_FOLIO(gen >= MAX_NR_GENS, folio);
>
> - /* unevictable */
> - if (!folio_evictable(folio)) {
> - success = lru_gen_del_folio(lruvec, folio, true);
> - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> - folio_set_unevictable(folio);
> - lruvec_add_folio(lruvec, folio);
> - __count_vm_events(UNEVICTABLE_PGCULLED, delta);
> - return true;
> - }
> + /* unevictable: let it through and the generic path will cull it */
> + if (!folio_evictable(folio))
> + return false;
OK, returning false early in sort_folio() is better. Although I think
mlocked folios won't stay in the LRU list for long, and
shrink_folio_list() will also reject them anyway.
> /* promoted */
> if (gen != lru_gen_from_seq(lrugen->min_seq[type])) {
> @@ -4921,11 +4914,9 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> list_for_each_entry_safe_reverse(folio, next, &list, lru) {
> DEFINE_MIN_SEQ(lruvec);
>
> - if (!folio_evictable(folio)) {
> - list_del(&folio->lru);
> - folio_putback_lru(folio);
> + /* move_folios_to_lru() culls unevictable folios via folio_putback_lru() */
> + if (!folio_evictable(folio))
> continue;
Yes. Still look good to me. So feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling
2026-08-12 12:22 [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling Kairui Song via B4 Relay
2026-08-12 20:59 ` Andrew Morton
2026-08-13 1:20 ` Baolin Wang
@ 2026-08-13 8:10 ` Barry Song
2026-08-26 18:07 ` Ketan Kishore
3 siblings, 0 replies; 5+ messages in thread
From: Barry Song @ 2026-08-13 8:10 UTC (permalink / raw)
To: kasong
Cc: linux-mm, Andrew Morton, Johannes Weiner, David Hildenbrand,
Michal Hocko, Qi Zheng, Shakeel Butt, Lorenzo Stoakes,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Oleksandr Natalenko,
Suleiman Souhlal, Jan Alexander Steffens (heftig),
Yu Zhao, Steven Barrett, Brian Geffon, Baolin Wang, Kairui Song,
linux-kernel
On Wed, Aug 12, 2026 at 8:22 PM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@kernel.org> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> sort_folio() has a shortcut for moving folios that are no longer
> evictable but are still sitting on a generation list. However, this
> shortcut is buggy. It does not follow the PG_lru usage convention,
> and it has a more serious issue.
>
> Unevictable folios are not threaded on lists[LRU_UNEVICTABLE], so that
> folio->lru can be reused to hold folio->mlock_count (see the comment in
> lruvec_init()). Hence lruvec_add_folio() skips the list_add() for them,
> and every other place that turns a folio unevictable initialises
> mlock_count explicitly: lru_add() sets it to 0, __mlock_folio() and
> __mlock_new_folio() set it to !!folio_test_mlocked(folio).
> sort_folio() sets nothing, and the lru_gen_del_folio() right above it
> may have already poisoned folio->lru via list_del(), so mlock_count
> ends up aliasing LIST_POISON2, which reads as 0x122, i.e. 290. The
> result is user visible. On munlock, __munlock_folio() decrements that
> bogus count, finds it still non-zero and bails out before clearing
> PG_mlocked, so the folio remains unevictable and the Mlocked
> accounting stays inflated until the folio is freed.
>
> The shortcut also touches the LRU flags in the wrong order. It calls
> lru_gen_del_folio() while PG_lru is still set, so a concurrent
> folio_test_clear_lru() (e.g. compaction, folio_isolate_lru()) can
> succeed on a folio that has already been taken off the generation list,
> which may lead to unexpected behavior.
>
> So fix it by isolating them as common folios and letting the generic
> shrink path cull them. This matches the classical LRU behavior, and
> there should be no visible effect on the generic eviction or isolation
> behavior.
>
> There is no performance concern either, such a folio goes through this
> once, and then it is off the generation lists for good.
>
> Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
> Signed-off-by: Kairui Song <kasong@tencent.com>
Reviewed-by: Barry Song <baohua@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling
2026-08-12 12:22 [PATCH v2] mm/mglru: fix and remove redundant unevictable folio handling Kairui Song via B4 Relay
` (2 preceding siblings ...)
2026-08-13 8:10 ` Barry Song
@ 2026-08-26 18:07 ` Ketan Kishore
3 siblings, 0 replies; 5+ messages in thread
From: Ketan Kishore @ 2026-08-26 18:07 UTC (permalink / raw)
To: Kairui Song, linux-mm
Cc: Andrew Morton, Johannes Weiner, David Hildenbrand, Michal Hocko,
Qi Zheng, Shakeel Butt, Lorenzo Stoakes, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Oleksandr Natalenko,
Suleiman Souhlal, Jan Alexander Steffens (heftig),
Yu Zhao, Steven Barrett, Brian Geffon, Baolin Wang, Kairui Song,
linux-kernel, prakash.gupta
On 8/12/2026 5:52 PM, Kairui Song wrote:
> sort_folio() has a shortcut for moving folios that are no longer
> evictable but are still sitting on a generation list. However, this
> shortcut is buggy. It does not follow the PG_lru usage convention,
> and it has a more serious issue.
>
> Unevictable folios are not threaded on lists[LRU_UNEVICTABLE], so that
> folio->lru can be reused to hold folio->mlock_count (see the comment in
> lruvec_init()). Hence lruvec_add_folio() skips the list_add() for them,
> and every other place that turns a folio unevictable initialises
> mlock_count explicitly: lru_add() sets it to 0, __mlock_folio() and
> __mlock_new_folio() set it to !!folio_test_mlocked(folio).
> sort_folio() sets nothing, and the lru_gen_del_folio() right above it
> may have already poisoned folio->lru via list_del(), so mlock_count
> ends up aliasing LIST_POISON2, which reads as 0x122, i.e. 290. The
> result is user visible. On munlock, __munlock_folio() decrements that
> bogus count, finds it still non-zero and bails out before clearing
> PG_mlocked, so the folio remains unevictable and the Mlocked
> accounting stays inflated until the folio is freed.
>
> The shortcut also touches the LRU flags in the wrong order. It calls
> lru_gen_del_folio() while PG_lru is still set, so a concurrent
> folio_test_clear_lru() (e.g. compaction, folio_isolate_lru()) can
> succeed on a folio that has already been taken off the generation list,
> which may lead to unexpected behavior.
>
> So fix it by isolating them as common folios and letting the generic
> shrink path cull them. This matches the classical LRU behavior, and
> there should be no visible effect on the generic eviction or isolation
> behavior.
>
> There is no performance concern either, such a folio goes through this
> once, and then it is off the generation lists for good.
>
> Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
> Signed-off-by: Kairui Song <kasong@tencent.com>
We reported an issue with evict_folios() at
https://lore.kernel.org/all/20260807-evict_folios_race-v1-1-b167c6b4cfde@oss.qualcomm.com/
with the following trace:
list_del corruption. prev->next should be fffffffeead4fbc8,
but was ffffeafeead44188. (prev=fffffffee4fc4c08)
kernel BUG at lib/list_debug.c:64!
Call trace:
__list_del_entry_valid_or_report+0x100/0x14c
evict_folios+0x145c/0x16dc
try_to_shrink_lruvec+0x228/0x35c
shrink_one+0x94/0x158
shrink_many+0x1c8/0x1f4
lru_gen_shrink_node+0x94/0x110
shrink_node+0x468/0x8b4
balance_pgdat+0x4f0/0x9a0
kswapd+0x268/0x470
Our v1 fix wrapped the folio_putback_lru() call in evict_folios() with
lruvec->lru_lock. That introduced a self-deadlock: folio_putback_lru()
-> folio_add_lru() can synchronously reach folio_batch_move_lru() ->
folio_lruvec_relock_irqsave().
We had a v2 in progress that instead tracks whether any
unevictable folio was returned via folio_putback_lru() in the lockless
section, and calls lru_add_drain_all() before move_folios_to_lru() only
when needed, to flush all per-CPU LRU-add batches and close the race
window without ever holding lru_lock across folio_putback_lru().
Your patch fixes the same underlying race through a simpler route:
removing the unevictable shortcut in sort_folio() and no longer
special-casing unevictable folios in evict_folios() lets them fall
through to move_folios_to_lru(), which already drops lru_lock before
calling folio_putback_lru() for them. That sidesteps the lock-ordering
hazard entirely and avoids the need for an explicit drain.
Reviewed-by: Ketan Kishore <ketan.kishore@oss.qualcomm.com>
> ---
> Changes in v2:
> - Proactively bypass MGLRU pid protection and lazy promotion to avoid
> hot unevcitable folios staying on list for a long time.
> - Link to v1: https://patch.msgid.link/20260811-mglru-mlock-fix-v1-1-8b2321d0e1d3@tencent.com
> ---
> mm/vmscan.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 3194da7dcc79..ca2b926520ea 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4648,7 +4648,6 @@ void lru_gen_reparent_memcg(struct mem_cgroup *memcg, struct mem_cgroup *parent,
> static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_control *sc,
> int tier_idx)
> {
> - bool success;
> int gen = folio_lru_gen(folio);
> int type = folio_is_file_lru(folio);
> int zone = folio_zonenum(folio);
> @@ -4660,15 +4659,9 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
>
> VM_WARN_ON_ONCE_FOLIO(gen >= MAX_NR_GENS, folio);
>
> - /* unevictable */
> - if (!folio_evictable(folio)) {
> - success = lru_gen_del_folio(lruvec, folio, true);
> - VM_WARN_ON_ONCE_FOLIO(!success, folio);
> - folio_set_unevictable(folio);
> - lruvec_add_folio(lruvec, folio);
> - __count_vm_events(UNEVICTABLE_PGCULLED, delta);
> - return true;
> - }
> + /* unevictable: let it through and the generic path will cull it */
> + if (!folio_evictable(folio))
> + return false;
>
> /* promoted */
> if (gen != lru_gen_from_seq(lrugen->min_seq[type])) {
> @@ -4921,11 +4914,9 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> list_for_each_entry_safe_reverse(folio, next, &list, lru) {
> DEFINE_MIN_SEQ(lruvec);
>
> - if (!folio_evictable(folio)) {
> - list_del(&folio->lru);
> - folio_putback_lru(folio);
> + /* move_folios_to_lru() culls unevictable folios via folio_putback_lru() */
> + if (!folio_evictable(folio))
> continue;
> - }
>
> /* retry folios that may have missed folio_rotate_reclaimable() */
> if (!skip_retry && !folio_test_active(folio) && !folio_mapped(folio) &&
>
> ---
> base-commit: 1029098ee3275ea5b78e329ce132262affa2f8cc
> change-id: 20260811-mglru-mlock-fix-20d8f8d4847a
>
> Best regards,
> --
> Kairui Song <kasong@tencent.com>
>
^ permalink raw reply [flat|nested] 5+ messages in thread