[PATCH] mm/mglru: fix and remove redundant unevictable folio handling

[PATCH] mm/mglru: fix and remove redundant unevictable folio handling

[Kairui Song via B4 Relay]

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, may
lead to unexpected behavior.  The generic path gets this right:
isolate_folio() clears PG_lru first, so a racing isolator loses the
atomic and bails.

And the shortcut is redundant.  A folio left on the generation list is
picked up by isolate_folio(), shrink_folio_list() sends it to
activate_locked on the !folio_evictable() check, and evict_folios()
then hands it to folio_putback_lru(), which sets PG_unevictable and
counts UNEVICTABLE_PGCULLED from lru_add(), with mlock_count
initialised properly.

There is no performance concern either: such a folio goes through this
once, and then it is off the generation lists for good, since
lru_gen_add_folio() refuses unevictable folios.

So just remove the shortcut.  This consolidates unevictable handling in
the generic path, and makes maintenance easier.

Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/vmscan.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 3194da7dcc79..eca5ff64238d 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,16 +4659,6 @@ 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;
-	}
-
 	/* promoted */
 	if (gen != lru_gen_from_seq(lrugen->min_seq[type])) {
 		list_move(&folio->lru, &lrugen->folios[gen][type][zone]);

---
base-commit: 1029098ee3275ea5b78e329ce132262affa2f8cc
change-id: 20260811-mglru-mlock-fix-20d8f8d4847a

Best regards,
--  
Kairui Song <kasong@tencent.com>

Re: [PATCH] mm/mglru: fix and remove redundant unevictable folio handling

[Kairui Song]

Sashiko reported this (TLDR: mostly a false positive, but I will send
a V2 to make things a bit cleaner):

--- Patch [1]: [PATCH] mm/mglru: fix and remove redundant unevictable
folio handling ---
commit 4d1ddce0f76b4c4700b6f963424566b2fce82fd9
Author: Kairui Song <kasong@tencent.com>

mm/mglru: fix and remove redundant unevictable folio handling

This commit removes the !folio_evictable(folio) shortcut in sort_folio()
to rely on the generic LRU culling path instead. The goal is to ensure
unevictable folios are handled by isolate_folio() and shrink_folio_list()
with correctly initialized mlock counts.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 3194da7dcc795..eca5ff64238dd 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4660,16 +4659,6 @@ 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;
> - }
> -
>   /* promoted */
>   if (gen != lru_gen_from_seq(lrugen->min_seq[type])) {
>   list_move(&folio->lru, &lrugen->folios[gen][type][zone]);

With the early !folio_evictable() check removed, how does this path handle
actively accessed unevictable folios?

If a folio is added to MGLRU, then the mapping is later marked unevictable
(for example, via shmctl passing SHM_LOCK), and the folio is actively
accessed by userspace, it will accumulate elevated MGLRU references.

When sort_folio() runs, wouldn't it hit this promoted branch (or the
subsequent protected branch) and return true?

sort_folio() {
...
/* promoted */
if (gen != lru_gen_from_seq(lrugen->min_seq[type])) {
list_move(&folio->lru, &lrugen->folios[gen][type][zone]);
return true;
}

/* protected */
if (tier > tier_idx || refs + workingset == BIT(LRU_REFS_WIDTH) + 1) {
gen = folio_inc_gen(lruvec, folio);
...
return true;
}
...
}

If sort_folio() returns true here, the folio is skipped for isolation.
It never reaches isolate_folio() or shrink_folio_list(), which means the
deferred !folio_evictable() check is never evaluated.

Could this cause actively accessed unevictable folios to circulate
indefinitely in the multi-gen LRU lists, artificially inflating the
active generations, instead of being culled once and for all?


========

The answer is we are fine, or at least I don't think that worth a
standalone optimization.

We have two kinds of unevictable folios here:

1. mlock(): mlock_folio() sets PG_mlocked immediately but defers the
list move to the per-CPU mlock folio batch (31/CPU), drained on
batch-full, by every lru_add_drain*(). These folios sit on a gen list
only briefly and are then moved off properly regardless of hotness.

2. SHM_LOCK / unevictable mapping: these do stay on the lists until
scanned. When their generation becomes the oldest, if they're not
worth protecting they're isolated and culled via the generic path. If
they are hot, they may be protected for a few extra passes, but this
protection decays, so it's not an indefinite circulation, and
correctness doesn't depend on when the cull happens.

But to avoid any potential issue or concern, I'll send an V2, let
sort_folio just return false for unevictable folios. I think this
change barely has any visible effect though, just to make it more
consistent with classical LRU which also unconditionally isolates
them.

