Re: [RFC PATCH] mm/mglru: Avoid reclaiming kept folios during retrying missed rotated folios

[Baolin Wang <baolin.wang@linux.alibaba.com>]

Hi Barry,

On 10/3/26 1:42 PM, Barry Song (Xiaomi) wrote:
> Since commit 4d5d14a01e2c ("mm/mglru: rework workingset protection"),
> `folio_test_referenced()` was accidentally removed when checking for
> missed rotated folios. As a result, kept folios without an active set
> could be added to the retry list and eventually reclaimed.
> 
> The impact should be very small, as we have the `folio_mapped(folio)`
> check before retrying. Kept folios won't have `try_to_unmap()` called,
> so this is really only applicable to folios that are unmapped by users
> after `folio_referenced()` has been called.
> 
> I'm not sure if anyone else has seen any issues with this. Sending an
> RFC to check whether it is worth fixing.
> 
> Fixes: 4d5d14a01e2c ("mm/mglru: rework workingset protection")
> Cc: Yu Zhao <yuzhao@google.com>
> Reported-by: Bo Zhang <zhangbo56@xiaomi.com>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
>   mm/vmscan.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 91295070ca33..6a931b576b7d 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -994,8 +994,10 @@ static enum folio_references folio_check_references(struct folio *folio,
>   		return FOLIOREF_KEEP;
>   
>   	if (lru_gen_enabled() && !lru_gen_switching()) {
> -		if (!referenced_ptes)
> +		if (!referenced_ptes) {
> +			folio_clear_referenced(folio);
>   			return FOLIOREF_RECLAIM;
> +		}

I've also been investigating the issue of referenced folios being 
reclaimed recently. The background is that classical LRU will return 
FOLIOREF_KEEP for folios accessed once via page table, giving the 
accessed folio another chance.

However, for MGLRU, if the folio's pagetable access has already been 
consumed by aging or lru_gen_look_around(), referenced_ptes may be 0 at 
that point, leading to reclaiming folios that were accessed via page 
table, which could cause more refaults (I don't have data yet).

But it's not easy to maintain logic consistent with classical LRU, 
because we cannot distinguish whether this one access count comes from 
page table or file descriptors (for anonymous pages, it can be 
distinguished). I'm not sure if Kairui's MGLRU-FG patchset helps with 
this case (I haven't looked at it carefully yet).

Perhaps at least for anonymous pages, if the referenced flag is set, we 
should return FOLIOREF_KEEP to give it another scan chance.

>   		return lru_gen_set_refs(folio, &vma_flags) ? FOLIOREF_ACTIVATE : FOLIOREF_KEEP;
>   	}
> @@ -5110,7 +5112,8 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   			continue;
>   
>   		/* retry folios that may have missed folio_rotate_reclaimable() */
> -		if (!skip_retry && !folio_test_active(folio) && !folio_mapped(folio) &&
> +		if (!skip_retry && !folio_test_active(folio) &&
> +		    !folio_test_referenced(folio) && !folio_mapped(folio) &&

This part makes sense to me.

>   		    !folio_test_dirty(folio) && !folio_test_writeback(folio)) {
>   			list_move(&folio->lru, &clean);
>   			continue;