[PATCH v3] mm/mglru: fix a no-op lru_gen_clear_refs() call in lru_lazyfree()

[PATCH v3] mm/mglru: fix a no-op lru_gen_clear_refs() call in lru_lazyfree()

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

lru_lazyfree() calls lru_gen_clear_refs() after lruvec_del_folio(), but
lru_gen_del_folio() has already cleared LRU_GEN_MASK, so folio_lru_gen()
returns -1 and lru_gen_clear_refs() exits at its "gen < 0" check without
clearing the refs or PG_workingset.

A MADV_FREE folio therefore keeps the state it had before madvise(2),
even though madvise_free_pte_range() has already cleared the accessed and
dirty bits of its PTEs.  lru_gen_folio_seq() places it one generation
younger, and PG_workingset puts it in the last tier, so sort_folio()
promotes and protects it instead of evicting it and charges it to
lrugen->protected[], which read_ctrl_pos() uses to balance anon against
file reclaim.  __folio_inc_gen() clears the refs but not PG_workingset,
so this repeats every time the folio reaches the oldest generation.  If
the folio is redirtied first, ttu_anon_lazyfree_folio() keeps it and
lru_gen_eviction() packs the stale state into the shadow entry, so a
later refault is promoted to the youngest generation.

Call lru_gen_clear_refs() while the folio is still on the generation
list, as deactivate_file_folio() and folio_deactivate() already do.  Its
return value only says whether the folio is in the oldest generation,
which lru_lazyfree() does not need because it re-adds the folio below.
Keep folio_clear_referenced() unconditional: the classic LRU needs it,
and it also covers the folios that lru_gen_clear_refs() skips.

Fixes: cc8ec7be78ff ("mm/mglru: optimize deactivation")
Cc: stable@vger.kernel.org
Suggested-by: Barry Song <baohua@kernel.org>
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
Reviewed-by: Barry Song <baohua@kernel.org>
---
Changelog:
v3:
Add Cc: stable@vger.kernel.org and pick up Barry's Reviewed-by.
No code change compared to v2.
Per the thread discussion, the __lru_gen_clear_refs() refactor will be
sent as a separate cleanup after this fix lands.
v2:
Call lru_gen_clear_refs() before lruvec_del_folio() instead of
open-coding the flag clears; drop the lru_gen_enabled() branch and
the comment (Barry Song).

 mm/folio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/folio.c b/mm/folio.c
index 35e242b48870b..ea3adb6a89d75 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -597,12 +597,10 @@ static void lru_lazyfree(struct lruvec *lruvec, struct folio *folio)
 	    folio_test_swapcache(folio) || folio_test_unevictable(folio))
 		return;
 
+	lru_gen_clear_refs(folio);
 	lruvec_del_folio(lruvec, folio);
 	folio_clear_active(folio);
