Skip to contentSkip to conversation

mm/vmscan: fix NR_ISOLATED accounting and throttling for MGLRU

PATCH Superseded Hui Zhu 2 participants 5 messages Aug 17 – Aug 18

Subsystems (3)
Memory management MGLRU MM reclaim

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Hui Zhu

author
#0 lore raw

v2 cover 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 extracts the throttling loop from shrink_inactive_list() into throttle_isolated() and calls it from evict_folios() as well, keeping the two reclaim paths unified, in line with what was done previously for writeback reclaim. This way the MGLRU eviction path is throttled 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 to force the throttle path.
Throttle events are counted via the mm_vmscan_throttled tracepoint. The test scripts and test log is in [1].

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

before after

throttle events (ISOLATED)       0       512472
  - from kswapd                  0            0
nr_isolated_anon peak            0         3670

pswpout 3383673 87134

Without the series MGLRU reclaim swaps heavily while nr_isolated_* stays at 0 and nothing is throttled. With the series the counters are updated and direct reclaimers are throttled (the pswpout drop is the reclaimers backing off); kswapd stays exempt.

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

before after
nr_isolated peak 59 1093

The "before" 59 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 ~23k pages on this box and needs more pile-up than the box can generate; test 1 exercises the same throttling mechanism end to end on the reclaim side.)

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

Changelog:
v2:
Accoding 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 | 74 +++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 11 deletions(-)
Signature · 1 line
2.53.0
author
#1 lore raw

PATCH v2 1/2 mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path

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.

Signed-off-by Hui Zhu <[email protected]>
---
 mm/vmscan.c | 7 +++++++
 1 file changed, 7 insertions(+)
mm/vmscan.c +7 -0
index c1404a59523d..fdc45d7d8fba 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,10 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 		goto retry;
 	}
 
+	if (nr_isolated)
+		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);
Signature · 1 line
2.53.0
reviewer Reviewed-by on v2 1/2
#2 lore raw

Re: [PATCH mm-unstable v2 1/2] mm/vmscan: fix missing NR_ISOLATED counter update in MGLRU reclaim path

Quoted 8 lines · Hui Zhu

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.

Quoted 6 lines

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.

Signed-off-by Hui Zhu <[email protected]>

---

Changes look good to me with one nit below.

Reviewed-by Baolin Wang <[email protected]>
Quoted 18 lines
  mm/vmscan.c | 7 +++++++
  1 file changed, 7 insertions(+)
mm/vmscan.c +3 -0
index c1404a59523d..fdc45d7d8fba 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)

mm/vmscan.c static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, · line 4960

Quoted 4 lines
mm/vmscan.c +1 -0
@@ -4954,6 +4957,10 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
  		goto retry;
  	}
 
+	if (nr_isolated)

Nit: we can remove this check, since we never reach here if nr_isolated is 0.

Quoted 6 lines
mm/vmscan.c +3 -0
+		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);
author
#3 lore raw

PATCH v2 2/2 mm/vmscan: apply too_many_isolated() throttling to MGLRU eviction

In reply to Hui Zhu

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(), extract the throttling loop from shrink_inactive_list() into throttle_isolated() and reuse it in evict_folios(). The type to isolate is predicted with get_type_to_scan() since it is unknown until isolation.

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.

Signed-off-by Hui Zhu <[email protected]>
---
 mm/vmscan.c | 67 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 11 deletions(-)
mm/vmscan.c +56 -11
index fdc45d7d8fba..886a53f563ab 100644
@@ -1819,7 +1819,7 @@ bool folio_isolate_lru(struct folio *folio)
  * the LRU list will go small and be scanned faster than necessary, leading to
  * unnecessary swapping, thrashing and OOM.
  */
-static bool too_many_isolated(struct pglist_data *pgdat, int file,
+static bool too_many_isolated(struct pglist_data *pgdat, bool file,
 		struct scan_control *sc)
 {
 	unsigned long inactive, isolated;

@@ -1856,6 +1856,37 @@ static bool too_many_isolated(struct pglist_data *pgdat, int file,
 	return too_many;
 }
 
+/*
+ * Throttle reclaim if too many isolated folios are piling up. If this makes
+ * no progress, the caller is probably looping on unevictable folios, so give
+ * up. Returns false to tell the caller to stop reclaiming, and sets @fatal
+ * if the task received a fatal signal while waiting, so that the caller can
+ * bail out faster.
+ */
+static bool throttle_isolated(struct pglist_data *pgdat, bool file,
+			      struct scan_control *sc, bool *fatal)
+{
+	bool stalled = false;
+
+	*fatal = false;
+	while (unlikely(too_many_isolated(pgdat, file, sc))) {
+		if (stalled)
+			return false;
+
+		/* wait a bit for the reclaimer. */
+		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 false;
+		}
+	}
+
+	return true;
+}
+
 /*
  * move_folios_to_lru() moves folios from private @list to appropriate LRU list.
  *

@@ -1992,19 +2023,14 @@ static unsigned long shrink_inactive_list(unsigned long nr_to_scan,
 	bool file = is_file_lru(lru);
 	enum node_stat_item item;
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
-	bool stalled = false;
-
-	while (unlikely(too_many_isolated(pgdat, file, sc))) {
-		if (stalled)
-			return 0;
-
-		/* wait a bit for the reclaimer. */
-		stalled = true;
-		reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);
+	bool fatal;
 
