Re: [RFC PATCH v5 05/14] 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(s) to consider:
- [High] mm/mglru: pinned min_seq in higher zones stalls aging in scan_folios()
--

commit 1b2f683d6b8bd1e171eb9677315ba8a3d9431b9b
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() during
eviction to zones at or below sc->reclaim_idx, and removes the ineligible-zone
rotation branch from sort_folio(). try_to_inc_min_seq() continues to check all
zones up to MAX_NR_ZONES before advancing min_seq.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index a365a4bc8c988..09ea72b354a56 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4132,7 +4132,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;
> @@ -4143,7 +4143,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;
>  		}
> @@ -4166,7 +4166,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;
>  	}

[ ... ]

> @@ -4863,10 +4864,8 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
>  	 * longer be promoted or protected, so just isolate them. The one
>  	 * exception is lru_gen_reparent_memcg(), which is handled above.
>  	 */
> -	if (min_gen == max_gen) {
> -		VM_WARN_ON_ONCE(zone > sc->reclaim_idx);
> +	if (min_gen == max_gen)
>  		return false;
> -	}
>  
>  	/* protected */
>  	if (tier > tier_idx || refs + workingset == BIT(LRU_REFS_WIDTH) + 1) {
> @@ -4883,13 +4882,6 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
>  		return true;
>  	}
>  
> -	/* ineligible */
> -	if (zone > sc->reclaim_idx) {
> -		gen = folio_inc_gen(lruvec, folio, min_gen);
> -		list_move_tail(&folio->lru, &lrugen->folios[gen][type][zone]);
> -		return true;
> -	}
> -
>  	return false;
>  }
>  
> @@ -4921,16 +4913,17 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  		       struct scan_control *sc, int type, int tier,
>  		       struct list_head *list, int *isolatedp)
>  {
> -	int i;
>  	enum node_stat_item item;
> +	int zone_idx;
>  	int sorted = 0;
>  	int scanned = 0;
>  	int isolated = 0;
>  	int skipped = 0;
>  	int gen, max_gen;
> -	unsigned long min_seq;
> +	unsigned long seq;
>  	unsigned long remaining = nr_to_scan;
>  	struct lru_gen_folio *lrugen = &lruvec->lrugen;
> +	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>  	DEFINE_MAX_SEQ(lruvec);
>  
>  	VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
> @@ -4942,24 +4935,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]
During zone-constrained reclaim in mm/vmscan.c (where sc->reclaim_idx is below
MAX_NR_ZONES - 1 and zones above sc->reclaim_idx are populated, such as
GFP_DMA32 reclaim with a populated ZONE_NORMAL or GFP_KERNEL reclaim with a
populated ZONE_MOVABLE), can keeping MAX_NR_ZONES in try_to_inc_min_seq()
while removing the ineligible-zone rotation from sort_folio() stall aging and
memcg rotation?

Previously, scan_folios() visited all zones and sort_folio() rotated folios in
zones above sc->reclaim_idx from min_gen to min_gen + 1. Once the oldest
generation was emptied across all zones, try_to_inc_min_seq() incremented
lrugen->min_seq[type] from max_seq - 3 (when MAX_NR_GENS generations exist) to
max_seq - 2, allowing should_run_aging() to trigger try_to_inc_max_seq().

With scan_folios() restricted to zones 0..sc->reclaim_idx, folios in zones
above sc->reclaim_idx remain in min_seq[type]. Because try_to_inc_min_seq()
still passes MAX_NR_ZONES to lruvec_populated_min_seq(), lrugen->min_seq[type]
stays pinned at max_seq - 3 even after min_seq[type] is completely drained in
eligible zones 0..sc->reclaim_idx.

In this commit, at sc->priority == DEF_PRIORITY, scan_folios() caps the
lruvec_populated_min_seq() search bound at seq = lrugen->min_seq[type]. Once
min_seq[type] is empty in zones 0..sc->reclaim_idx, lruvec_populated_min_seq()
still returns the pinned min_seq[type], so scan_folios() scans empty lists and
returns 0 at DEF_PRIORITY.

A later commit in this series ("mm/mglru: lift youngest gen protection for
higher priority") changes the DEF_PRIORITY search bound in scan_folios() to
lrugen->max_seq - 1, and another commit ("mm/mglru: Run aging when pages are
rare outside the youngest gen") adds lru_gen_imbalanced() to
should_run_aging(). However, both should_run_aging() and shrink_one() still
rely on state across all zones:

mm/vmscan.c:should_run_aging() {
	DEFINE_MIN_SEQ(lruvec);

	/* have to run aging, since eviction is not possible anymore */
	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;
}

mm/vmscan.c:shrink_one() {
	...
	need_rotate = try_to_shrink_lruvec(lruvec, sc);
	...
	if (need_rotate && mem_cgroup_online(memcg))
		return MEMCG_LRU_YOUNG;

	if (!need_rotate && lruvec_is_sizable(lruvec, sc))
		return 0;
}

When lrugen->min_seq[type] remains pinned at max_seq - 3 by folios in zones
above sc->reclaim_idx (and lru_gen_imbalanced() / lruvec_is_sizable() sum
nr_pages across all MAX_NR_ZONES):

1. should_run_aging() sees min_seq + MIN_NR_GENS < max_seq and does not run
   try_to_inc_max_seq() (or inc_min_seq() inside inc_max_seq()), stalling
   page-table aging.

2. Once eligible zones 0..sc->reclaim_idx drain their older generations and
   lruvec_populated_min_seq() advances seq to max_seq in scan_folios(),
   sort_folio() hits min_gen == max_gen and isolates folios from the youngest
   generation without tier or reference protection.

3. If a memcg has no folios in eligible zones 0..sc->reclaim_idx but has
   folios in zones above sc->reclaim_idx, try_to_shrink_lruvec() returns
   need_rotate = false while lruvec_is_sizable() returns true, so shrink_one()
   returns 0 instead of rotating the memcg.

Should should_run_aging() and the size checks also account for sc->reclaim_idx,
or how should min_seq and max_seq advance when older generations are only
populated in zones above sc->reclaim_idx?

[ ... ]

([PVF1])

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