Re: [PATCH 1/6] mm/mglru: batch update lrugen->nr_pages in inc_min_seq()

[Baoquan He <baoquan.he@linux.dev>]

Hi Barry,

On 08/21/26 at 06:25pm, Barry Song (Xiaomi) wrote:
> Currently, folio_inc_gen() updates lrugen->nr_pages for every folio
> as it advances generations. Instead, accumulate the size changes
> and update lrugen->nr_pages in a batch after scanning the entire
> oldest generation, or when the scan stops because remaining reaches
> zero.

This patch refactor code to split out __folio_inc_gen() and introduce 
gen_increased, this is the base of later patches. You seem to only
mention the minor optimization of lrugen->nr_pages. IMHO, this
refactoring can be split out to an independent patch. The lrugen->nr_pages
can be integrated with other patch, e.g patch 2 or patch 6.

> 
> Since we only move folios from the oldest generation to the second
> oldest generation, the active/inactive state cannot change. We can
> therefore skip __lru_update_size().

For the justification of skipping __lru_update_size(), there's a
precondition: get_nr_gens(lruvec, type) == MAX_NR_GENS; With this, the
oldest gen and 2nd oldest gen are both inactive. Calling
__lru_update_size() may waste tiny cpu, while skipping it may cause
issue in future if the precondition is changed. Do you think adding
a VM_WARN_ON_ONCE is necessary?

VM_WARN_ON_ONCE(lru_gen_is_active(lruvec, old_gen) ||
		lru_gen_is_active(lruvec, target_gen));


Thanks
Baoquan
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
>  mm/vmscan.c | 46 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c1404a59523d..0d74fc00abd3 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3296,20 +3296,21 @@ static int folio_update_gen(struct folio *folio, int gen, const vma_flags_t *vma
>  }
>  
>  /* protect pages accessed multiple times through file descriptors */
> -static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio)
> +static int __folio_inc_gen(struct folio *folio, int old_gen, bool *increased)
>  {
> -	int type = folio_is_file_lru(folio);
> -	struct lru_gen_folio *lrugen = &lruvec->lrugen;
> -	int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]);
>  	unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f);
> +	int new_gen;
>  
>  	VM_WARN_ON_ONCE_FOLIO(!(old_flags & LRU_GEN_MASK), folio);
>  
>  	do {
>  		new_gen = ((old_flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1;
>  		/* folio_update_gen() has promoted this page? */
> -		if (new_gen >= 0 && new_gen != old_gen)
> +		if (new_gen >= 0 && new_gen != old_gen) {
> +			if (increased)
> +				*increased = false;
>  			return new_gen;
> +		}
>  
>  		new_gen = (old_gen + 1) % MAX_NR_GENS;
>  
> @@ -3317,8 +3318,21 @@ static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio)
>  		new_flags |= (new_gen + 1UL) << LRU_GEN_PGOFF;
>  	} while (!try_cmpxchg(&folio->flags.f, &old_flags, new_flags));
>  
> -	lru_gen_update_size(lruvec, folio, old_gen, new_gen);
> +	if (increased)
> +		*increased = true;
> +	return new_gen;
> +}
>  
> +static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio)
> +{
> +	int type = folio_is_file_lru(folio);
> +	struct lru_gen_folio *lrugen = &lruvec->lrugen;
> +	int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]);
> +	bool gen_increased;
> +
> +	new_gen = __folio_inc_gen(folio, old_gen, &gen_increased);
> +	if (gen_increased)
> +		lru_gen_update_size(lruvec, folio, old_gen, new_gen);
>  	return new_gen;
>  }
>  
> @@ -3904,6 +3918,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
>  	struct lru_gen_folio *lrugen = &lruvec->lrugen;
>  	int hist = lru_hist_from_seq(lrugen->min_seq[type]);
>  	int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]);
> +	int target_gen = (old_gen + 1) % MAX_NR_GENS;
>  
>  	/* For file type, skip the check if swappiness is anon only */
>  	if (type && (swappiness == SWAPPINESS_ANON_ONLY))
> @@ -3916,32 +3931,41 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
>  	/* prevent cold/hot inversion if the type is evictable */
>  	for (zone = 0; zone < MAX_NR_ZONES; zone++) {
>  		struct list_head *head = &lrugen->folios[old_gen][type][zone];
> +		unsigned long delta = 0;
>  
>  		while (!list_empty(head)) {
>  			struct folio *folio = lru_to_folio(head);
> +			long nr_pages = folio_nr_pages(folio);
>  			int refs = folio_lru_refs(folio);
>  			bool workingset = folio_test_workingset(folio);
> +			bool gen_increased;
>  
>  			VM_WARN_ON_ONCE_FOLIO(folio_test_unevictable(folio), folio);
>  			VM_WARN_ON_ONCE_FOLIO(folio_test_active(folio), folio);
>  			VM_WARN_ON_ONCE_FOLIO(folio_is_file_lru(folio) != type, folio);
>  			VM_WARN_ON_ONCE_FOLIO(folio_zonenum(folio) != zone, folio);
>  
> -			new_gen = folio_inc_gen(lruvec, folio);
> +			new_gen = __folio_inc_gen(folio, old_gen, &gen_increased);
>  			list_move_tail(&folio->lru, &lrugen->folios[new_gen][type][zone]);
> -
> +			if (gen_increased)
> +				delta += nr_pages;
>  			/* don't count the workingset being lazily promoted */
>  			if (refs + workingset != BIT(LRU_REFS_WIDTH) + 1) {
>  				int tier = lru_tier_from_refs(refs, workingset);
> -				int delta = folio_nr_pages(folio);
>  
>  				WRITE_ONCE(lrugen->protected[hist][type][tier],
> -					   lrugen->protected[hist][type][tier] + delta);
> +					   lrugen->protected[hist][type][tier] + nr_pages);
>  			}
>  
>  			if (!--remaining)
> -				return false;
> +				break;
>  		}
> +		WRITE_ONCE(lrugen->nr_pages[old_gen][type][zone],
> +			   lrugen->nr_pages[old_gen][type][zone] - delta);
> +		WRITE_ONCE(lrugen->nr_pages[target_gen][type][zone],
> +			   lrugen->nr_pages[target_gen][type][zone] + delta);
> +		if (!remaining)
> +			return false;
>  	}
>  done:
>  	reset_ctrl_pos(lruvec, type, true);
> -- 
> 2.34.1
>