Re: [PATCH v7 05/16] mm/mglru: skip zones above reclaim_idx instead of rotating them

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mm/mglru: pinned min_seq in try_to_inc_min_seq() stalls aging
--

commit a9558af5b6eefdf9bb9a64058218b290ef8ecc66
Author: Kairui Song <kasong@tencent.com>

mm/mglru: skip zones above reclaim_idx instead of rotating them

This commit restricts scan_folios() and lruvec_populated_min_seq() to zones up
to sc->reclaim_idx instead of scanning and rotating folios in higher zones.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 51c2ee66b3ee8..273aa8c56bec8 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3992,7 +3992,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
>  
>  /* Similar to lruvec_evictable_size, return the oldest populated seq of the type */
>  static unsigned long lruvec_populated_min_seq(struct lruvec *lruvec, int type,
> -					      unsigned long max_seq)
> +					      int zone_limit, unsigned long max_seq)
>  {
>  	int gen, zone;
>  	unsigned long min_seq;
> @@ -4003,7 +4003,7 @@ static unsigned long lruvec_populated_min_seq(struct lruvec *lruvec, int type,
>  	min_seq = READ_ONCE(lrugen->min_seq[type]);
>  	while (min_seq <= max_seq) {
>  		gen = lru_gen_from_seq(min_seq);
> -		for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> +		for (zone = 0; zone < zone_limit; zone++) {
>  			if (!list_empty(&lrugen->folios[gen][type][zone]))
>  				return min_seq;
>  		}
> @@ -4026,7 +4026,7 @@ static void try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
>  	/* Requires at least MIN_NR_GENS gens for basic functionality */
>  	seq = lrugen->max_seq - MIN_NR_GENS;
>  	for_each_evictable_type(type, swappiness) {
> -		min_seq[type] = lruvec_populated_min_seq(lruvec, type, seq + 1);
> +		min_seq[type] = lruvec_populated_min_seq(lruvec, type, MAX_NR_ZONES, seq + 1);
>  		if (min_seq[type] > lrugen->min_seq[type])
>  			seq_inc_flag = true;
>  	}

[ ... ]

> @@ -4780,24 +4773,22 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	 * when under pressure to respect swappiness more and reduce IO cost.
>  	 */
>  	if (sc->priority == DEF_PRIORITY)
> -		min_seq = lrugen->min_seq[type];
> +		seq = lrugen->min_seq[type];
>  	else
> -		min_seq = lruvec_populated_min_seq(lruvec, type, lrugen->max_seq);
> +		seq = lrugen->max_seq;
>  
> -	gen = lru_gen_from_seq(min_seq);
> +	seq = lruvec_populated_min_seq(lruvec, type, sc->reclaim_idx + 1, seq);
> +	gen = lru_gen_from_seq(seq);
>  	max_gen = lru_gen_from_seq(max_seq);
>  
> -	for (i = MAX_NR_ZONES; i > 0; i--) {
> +	/* Scan eligible zones in descending order. */
> +	for (zone_idx = sc->reclaim_idx; zone_idx >= 0; zone_idx--) {

[Severity: High]
As reported by KunWu Chan on RFC v5 [1], does skipping zones above
sc->reclaim_idx in scan_folios() while try_to_inc_min_seq() still checks all
MAX_NR_ZONES stall aging during zone-constrained reclaim (for example,
GFP_DMA32 reclaim when ZONE_NORMAL is populated)?

When an lruvec has MAX_NR_GENS (4) generations, lrugen->min_seq[type] starts
at max_seq - 3.  Because scan_folios() no longer rotates folios in zones above
sc->reclaim_idx, any folios in those zones keep try_to_inc_min_seq() from
advancing lrugen->min_seq[type] once eligible zones are drained.

In should_run_aging():

mm/vmscan.c:should_run_aging() {
	...
	if (evictable_min_seq(min_seq, swappiness) + MIN_NR_GENS > max_seq)
		return true;

	/* try to avoid aging, do gentle reclaim at the default priority */
	if (sc->priority == DEF_PRIORITY)
		return false;

	/* better to run aging even though eviction is still possible */
	return evictable_min_seq(min_seq, swappiness) + MIN_NR_GENS == max_seq;
}

With min_seq pinned at max_seq - 3, evictable_min_seq() + MIN_NR_GENS never
reaches max_seq, so should_run_aging() returns false and scan_folios() drains
eligible zones down to the youngest generation max_seq where sort_folio()
disables tier protection and workingset promotion (min_gen == max_gen).

Even with the follow-up commits in this series (commit "mm/mglru: Run aging
when pages are rare outside the youngest gen", commit "mm/mglru: Run aging
when one type has no populatable folios", and commit "mm/mglru: Restrict
eviction accounting to eligible zones"), at priorities above DEF_PRIORITY / 2
scan_folios() caps seq at max_seq - 1 and evict_folio_lists() returns 0 once
older generations are empty in eligible zones, causing try_to_shrink_lruvec()
to break out before calling should_run_aging() until priority drops to
DEF_PRIORITY / 2 and scans max_seq without tier protection.

[1] https://lore.kernel.org/r/CAN_Opa-AScm9=EeXO_Uy8VjbQ=bqY9P++rFP8H1RhUB4dOt6xw@mail.gmail.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111232603.9934-1-baohua@kernel.org?part=5