mm/vmscan: fix NR_ISOLATED accounting and throttling for MGLRU

From: Hui Zhu <[email protected]>

The legacy reclaim path updates the NR_ISOLATED_ANON/FILE node counters around isolation and throttles direct reclaimers via too_many_isolated() when isolated folios pile up. The MGLRU eviction path does neither: evict_folios() isolates folios without touching the counters and never consults too_many_isolated().

Patch 1 updates NR_ISOLATED_ANON/FILE around isolation in evict_folios(), reusing the existing nr_isolated. Without this the counters stay at zero while MGLRU reclaim is active, so compaction's too_many_isolated() cannot see the pages MGLRU has isolated.

Patch 2 adds throttle_evictable_types() and calls it from evict_folios(), before the lruvec lock is taken since throttling sleeps, leaving the legacy path untouched. The MGLRU check differs from the legacy per-list one in shrink_inactive_list() because isolate_folios() picks the type to scan from the refault feedback and may fall back to the other one: it computes the set of evictable types that are not over-isolated and only sleeps when all of them are, waiting once for concurrent reclaimers exactly like the legacy path. The mask of the remaining types is passed to isolate_folios(), which restricts both its initial choice and its fallback to it, so isolation never lands on an over-isolated type and a type that is merely over-isolated never blocks the reclaim of the other one. This way the MGLRU eviction path backs off when isolated folios pile up instead of thrashing the shrinking LRU lists - the scenario the too_many_isolated() check exists for. A dying task fakes reclaim progress exactly like the legacy path so it exits reclaim quickly.

Testing
=======

Test on 8G RAM qemu.
The reproducer confines stress-ng workers in a 192M memcg and swaps through dm-delay (300ms write latency) so pageout is slow and isolated folios pile up; the workload is intentionally extreme. nr_isolated_* is sampled every 50ms against the per-type too_many_isolated threshold (inactive/8), and throttle events are counted via the mm_vmscan_throttled tracepoint.
The test scripts and test log are in [1].

Test 1, reclaim throttling, parallel direct reclaim in the memcg:

before after

throttle events (ISOLATED)        0            0
  - from kswapd                   0            0
nr_isolated_anon peak             0         3166
nr_isolated_file peak             0          174
  - samples above the
    too_many_isolated threshold  0/1088     89/1077

pgscan_direct 1540044096 858332151
pswpout 6299497 725819

Without the series MGLRU reclaim spins on the shrinking LRU lists while nr_isolated_* stays at 0 and nothing is throttled:
pgscan_direct runs to 1.5 billion and 6.3M pages are pushed through the 300ms swap device, i.e. the memcg anon is recycled over and over. With the series the counters are updated and the too_many_isolated threshold is crossed (89 of 1077 samples). Only anon is over-isolated in this workload, so the throttle never has to sleep: it stops reclaimers from isolating more anon and has them fall back to file instead (nr_isolated_file peaks at 174 instead of 0), and anon isolation stays around the threshold (peak 3166). Scanning drops by ~45% and the swap-out storm by ~9x: the reclaimers stop hammering the shrinking anon LRU while the slow swap device is still writing out the previous batches. kswapd stays exempt.

Test 2, counters visible to compaction, same pressure plus compact_memory in parallel:

before after
nr_isolated peak 456 2232

The "before" 456 is compaction's own transient isolation; reclaim's isolation is invisible. With patch 1 it becomes visible to compaction's too_many_isolated(). (Compaction's own throttling threshold, (inactive + active) / 16, is about 25k pages on this box and needs more pile-up than the box can generate; test 1 shows the same too_many_isolated() check firing on the reclaim side.)

[1] https://gist.github.com/teawater/d3968aac92eb6bd1378beb54a82933f4

Changelog:
v4:
According to the commens of Baolin and Barry, rework patch 2:
drop the throttle_is_throttled() helper extracted
from shrink_inactive_list() and leave the legacy path untouched. The new MGLRU-only throttle_evictable_types() only sleeps when all evictable types are over-isolated - v3 throttled as soon as any of them was, which unnecessarily blocked the reclaim of the other type and passes the mask of the remaining types to isolate_folios(), which restricts both its initial choice and its fallback, so that isolation never lands on a throttled type. The v3 gate did not constrain the type actually isolated, so the fallback could still pick the over-isolated one.
Re-run the tests and update the test log.
v3:
According to the commens of Baolin, remove the redundant nr_isolated check before restoring the NR_ISOLATED_* counters in evict_folios(). rename the extracted helper to throttle_is_throttled() to avoid confusion with the existing wake_throttle_isolated() naming space. Use for_each_evictable_type() in the MGLRU throttle to check each evictable type's isolation instead of only the type returned by get_type_to_scan(), since isolate_folios() may fall back to the other type.
Re-run the tests and update the test log.
v2:
According to the commens of Kairun, Rebased on mm-unstable. Split into two patches; patch 2 is new and adds the too_many_isolated() throttling to the MGLRU eviction path, which v1 did not cover.
Add test infomations.

