[PATCH v2 0/2] mm/mglru: clean up isolate_folios for readability and clarity

[PATCH v2 0/2] mm/mglru: clean up isolate_folios for readability and clarity

[Barry Song (Xiaomi)]

Right now, `isolate_folios()` is quite difficult to follow:

1. It uses `for_each_evictable_type(i, swappiness)` to iterate over
   the types, but `i` is not actually used as the type within the
   loop body.

2. It retries the same type when folios were scanned but none could
   be isolated, but the retry is implemented in a rather subtle way
   that is difficult to understand.

This patchset makes both behaviors explicit and much easier to follow.

There are no functional changes for swappiness values from 1 to 200.
There is a slight functional change for 0 and 201: with the existing
code, there is no chance to retry for these values because
`for_each_evictable_type()` only iterates once. After this patch, 0 and
201 have behavior that is more consistent with the 1-200 range.

-v2:
* Rename patch 1, to address Baoquan's comments;
* Drop patches 2/3. Patch 2 seems to improve the zRAM case
  but negatively affect the SSD/NVMe case, according to Baolin
  and Kairui. Drop it to keep the patchset focused on readability.
* We received many tags from Baolin, Kairui, Ridong, and Lian.
  Since patch 1/3 and 3/3 were renamed and patches 2/3 were dropped,
  I did not carry the tags forward. Many thanks for the reviews,
  and hopefully you can re-review this version.
-v1:
  https://lore.kernel.org/linux-mm/20260820045603.68809-1-baohua@kernel.org/

Barry Song (Xiaomi) (1):
  mm/mglru: make retry logic explicit in isolate_folios()

Ridong Chen (1):
  mm/mglru: make type fallback logic explicit in isolate_folios()

 mm/vmscan.c | 56 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] mm/mglru: make type fallback logic explicit in isolate_folios()

[Barry Song (Xiaomi)]

From: Ridong Chen <chenridong@xiaomi.com>

The for_each_evictable_type() loop in isolate_folios()
is misleading: it does not actually iterate over each
evictable type. Instead, get_type_to_scan() selects the
type to scan, while the iterator `i` merely bounds the
number of attempts.

Make the fallback behavior explicit in the code and remove the
opaque for_each_evictable_type(i, swappiness).

Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
Co-developed-by: Barry Song (Xiaomi) <baohua@kernel.org>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/vmscan.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index fdd13299a04a..35a233623368 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4838,35 +4838,41 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
 	return positive_ctrl_err(&sp, &pv);
 }
 
+static inline bool is_single_type_reclaim(int swappiness)
+{
+	return swappiness == MIN_SWAPPINESS ||
+	       swappiness == SWAPPINESS_ANON_ONLY;
+}
+
 static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 			  struct scan_control *sc, int swappiness,
 			  struct list_head *list, int *isolated,
 			  int *isolate_type, int *isolate_scanned)
 {
-	int i;
-	int total_scanned = 0;
+	bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
 	int type = get_type_to_scan(lruvec, swappiness);
+	int total_scanned = 0, scanned, tier;
 
-	for_each_evictable_type(i, swappiness) {
-		int scanned;
-		int tier = get_tier_idx(lruvec, type);
+retry:
+	tier = get_tier_idx(lruvec, type);
+	scanned = scan_folios(nr_to_scan, lruvec, sc,
+			      type, tier, list, isolated);
 
-		scanned = scan_folios(nr_to_scan, lruvec, sc,
-				      type, tier, list, isolated);
+	total_scanned += scanned;
+	if (*isolated) {
+		*isolate_type = type;
+		*isolate_scanned = scanned;
+		return total_scanned;
+	}
 
-		total_scanned += scanned;
-		if (*isolated) {
-			*isolate_type = type;
-			*isolate_scanned = scanned;
-			break;
-		}
-		/*
-		 * If scanned > 0 and isolated == 0, avoid falling back to the
-		 * other type, as this type remains sufficient. Falling back
-		 * too readily can disrupt the positive_ctrl_err() bias.
-		 */
-		if (!scanned)
-			type = !type;
+	/*
+	 * We are running out of the current reclaim type. Fall back to
+	 * the other type if allowed.
+	 */
+	if (!scanned && type_fallback_allowed) {
+		type = !type;
+		type_fallback_allowed = false;
+		goto retry;
 	}
 
 	return total_scanned;
-- 
2.34.1

[PATCH v2 2/2] mm/mglru: make retry logic explicit in isolate_folios()

[Barry Song (Xiaomi)]

The existing mainline code retries the same type once in a rather
subtle way. `for_each_evictable_type()` may provide one more iteration,
allowing the same type to be retried if we scanned some folios but
failed to isolate any due to protections, promotions, or races. This
patch makes the retry behavior explicit.

Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/vmscan.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 35a233623368..718f59ffc688 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4852,6 +4852,7 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	bool type_fallback_allowed = !is_single_type_reclaim(swappiness);
 	int type = get_type_to_scan(lruvec, swappiness);
 	int total_scanned = 0, scanned, tier;
+	bool tried = false;
 
 retry:
 	tier = get_tier_idx(lruvec, type);
@@ -4871,9 +4872,18 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	 */
 	if (!scanned && type_fallback_allowed) {
 		type = !type;
+		tried = true;
 		type_fallback_allowed = false;
 		goto retry;
 	}
+	/*
+	 * We scanned some folios but failed to isolate any due to promotions,
+	 * protections, or races. Retry once to avoid a larger loop.
+	 */
+	if (scanned && !tried) {
+		tried = true;
+		goto retry;
+	}
 
 	return total_scanned;
 }
-- 
2.34.1