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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] mm: inconsistent PGSCAN stat accounting in scan_folios() [1]
--

commit 91b0962145533fb9dabae36c077150538170ace0
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 rather than rotating folios in ineligible zones to a
younger generation.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index db0a8a3e88872..6b16706779744 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c

[ ... ]

> @@ -4758,16 +4750,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);
> @@ -4779,24 +4772,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--) {
>  		LIST_HEAD(moved);
>  		int skipped_zone = 0;
> -		int zone = (sc->reclaim_idx + i) % MAX_NR_ZONES;
> -		struct list_head *head = &lrugen->folios[gen][type][zone];
> +		struct zone *zone = &pgdat->node_zones[zone_idx];
> +		struct list_head *head = &lrugen->folios[gen][type][zone_idx];
>  
> -		/*
> -		 * If there is only one gen left, rotating the unreclaimable
> -		 * zone become meaningless, just skip them.
> -		 */
> -		if (gen == max_gen && zone > sc->reclaim_idx)
> +		if (!managed_zone(zone))
>  			continue;
>  
>  		while (!list_empty(head)) {
> @@ -4806,7 +4797,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  			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);
> +			VM_WARN_ON_ONCE_FOLIO(folio_zonenum(folio) != zone_idx, folio);
>  
>  			scanned += delta;
>  			if (sort_folio(lruvec, folio, sc, gen, max_gen, tier))
> @@ -4825,7 +4816,7 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  
>  		if (skipped_zone) {
>  			list_splice(&moved, head);
> -			__count_zid_vm_events(PGSCAN_SKIP, zone, skipped_zone);
> +			__count_zid_vm_events(PGSCAN_SKIP, zone_idx, skipped_zone);
>  			skipped += skipped_zone;
>  		}
>  

[Severity: Medium]
This problem wasn't introduced by this patch, but should scan_folios() pass
scanned rather than isolated to mod_lruvec_state() for PGSCAN_KSWAPD and
PGSCAN_ANON + type [1]?

mm/vmscan.c:scan_folios() {
	...
	item = PGSCAN_KSWAPD + reclaimer_offset(sc);
	mod_lruvec_state(lruvec, item, isolated);
	mod_lruvec_state(lruvec, PGREFILL, sorted);
	mod_lruvec_state(lruvec, PGSCAN_ANON + type, isolated);
	...
}

Folios that are promoted via sort_folio() or skipped are counted in
scanned, but passing isolated here omits them from the PGSCAN vmstat
counters and underreports the scan rate compared to the classic LRU path.

[1] https://sashiko.dev/#/bug/linux-ebd28175-5aad-4454-b9f3-a9510e3c4655

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