Hui Zhu (2):
mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim
path
mm/vmscan: apply too_many_isolated() throttling to MGLRU eviction

 mm/vmscan.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 85 insertions(+), 5 deletions(-)

2.53.0

From: Hui Zhu <[email protected]>

MGLRU evict_folios() isolates folios from the LRU without updating the NR_ISOLATED_ANON/FILE counters, unlike the legacy shrink_inactive_list() path. This causes compaction's too_many_isolated() check to under-count isolated pages when MGLRU reclaim is active.

Add NR_ISOLATED counter updates in evict_folios(): increment after isolate_folios() and decrement after all retry passes complete, using the existing nr_isolated which holds the original isolated count.

---
 mm/vmscan.c | 5 +++++
 1 file changed, 5 insertions(+)

index c1404a59523d..98226bb021f3 100644
@@ -4892,6 +4892,9 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness,
 				 &list, &isolated, &type, &type_scanned);
 	nr_isolated = isolated;
+	if (nr_isolated)
+		__mod_node_page_state(pgdat, NR_ISOLATED_ANON + type,
+				      nr_isolated);
 
 	/* Scanning may have emptied the oldest gen, flush it */
 	if (scanned)

@@ -4954,6 +4957,8 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 		goto retry;
 	}
 
+	mod_node_page_state(pgdat, NR_ISOLATED_ANON + type, -nr_isolated);
+
 	if (nr_isolated > total_reclaimed)
 		mod_lruvec_state(lruvec, PGROTATE_ANON + type,
 				 nr_isolated - total_reclaimed);

2.53.0

From: Hui Zhu <[email protected]>

The legacy path throttles direct reclaim in shrink_inactive_list() when too many isolated folios pile up, but MGLRU's evict_folios() isolates folios without this check, which can lead to unnecessary swapping, thrashing and OOM.

With the NR_ISOLATED counters now updated in evict_folios(), add throttle_evictable_types() and call it from evict_folios(), before the lruvec lock is taken since throttling sleeps. The legacy path is left untouched.

Unlike the legacy path, where the LRU list to isolate from is known before isolation, isolate_folios() picks the type to scan from the refault feedback and may fall back to the other one. Therefore, instead of throttling on a single type, throttle_evictable_types() collects the evictable types that do not have too many isolated folios, and only sleeps when all of them do: like the legacy path, it waits once for concurrent reclaimers to put their isolated folios back, and gives up if that makes no progress.

The resulting mask of the types that are not over-isolated is passed to isolate_folios(), which restricts both its initial choice and its fallback to it. This way a type that is merely over-isolated never blocks the reclaim of the other type, and isolation never lands on a throttled type.

If a fatal signal is pending, fake reclaim progress the same way the legacy path does, so the dying task exits reclaim quickly instead of being held in the throttle.

---
 mm/vmscan.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 80 insertions(+), 5 deletions(-)

index 98226bb021f3..693dc91a1695 100644
@@ -4833,15 +4833,73 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
 	return positive_ctrl_err(&sp, &pv);
 }
 
+/*
+ * Unlike the legacy path, where the LRU list to isolate from is known
+ * before isolation, isolate_folios() picks the type from the refault
+ * feedback and may fall back to the other one. Therefore, instead of
+ * throttling on a single type, collect the evictable types that do not
+ * have too many isolated folios, and only sleep when all of them do.
+ *
+ * Returns the mask of the types isolate_folios() may isolate from, or
+ * 0 if reclaim should stop. Also sets @fatal to tell the caller that
+ * the task received a fatal signal while waiting.
+ */
+static unsigned int throttle_evictable_types(struct pglist_data *pgdat,
+					     int swappiness,
+					     struct scan_control *sc,
+					     bool *fatal)
+{
+	unsigned int allowed;
+	bool stalled = false;
+	int i;
+
+	*fatal = false;
+
+	for (;;) {
+		allowed = 0;
+		for_each_evictable_type(i, swappiness) {
+			if (!too_many_isolated(pgdat, i, sc))
+				allowed |= BIT(i);
+		}
+
+		if (allowed)
+			return allowed;
+
+		/*
+		 * All evictable types are over-isolated. Like the legacy
+		 * path, wait once for concurrent reclaimers to put their
+		 * isolated folios back; give up if that makes no progress.
+		 */
+		if (stalled)
+			return 0;
+
+		stalled = true;
+		reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);
+
+		/* We are about to die and free our memory. Return now. */
+		if (fatal_signal_pending(current)) {
+			*fatal = true;
+			return 0;
+		}
+	}
+}
+
 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)
