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

[PATCH] 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 anything.

A MADV_FREE folio therefore keeps the refs and PG_workingset it had
before madvise(2).  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.

Clear both flags here instead.  lru_gen_clear_refs() only reports whether
the folio is already in the oldest generation, which lru_lazyfree() does
not need, and moving the call up would still miss the folios that are on
the classic LRU while lru_gen_enabled() is true.  Use
folio_clear_referenced() rather than folio_set_lru_refs(), which is not
defined without CONFIG_LRU_GEN; PG_referenced gates LRU_REFS_MASK, so
clearing it zeroes the refs count as well.  PG_workingset is cleared only
while MGLRU is running, which leaves the classic LRU unchanged.

Fixes: cc8ec7be78ff ("mm/mglru: optimize deactivation")
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 mm/folio.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/mm/folio.c b/mm/folio.c
index 47a437e0f7fd..03be101334eb 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -599,10 +599,16 @@ static void lru_lazyfree(struct lruvec *lruvec, struct folio *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);
+	if (lru_gen_enabled()) {
+		/*
+		 * lru_gen_clear_refs() skips an off-list folio, which
+		 * lruvec_del_folio() has just made this one.  Left set,
+		 * PG_workingset keeps it in the last tier that sort_folio()
+		 * protects from eviction.
+		 */
+		folio_clear_workingset(folio);
+	}
 	/*
 	 * Lazyfree folios are clean anonymous folios.  They have
 	 * the swapbacked flag cleared, to distinguish them from normal
-- 
2.53.0

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

[Barry Song]

On Fri, Oct 23, 2026 at 4:04 PM 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 anything.
>
> A MADV_FREE folio therefore keeps the refs and PG_workingset it had
> before madvise(2).  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.
>
> Clear both flags here instead.  lru_gen_clear_refs() only reports whether
> the folio is already in the oldest generation, which lru_lazyfree() does
> not need, and moving the call up would still miss the folios that are on
> the classic LRU while lru_gen_enabled() is true.  Use
> folio_clear_referenced() rather than folio_set_lru_refs(), which is not
> defined without CONFIG_LRU_GEN; PG_referenced gates LRU_REFS_MASK, so
> clearing it zeroes the refs count as well.  PG_workingset is cleared only
> while MGLRU is running, which leaves the classic LRU unchanged.
>
> Fixes: cc8ec7be78ff ("mm/mglru: optimize deactivation")
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> ---
>  mm/folio.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/mm/folio.c b/mm/folio.c
> index 47a437e0f7fd..03be101334eb 100644
> --- a/mm/folio.c
> +++ b/mm/folio.c
> @@ -599,10 +599,16 @@ static void lru_lazyfree(struct lruvec *lruvec, struct folio *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);
> +       if (lru_gen_enabled()) {
> +               /*
> +                * lru_gen_clear_refs() skips an off-list folio, which
> +                * lruvec_del_folio() has just made this one.  Left set,
> +                * PG_workingset keeps it in the last tier that sort_folio()
> +                * protects from eviction.
> +                */
> +               folio_clear_workingset(folio);
> +       }

So we can't do the below? But why?

diff --git a/mm/folio.c b/mm/folio.c
index 45c814f3857b..6b779a1ad2f6 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -611,12 +611,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);

Best Regards
Barry