+	if (!throttle_isolated(pgdat, file, sc, &fatal)) {
 		/* We are about to die and free our memory. Return now. */
-		if (fatal_signal_pending(current))
+		if (fatal)
 			return SWAP_CLUSTER_MAX;
+
+		return 0;
 	}
 
 	lru_add_drain();

@@ -4883,6 +4909,25 @@ 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);
+	bool fatal;
+
+	/*
+	 * The type to isolate is unknown until isolation, so predict it for
+	 * the throttling check. isolate_folios() may still fall back to the
+	 * other type, which is fine for this heuristic.
+	 */
+	type = get_type_to_scan(lruvec, swappiness);
+	if (!throttle_isolated(pgdat, type, sc, &fatal)) {
+		/*
+		 * 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);
Signature · 1 line
2.53.0
reviewer
#4 lore raw

Re: [PATCH mm-unstable v2 2/2] mm/vmscan: apply too_many_isolated() throttling to MGLRU eviction

Quoted 58 lines · Hui Zhu

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(), extract the throttling loop from shrink_inactive_list() into throttle_isolated() and reuse it in evict_folios(). The type to isolate is predicted with get_type_to_scan() since it is unknown until isolation.

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.

Signed-off-by Hui Zhu <[email protected]>
---
  mm/vmscan.c | 67 ++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 56 insertions(+), 11 deletions(-)
mm/vmscan.c +20 -1
index fdc45d7d8fba..886a53f563ab 100644
@@ -1819,7 +1819,7 @@ bool folio_isolate_lru(struct folio *folio)
   * the LRU list will go small and be scanned faster than necessary, leading to
   * unnecessary swapping, thrashing and OOM.
   */
-static bool too_many_isolated(struct pglist_data *pgdat, int file,
+static bool too_many_isolated(struct pglist_data *pgdat, bool file,
  		struct scan_control *sc)
  {
  	unsigned long inactive, isolated;

@@ -1856,6 +1856,37 @@ static bool too_many_isolated(struct pglist_data *pgdat, int file,
  	return too_many;
  }
 
+/*
+ * Throttle reclaim if too many isolated folios are piling up. If this makes
+ * no progress, the caller is probably looping on unevictable folios, so give
+ * up. Returns false to tell the caller to stop reclaiming, and sets @fatal
+ * if the task received a fatal signal while waiting, so that the caller can
+ * bail out faster.
+ */
+static bool throttle_isolated(struct pglist_data *pgdat, bool file,
+			      struct scan_control *sc, bool *fatal)
+{
+	bool stalled = false;
+
+	*fatal = false;
+	while (unlikely(too_many_isolated(pgdat, file, sc))) {
+		if (stalled)
+			return false;
+
+		/* wait a bit for the reclaimer. */
+		stalled = true;

mm/vmscan.c static bool too_many_isolated(struct pglist_data *pgdat, int file, · line 1887

Quoted 10 lines
mm/vmscan.c +10 -0
+		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 false;
+		}
+	}
+
+	return true;

Returning ‘true’ or 'false' looks confusing to me, maybe use a readable variable 'is_throttled'?

Quoted 32 lines
mm/vmscan.c +7 -10
+}
+
  /*
   * move_folios_to_lru() moves folios from private @list to appropriate LRU list.
   *

@@ -1992,19 +2023,14 @@ static unsigned long shrink_inactive_list(unsigned long nr_to_scan,
  	bool file = is_file_lru(lru);
  	enum node_stat_item item;
  	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
-	bool stalled = false;
-
-	while (unlikely(too_many_isolated(pgdat, file, sc))) {
-		if (stalled)
-			return 0;
-
-		/* wait a bit for the reclaimer. */
-		stalled = true;
-		reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);
+	bool fatal;
 
+	if (!throttle_isolated(pgdat, file, sc, &fatal)) {
  		/* We are about to die and free our memory. Return now. */
-		if (fatal_signal_pending(current))
+		if (fatal)
  			return SWAP_CLUSTER_MAX;
+
+		return 0;
  	}
 
  	lru_add_drain();

@@ -4883,6 +4909,25 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
  	bool skip_retry = false;

mm/vmscan.c static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, · line 4919

Quoted 10 lines
mm/vmscan.c +8 -0
  	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
  	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
+	bool fatal;
+
+	/*
+	 * The type to isolate is unknown until isolation, so predict it for
+	 * the throttling check. isolate_folios() may still fall back to the
+	 * other type, which is fine for this heuristic.
+	 */
+	type = get_type_to_scan(lruvec, swappiness);

Perhaps we could use 'for_each_evictable_type(i, swappiness)' to check each type's isolation?

Quoted 14 lines
mm/vmscan.c +11 -0
+	if (!throttle_isolated(pgdat, type, sc, &fatal)) {
+		/*
+		 * 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);