[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

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

[Bo Zhang]

On Mon, Nov 9, 2026 at 1:08 PM Barry Song <baohua@kernel.org> wrote:
> 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.

Tested on a Xiaomi phone (android16-6.12 GKI with this fix backported,
MGLRU enabled). The workload launches and scrolls 26 common apps for
5 rounds; each run produces ~7-9M MADV_FREE pages. 3 runs per kernel,
same system image, same sequence (fresh boot, then back-to-back), only
the kernel differs:

                            without fix   with fix
  workingset_refault_file      44.7M        29.0M   (-35%)
  pgmajfault                   13.9M         9.0M   (-35%)
  pgsteal (kswapd+direct)     100.9M        75.6M   (-25%)
  cold launch (avg)           2436ms       2034ms   (-17%)

Without the fix, lazyfree folios keep their stale refs/PG_workingset
and get protected instead of evicted. Since they sit on the file LRU,
page cache is reclaimed in their place, which shows up as more file
refaults and major faults.

Tested-by: Bo Zhang <zhangbo56@xiaomi.com>

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

[Kairui Song]

On Mon, Nov 9, 2026 at 10:16 AM 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.
>
> 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

Reviewed-by: Kairui Song <kasong@tencent.com>

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

[Liam R. Howlett]

On 26/11/09 10:16AM, Hui Zhu 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.
> 
> 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>

One nit..

Reviewed-by: Liam R. Howlett (Google) <liam@infradead.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);

Since you're not checking the return here, you could use
folio_set_lru_refs(folio, 0);
folio_clear_workingset(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()

[Hui Zhu]

2026/11/12 01:21, Liam R. Howlett:

> On 26/11/09 10:16AM, Hui Zhu 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.
>>
>> 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>
> One nit..
>
> Reviewed-by: Liam R. Howlett (Google) <liam@infradead.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);
> Since you're not checking the return here, you could use
> folio_set_lru_refs(folio, 0);
> folio_clear_workingset(folio);

Thanks for the review.

That is exactly the shape we agreed on for the follow-up cleanup in
this thread, Barry's __lru_gen_clear_refs() draft. I would still keep
v3 as it is, for two reasons.

folio_set_lru_refs() is only defined under CONFIG_LRU_GEN, so the
open-coded version needs a new stub in mm_inline.h. That turns this
one-spot fix into the header change we planned for the cleanup patch.

The two calls are also unconditional, so on the classic LRU every
lazyfree folio would lose PG_workingset too, which upstream never did
before cc8ec7be78ff. Barry and Kairui just agreed to take this patch
as a clean backport for stable, so that CLRU behaviour change should
go into the cleanup patch with its own justification.

lru_gen_clear_refs() is self-gated on gen >= 0, so the call is a
no-op for folios that are not on a generation list, and the return
value it computes shares the cacheline lruvec_del_folio() touches
right after. I will send the cleanup, with the open-coded clearing
and the stub in one place, once this lands.

Best,
Hui

>
>>   	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
>>