Re: [PATCH 6/7] mm/mglru: fix potential generation folio number leak

On 8/18/26 1:38 PM, Kairui Song via B4 Relay wrote:
> From: Kairui Song <[email protected]>
> 
> Each generation of MGLRU accounts anon and file folio numbers
> separately. The page table walker's update_batch_size() derives the
> anon / file type of a folio from its current flags, but the page table
> walk holds neither the lruvec lock nor the folio lock, so the type can
> change during that period.

Right.

> MADV_FREE's lazyfree path clears PG_swapbacked under the lruvec lock,
> so the folio is no longer considered on the anon LRU list. Lazyfreed
> folios can also be changed back to the anon list again. If the flip
> lands between folio_update_gen()'s cmpxchg and the type read in
> update_batch_size(), the batched delta pair is applied to the wrong
> type. The anon and file generation counters then carry phantom deltas
> that nothing reconciles, permanently skewing lrugen->nr_pages and the
> reclaim budgets derived from it.

But I think the problem occurs between update_batch_size() and 
sort_folio(). update_batch_size() only updates the anon or file folio 
statistics, while sort_folio() moves promoted folios to the 
corresponding type's list:

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

If the folio's anon/file type changes between these two steps (e.g., a 
lazyfree folio), it would lead to what you described: "The anon and file 
generation counters then carry phantom deltas that nothing reconciles, 
permanently skewing lrugen->nr_pages and the reclaim budgets derived 
from it."

If you agree that this is where the problem lies, I don't see a good way 
to fix it, since the state of a lazyfree folio can change between 
update_batch_size() and sort_folio().

A simple approach would be to skip checking the access flag for lazyfree 
folios during the page table walk, and let shrink_folio_list() 
reactivate accessed lazyfree folios instead. What do you think?

> Fix it by capturing the type from the flags snapshot the cmpxchg
> linearized against: folio_update_gen() returns the type of the state
> it transitioned from, and update_batch_size() accounts with it instead
> of re-reading the live flags. The batched deltas then always match the
> type of the state the cmpxchg transitioned from.
> 
> Fixes: 018ee47f1489 ("mm: multi-gen LRU: exploit locality in rmap")
> Signed-off-by: Kairui Song <[email protected]>
> ---
>   include/linux/mm_inline.h |  7 ++++++-
>   mm/vmscan.c               | 13 +++++++------
>   2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index df62daaa2ee7..4bb390d9516e 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -10,6 +10,11 @@
>   #include <linux/userfaultfd_k.h>
>   #include <linux/leafops.h>
>   
> +static inline int folio_flags_is_file_lru(const unsigned long *flags)
> +{
> +	return !test_bit(PG_swapbacked, flags);
> +}
> +
>   /**
>    * folio_is_file_lru - Should the folio be on a file LRU or anon LRU?
>    * @folio: The folio to test.
> @@ -27,7 +32,7 @@
>    */
>   static inline int folio_is_file_lru(const struct folio *folio)
>   {
> -	return !folio_test_swapbacked(folio);
> +	return folio_flags_is_file_lru(const_folio_flags(folio, 0));
>   }
>   
>   static __always_inline void __update_lru_size(struct lruvec *lruvec,
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index a613bb8d7271..7169cac60869 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3269,7 +3269,8 @@ static bool positive_ctrl_err(struct ctrl_pos *sp, struct ctrl_pos *pv)
>    ******************************************************************************/
>   
>   /* promote pages accessed through page tables */
> -static int folio_update_gen(struct folio *folio, int new_gen, const vma_flags_t *vma_flags)
> +static int folio_update_gen(struct folio *folio, int new_gen, int *is_file,
> +			    const vma_flags_t *vma_flags)
>   {
>   	unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0));
>   	int old_gen;
> @@ -3298,6 +3299,7 @@ static int folio_update_gen(struct folio *folio, int new_gen, const vma_flags_t
>   		new_flags |= BIT(PG_workingset);
>   	} while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>   
> +	*is_file = folio_flags_is_file_lru(&old_flags);
>   	return old_gen;
>   }
>   
> @@ -3328,9 +3330,8 @@ static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio)
>   }
>   
>   static void update_batch_size(struct lru_gen_mm_walk *walk, struct folio *folio,
> -			      int old_gen, int new_gen)
> +			      int old_gen, int new_gen, int type)
>   {
> -	int type = folio_is_file_lru(folio);
>   	int zone = folio_zonenum(folio);
>   	int delta = folio_nr_pages(folio);
>   
> @@ -3519,7 +3520,7 @@ static bool suitable_to_scan(int total, int young)
>   static void walk_update_folio(struct lru_gen_mm_walk *walk, struct vm_area_struct *vma,
>   			      struct lruvec *lruvec, struct folio *folio, bool dirty)
>   {
> -	int new_gen, old_gen;
> +	int new_gen, old_gen, file;
>   
>   	if (!folio)
>   		return;
> @@ -3532,9 +3533,9 @@ static void walk_update_folio(struct lru_gen_mm_walk *walk, struct vm_area_struc
>   		folio_mark_dirty(folio);
>   
>   	if (walk) {
> -		old_gen = folio_update_gen(folio, new_gen, &vma->flags);
> +		old_gen = folio_update_gen(folio, new_gen, &file, &vma->flags);
>   		if (old_gen >= 0 && old_gen != new_gen)
> -			update_batch_size(walk, folio, old_gen, new_gen);
> +			update_batch_size(walk, folio, old_gen, new_gen, file);
>   	} else if (lru_gen_set_refs(folio, &vma->flags)) {
>   		old_gen = folio_lru_gen(folio);
>   		if (old_gen >= 0 && old_gen != new_gen)
>