+			  unsigned int allowed, struct list_head *list,
+			  int *isolated, int *isolate_type, int *isolate_scanned)
 {
 	int i;
 	int total_scanned = 0;
 	int type = get_type_to_scan(lruvec, swappiness);
 
+	/*
+	 * The preferred type may have been excluded by
+	 * throttle_evictable_types(); start from the other one.
+	 */
+	if (!(allowed & BIT(type)))
+		type = !type;
+
 	for_each_evictable_type(i, swappiness) {
 		int scanned;
 		int tier = get_tier_idx(lruvec, type);

@@ -4858,9 +4916,10 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 		/*
 		 * 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.
+		 * too readily can disrupt the positive_ctrl_err() bias. Only
+		 * fall back to a type that is not throttled.
 		 */
-		if (!scanned)
+		if (!scanned && (allowed & BIT(!type)))
 			type = !type;
 	}
 

@@ -4883,13 +4942,29 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 	bool skip_retry = false;
 	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
+	unsigned int allowed;
+	bool fatal;
+
+	allowed = throttle_evictable_types(pgdat, swappiness, sc, &fatal);
+	if (!allowed) {
+		/*
+		 * We are about to die and free our memory. Like the legacy
+		 * path, pretend some pages were reclaimed so reclaim
+		 * unwinds quickly instead of looping back into the
+		 * throttle.
+		 */
+		if (fatal)
+			sc->nr_reclaimed += SWAP_CLUSTER_MAX;
+
+		return 0;
+	}
 
 	lruvec_lock_irq(lruvec);
 
 	/* In case folio deletion left empty old gens, flush them */
 	try_to_inc_min_seq(lruvec, swappiness);
 
-	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness,
+	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness, allowed,
 				 &list, &isolated, &type, &type_scanned);
 	nr_isolated = isolated;
 	if (nr_isolated)

2.53.0

Hello, thanks for the update!

Hmm, could too_many_isolated gets over aggressive or over passive, since the inactive number of MGLRU is just a compatiblity shim and does not have the same meaning of classical LRU? Especially when you run out of swap space or hit a memcg's swap limit, the anon inactive number becomes a jumpy random number. Proactive aging also makes the inactive number become huge for MGLRU. I still think even a "/ MIN_NR_GENS" is better than using inactive value, MGLRU used to use that as the aging trigger like this:

if (young * MIN_NR_GENS > total)

    return true;
if (old * (MIN_NR_GENS + 2) < total)
    return true;

The throttling avoids premature OOM and over scan when there are too many reclaimers, it is not for IO balancing. Because writeback folios are not isolated, they are putback to LRU waiting to be rotated, not isolated. So reclaimers never wait on the device however slow the device is (except cgroup V1 have some special quirks I think we should ignore), it's mostly CPU bound. I think the mechanism might be a bit outdated, even for classical LRU.

The result just shows that swap became less aggressive when under pressure, which is somewhat counter-intuitive. Overly aggressive throttling will unnecessarily slows performance if anon pages are actually cold or should be evicted. These results reflect a reclaim behavior change, not a performance improvement. And I think we shouldn't rely on throttling for balancing, those are two different things. Heat or I/O cost-based methods are better.

For example the test you posted, it just stress-ng which only spawns anon memory IIUC, so when you reclaim file, your code segments got reclaimed and hence the process stall and has to wait for code segments IO, which I think it will only slowdown the actual performance?

Agreed. The next version drops the inactive-based threshold and instead compares the isolated count against the total evictable pages of the type divided by MIN_NR_GENS, and makes the check per lruvec, where the MGLRU isolation happens.

Right. This script is only meant to show that the isolation accounting and the throttling kick in; it was not intended as a performance evaluation. As you pointed out, it also sampled the old node-level inactive-based threshold, which no longer matches the new check. Will provide a new test aimed at the premature OOM / overscanning scenario.

Yes, will fix that in the new test as well.

Best,
Hui