Re: [PATCH v2 2/7] mm/mglru: batch update lrugen->nr_pages in inc_min_seq()

[Kunwu Chan]

On Fri, 28 Aug 2026 07:46:59 +0800 "Barry Song (Xiaomi)" <baohua@xxxxxxxxxx> 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.
> 
> 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().
> 
> Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> Tested-by: Xueyuan Chen <xueyuan.chen21@xxxxxxxxx>
> ---
>  mm/vmscan.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 8f187d296b8e..07c22d51debd 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3918,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))
> @@ -3927,35 +3928,47 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
>  	if (!type && !swappiness)
>  		goto done;
>  
> +	VM_WARN_ON_ONCE(get_nr_gens(lruvec, type) != MAX_NR_GENS);
> +	VM_WARN_ON_ONCE(lru_gen_is_active(lruvec, old_gen) !=
> +			lru_gen_is_active(lruvec, target_gen));
>  	/* 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];
> +		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);

Hi Barry,

One subtle point about the `remaining` handling:
when `remaining` reaches zero, we now `break` rather than return
so that the accumulated `delta` is applied before returning.

As I understand it, this is required because `__folio_inc_gen()`
has already changed the generation of the scanned folios,
while `lrugen->nr_pages[]` is now updated only in batch.

So the invariant is that every successful generation increment must
have its corresponding `delta` flushed before `inc_min_seq()` returns.

Is this the intended accounting invariant?

Thanks,
KunWu

> +		if (!remaining)
> +			return false;
>  	}
>  done:
>  	reset_ctrl_pos(lruvec, type, true);
> -- 
> 2.34.1
> 
>