-	if (lru_gen_enabled())
-		lru_gen_clear_refs(folio);
-	else
-		folio_clear_referenced(folio);
+	folio_clear_referenced(folio);
 	/*
 	 * Lazyfree folios are clean anonymous folios.  They have
 	 * the swapbacked flag cleared, to distinguish them from normal
-- 
2.43.0

Re: [PATCH v3] mm/mglru: fix a no-op lru_gen_clear_refs() call in lru_lazyfree()

[Andrew Morton]

On Mon,  9 Nov 2026 10:16:11 +0800 Hui Zhu <hui.zhu@linux.dev> wrote:

> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> lru_lazyfree() calls lru_gen_clear_refs() after lruvec_del_folio(), but
> lru_gen_del_folio() has already cleared LRU_GEN_MASK, so folio_lru_gen()
> returns -1 and lru_gen_clear_refs() exits at its "gen < 0" check without
> clearing the refs or PG_workingset.
> 
> A MADV_FREE folio therefore keeps the state it had before madvise(2),
> even though madvise_free_pte_range() has already cleared the accessed and
> dirty bits of its PTEs.  lru_gen_folio_seq() places it one generation
> younger, and PG_workingset puts it in the last tier, so sort_folio()
> promotes and protects it instead of evicting it and charges it to
> lrugen->protected[], which read_ctrl_pos() uses to balance anon against
> file reclaim.  __folio_inc_gen() clears the refs but not PG_workingset,
> so this repeats every time the folio reaches the oldest generation.  If
> the folio is redirtied first, ttu_anon_lazyfree_folio() keeps it and
> lru_gen_eviction() packs the stale state into the shadow entry, so a
> later refault is promoted to the youngest generation.
> 
> Call lru_gen_clear_refs() while the folio is still on the generation
> list, as deactivate_file_folio() and folio_deactivate() already do.  Its
> return value only says whether the folio is in the oldest generation,
> which lru_lazyfree() does not need because it re-adds the folio below.
> Keep folio_clear_referenced() unconditional: the classic LRU needs it,
> and it also covers the folios that lru_gen_clear_refs() skips.

Thanks.

> Fixes: cc8ec7be78ff ("mm/mglru: optimize deactivation")
> Cc: stable@vger.kernel.org

I'm not really understanding why a cc:stable is proposed.  Seems that
cc8ec7be78ff's changes can lead to MADV_FREE folios hanging around for
too long, but what is the runtime-visible effect of this upon our
users?

Re: [PATCH v3] mm/mglru: fix a no-op lru_gen_clear_refs() call in lru_lazyfree()

[Barry Song]

On Mon, Nov 9, 2026 at 12:51 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon,  9 Nov 2026 10:16:11 +0800 Hui Zhu <hui.zhu@linux.dev> wrote:
>
> > From: Hui Zhu <zhuhui@kylinos.cn>
> >
> > lru_lazyfree() calls lru_gen_clear_refs() after lruvec_del_folio(), but
> > lru_gen_del_folio() has already cleared LRU_GEN_MASK, so folio_lru_gen()
> > returns -1 and lru_gen_clear_refs() exits at its "gen < 0" check without
> > clearing the refs or PG_workingset.
> >
> > A MADV_FREE folio therefore keeps the state it had before madvise(2),
> > even though madvise_free_pte_range() has already cleared the accessed and
> > dirty bits of its PTEs.  lru_gen_folio_seq() places it one generation
> > younger, and PG_workingset puts it in the last tier, so sort_folio()
> > promotes and protects it instead of evicting it and charges it to
> > lrugen->protected[], which read_ctrl_pos() uses to balance anon against
> > file reclaim.  __folio_inc_gen() clears the refs but not PG_workingset,
> > so this repeats every time the folio reaches the oldest generation.  If
> > the folio is redirtied first, ttu_anon_lazyfree_folio() keeps it and
> > lru_gen_eviction() packs the stale state into the shadow entry, so a
> > later refault is promoted to the youngest generation.
> >
> > Call lru_gen_clear_refs() while the folio is still on the generation
> > list, as deactivate_file_folio() and folio_deactivate() already do.  Its
> > return value only says whether the folio is in the oldest generation,
> > which lru_lazyfree() does not need because it re-adds the folio below.
> > Keep folio_clear_referenced() unconditional: the classic LRU needs it,
> > and it also covers the folios that lru_gen_clear_refs() skips.
>
> Thanks.
>
> > Fixes: cc8ec7be78ff ("mm/mglru: optimize deactivation")
> > Cc: stable@vger.kernel.org
>
> I'm not really understanding why a cc:stable is proposed.  Seems that
> cc8ec7be78ff's changes can lead to MADV_FREE folios hanging around for
> too long, but what is the runtime-visible effect of this upon our
> users?

For folios in a MADV_FREE VMA, userspace has
explicitly indicated that they are cold and can be
reclaimed without being swapped out, unless they are
re-dirtied before the reclaim path encounters them.

Therefore, these folios should ideally be placed at the
tail of the LRU so that they are reclaimed first when
memory pressure arises.

The problem right now is that they are protected, so
some potentially warmer folios may be reclaimed instead,
which could lead to more refaults.

I once proposed something similar, although more
aggressive:

https://lore.kernel.org/linux-mm/20240914063746.46290-1-21cnbao@gmail.com/

According to Gao Xu, this can have a visible performance
impact:

https://lore.kernel.org/linux-mm/7dcd3446cd8c4da69242e5d6680c1429@honor.com/

I would expect Hui's patch to provide a similar
performance benefit by addressing this issue. I believe
commit cc8ec7be78ff introduced a regression for
workloads with MADV_FREE.

However, real-world workload data would be helpful
to demonstrate the actual performance impact and make
the case more convincing.

The patch itself makes a lot of sense in theory, though.

I'll arrange some tests with Android workloads and report
back once the data is available. If Hui already has some
data, it would be great if he could share it as well.

Thanks,
Barry