Re: [PATCH] mm/mglru: fix and remove redundant unevictable folio handling

[Baolin Wang]

On 8/11/26 5:40 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, may
> lead to unexpected behavior.  The generic path gets this right:
> isolate_folio() clears PG_lru first, so a racing isolator loses the
> atomic and bails.
> 
> And the shortcut is redundant.  A folio left on the generation list is
> picked up by isolate_folio(), shrink_folio_list() sends it to
> activate_locked on the !folio_evictable() check, and evict_folios()
> then hands it to folio_putback_lru(), which sets PG_unevictable and
> counts UNEVICTABLE_PGCULLED from lru_add(), with mlock_count
> initialised properly.
> 
> There is no performance concern either: such a folio goes through this
> once, and then it is off the generation lists for good, since
> lru_gen_add_folio() refuses unevictable folios.
> 
> So just remove the shortcut.  This consolidates unevictable handling in
> the generic path, and makes maintenance easier.
> 
> Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---

Good catch. Make sense to me.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Re: [PATCH] mm/mglru: fix and remove redundant unevictable folio handling

[Kairui Song]

On Wed, Aug 12, 2026 at 6:06 PM Baolin Wang
<baolin.wang@linux.alibaba.com> wrote:
>
> On 8/11/26 5:40 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, may
> > lead to unexpected behavior.  The generic path gets this right:
> > isolate_folio() clears PG_lru first, so a racing isolator loses the
> > atomic and bails.
> >
> > And the shortcut is redundant.  A folio left on the generation list is
> > picked up by isolate_folio(), shrink_folio_list() sends it to
> > activate_locked on the !folio_evictable() check, and evict_folios()
> > then hands it to folio_putback_lru(), which sets PG_unevictable and
> > counts UNEVICTABLE_PGCULLED from lru_add(), with mlock_count
> > initialised properly.
> >
> > There is no performance concern either: such a folio goes through this
> > once, and then it is off the generation lists for good, since
> > lru_gen_add_folio() refuses unevictable folios.
> >
> > So just remove the shortcut.  This consolidates unevictable handling in
> > the generic path, and makes maintenance easier.
> >
> > Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
> > Signed-off-by: Kairui Song <kasong@tencent.com>
> > ---
>
> Good catch. Make sense to me.
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Hi Baolin

Thank you very mcuh for the review!

I just sent a V2 here:
https://lore.kernel.org/linux-mm/20260812-mglru-mlock-fix-v2-1-a3fec5853c08@tencent.com/T/#u

This is a bit different, idea is still the same but I changed the
code, since the patch is very small so it's basically a rewrite, hence
I didn't include this review by. Can you help have a look at v2 as
well?

Re: [PATCH] mm/mglru: fix and remove redundant unevictable folio handling

[Baolin Wang]

On 8/12/26 8:28 PM, Kairui Song wrote:
> On Wed, Aug 12, 2026 at 6:06 PM Baolin Wang
> <baolin.wang@linux.alibaba.com> wrote:
>>
>> On 8/11/26 5:40 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, may
>>> lead to unexpected behavior.  The generic path gets this right:
>>> isolate_folio() clears PG_lru first, so a racing isolator loses the
>>> atomic and bails.
>>>
>>> And the shortcut is redundant.  A folio left on the generation list is
>>> picked up by isolate_folio(), shrink_folio_list() sends it to
>>> activate_locked on the !folio_evictable() check, and evict_folios()
>>> then hands it to folio_putback_lru(), which sets PG_unevictable and
>>> counts UNEVICTABLE_PGCULLED from lru_add(), with mlock_count
>>> initialised properly.
>>>
>>> There is no performance concern either: such a folio goes through this
>>> once, and then it is off the generation lists for good, since
>>> lru_gen_add_folio() refuses unevictable folios.
>>>
>>> So just remove the shortcut.  This consolidates unevictable handling in
>>> the generic path, and makes maintenance easier.
>>>
>>> Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
>>> Signed-off-by: Kairui Song <kasong@tencent.com>
>>> ---
>>
>> Good catch. Make sense to me.
>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> 
> Hi Baolin
> 
> Thank you very mcuh for the review!
> 
> I just sent a V2 here:
> https://lore.kernel.org/linux-mm/20260812-mglru-mlock-fix-v2-1-a3fec5853c08@tencent.com/T/#u
> 
> This is a bit different, idea is still the same but I changed the
> code, since the patch is very small so it's basically a rewrite, hence
> I didn't include this review by. Can you help have a look at v2 as
> well?

Sure. Thanks for the fix.