Re: [RFC PATCH v2 1/5] mm: mglru: avoid scanning empty generations in scan_folios()

[Lian Wang]

Hi Barry,

I tested a list-based exhaustion check on top of the v2 series.

Using remaining > 0 still misses the case where remaining reaches zero
exactly when the last folio leaves the oldest generation. Since
scan_folios() still holds the lruvec lock, checking the generation's zone
lists directly avoids both this boundary and the scanned/nr_to_scan
page-versus-folio mismatch.

The change below only replaces the exhaustion check. It keeps the
priority-gated fallback unchanged, and reports the MIN_NR_GENS early
return as exhausted to preserve the current v2 behavior.

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 6a73be7590cd..59b8725a0650 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4635,7 +4635,8 @@ static bool isolate_folio(struct lruvec *lruvec, struct folio *folio, struct sca
 
 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)
+		       struct list_head *list, int *isolatedp,
+		       bool *scan_exhausted)
 {
 	int i;
 	int gen;
@@ -4650,8 +4651,10 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
 	VM_WARN_ON_ONCE(!list_empty(list));
 
-	if (get_nr_gens(lruvec, type) == MIN_NR_GENS)
+	if (get_nr_gens(lruvec, type) == MIN_NR_GENS) {
+		*scan_exhausted = true;
 		return 0;
+	}
 
 	gen = lru_gen_from_seq(lrugen->min_seq[type]);
 
@@ -4704,6 +4707,14 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 				scanned, skipped, isolated,
 				type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
 
+	*scan_exhausted = true;
+	for (i = 0; i < MAX_NR_ZONES; i++) {
+		if (!list_empty(&lrugen->folios[gen][type][i])) {
+			*scan_exhausted = false;
+			break;
+		}
+	}
+
 	*isolatedp = isolated;
 	return scanned;
 }
@@ -4757,11 +4768,13 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	int type = get_type_to_scan(lruvec, swappiness);
 
 	for_each_evictable_type(i, swappiness) {
+		bool scan_exhausted;
 		int scanned;
 		int tier = get_tier_idx(lruvec, type);
 
 		scanned = scan_folios(nr_to_scan, lruvec, sc,
-				      type, tier, list, isolated);
+				      type, tier, list, isolated,
+				      &scan_exhausted);
 
 		total_scanned += scanned;
 		if (*isolated) {
@@ -4770,15 +4783,12 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 			break;
 		}
 		/*
-		 * If scanned >= nr_to_scan or isolated >= MIN_LRU_BATCH,
-		 * avoid falling back to the other type. The preferred
-		 * type is still reclaimable; otherwise, it would have
-		 * already run out of reclaimable generations. Falling
-		 * back too readily can disrupt the positive_ctrl_err()
-		 * bias. Also, only fall back when reclaim is running at
-		 * a high priority.
+		 * The preferred type is still reclaimable unless this scan
+		 * target is exhausted. Preserve the existing priority-gated
+		 * fallback without inferring exhaustion from scanned, which
+		 * counts base pages rather than iterations.
 		 */
-		if (scanned < nr_to_scan && *isolated < MIN_LRU_BATCH) {
+		if (scan_exhausted) {
 			if (sc->priority > 2)
 				break;
 			type = !type;

I built and booted this on an arm64 VM with MGLRU enabled. A fresh
swappiness=200 pressure smoke with a 2 GiB file-cache setup and about
7 GiB of touched anonymous memory completed without a
WARN/Oops/BUG/panic.

Would this be worth considering for the next revision? If I misunderstood
the intended semantics, please ignore this.

Thanks,
Lian