mm: compaction: abort compaction if too many pages are isolated and caller is asynchronous V2

Asynchronous compaction is used when promoting to huge pages. This is all very nice but if there are a number of processes in compacting memory, a large number of pages can be isolated. An "asynchronous" process can stall for long periods of time as a result with a user reporting that firefox can stall for 10s of seconds. This patch aborts asynchronous compaction if too many pages are isolated as it's better to fail a hugepage promotion than stall a process. [minchan.kim@gmail.com: return COMPACT_PARTIAL for abort] Reported-and-tested-by: Ury Stankevich <urykhy@gmail.com> Signed-off-by: Mel Gorman <mgorman@suse.de> Reviewed-by: Minchan Kim <minchan.kim@gmail.com> Reviewed-by: Michal Hocko <mhocko@suse.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

mm: compaction: abort compaction if too many pages are isolated and caller is asynchronous V2
Asynchronous compaction is used when promoting to huge pages. This is all very nice but if there are a number of processes in compacting memory, a large number of pages can be isolated. An "asynchronous" process can stall for long periods of time as a result with a user reporting that firefox can stall for 10s of seconds. This patch aborts asynchronous compaction if too many pages are isolated as it's better to fail a hugepage promotion than stall a process. [minchan.kim@gmail.com: return COMPACT_PARTIAL for abort] Reported-and-tested-by: Ury Stankevich <urykhy@gmail.com> Signed-off-by: Mel Gorman <mgorman@suse.de> Reviewed-by: Minchan Kim <minchan.kim@gmail.com> Reviewed-by: Michal Hocko <mhocko@suse.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Mel Gorman · Linus Torvalds
1 parent d179e84ba5
Showing 1 changed file with 24 additions and 5 deletions Side-by-side Diff
mm/compaction.c
@@ -251,11 +251,18 @@
 	return isolated > (inactive + active) / 2;
 }
  
+/* possible outcome of isolate_migratepages */
+typedef enum {
+	ISOLATE_ABORT,		/* Abort compaction now */
+	ISOLATE_NONE,		/* No pages isolated, continue scanning */
+	ISOLATE_SUCCESS,	/* Pages isolated, migrate */
+} isolate_migrate_t;
+
 /*
  * Isolate all pages that can be migrated from the block pointed to by
  * the migrate scanner within compact_control.
  */
-static unsigned long isolate_migratepages(struct zone *zone,
+static isolate_migrate_t isolate_migratepages(struct zone *zone,
 					struct compact_control *cc)
 {
 	unsigned long low_pfn, end_pfn;
@@ -272,7 +279,7 @@
 	/* Do not cross the free scanner or scan within a memory hole */
 	if (end_pfn > cc->free_pfn || !pfn_valid(low_pfn)) {
 		cc->migrate_pfn = end_pfn;
-		return 0;
+		return ISOLATE_NONE;
 	}
  
 	/*
  
@@ -281,10 +288,14 @@
 	 * delay for some time until fewer pages are isolated
 	 */
 	while (unlikely(too_many_isolated(zone))) {
+		/* async migration should just abort */
+		if (!cc->sync)
+			return ISOLATE_ABORT;
+
 		congestion_wait(BLK_RW_ASYNC, HZ/10);
  
 		if (fatal_signal_pending(current))
-			return 0;
+			return ISOLATE_ABORT;
 	}
  
 	/* Time to isolate some pages for migration */
@@ -369,7 +380,7 @@
  
 	trace_mm_compaction_isolate_migratepages(nr_scanned, nr_isolated);
  
-	return cc->nr_migratepages;
+	return ISOLATE_SUCCESS;
 }
  
 /*
  
@@ -535,8 +546,15 @@
 		unsigned long nr_migrate, nr_remaining;
 		int err;
  
-		if (!isolate_migratepages(zone, cc))
+		switch (isolate_migratepages(zone, cc)) {
+		case ISOLATE_ABORT:
+			ret = COMPACT_PARTIAL;
+			goto out;
+		case ISOLATE_NONE:
 			continue;
+		case ISOLATE_SUCCESS:
+			;
+		}
  
 		nr_migrate = cc->nr_migratepages;
 		err = migrate_pages(&cc->migratepages, compaction_alloc,
@@ -560,6 +578,7 @@
  
 	}
  
+out:
 	/* Release free pages and check accounting */
 	cc->nr_freepages -= release_freepages(&cc->freepages);
 	VM_BUG_ON(cc->nr_freepages != 0);
...	...	@@ -251,11 +251,18 @@
251	251	return isolated > (inactive + active) / 2;
252	252	}
253	253
	254	+/* possible outcome of isolate_migratepages */
	255	+typedef enum {
	256	+ ISOLATE_ABORT, /* Abort compaction now */
	257	+ ISOLATE_NONE, /* No pages isolated, continue scanning */
	258	+ ISOLATE_SUCCESS, /* Pages isolated, migrate */
	259	+} isolate_migrate_t;
	260	+
254	261	/*
255	262	* Isolate all pages that can be migrated from the block pointed to by
256	263	* the migrate scanner within compact_control.
257	264	*/
258		-static unsigned long isolate_migratepages(struct zone *zone,
	265	+static isolate_migrate_t isolate_migratepages(struct zone *zone,
259	266	struct compact_control *cc)
260	267	{
261	268	unsigned long low_pfn, end_pfn;
...	...	@@ -272,7 +279,7 @@
272	279	/* Do not cross the free scanner or scan within a memory hole */
273	280	if (end_pfn > cc->free_pfn \|\| !pfn_valid(low_pfn)) {
274	281	cc->migrate_pfn = end_pfn;
275		- return 0;
	282	+ return ISOLATE_NONE;
276	283	}
277	284
278	285	/*
279	286
...	...	@@ -281,10 +288,14 @@
281	288	* delay for some time until fewer pages are isolated
282	289	*/
283	290	while (unlikely(too_many_isolated(zone))) {
	291	+ /* async migration should just abort */
	292	+ if (!cc->sync)
	293	+ return ISOLATE_ABORT;
	294	+
284	295	congestion_wait(BLK_RW_ASYNC, HZ/10);
285	296
286	297	if (fatal_signal_pending(current))
287		- return 0;
	298	+ return ISOLATE_ABORT;
288	299	}
289	300
290	301	/* Time to isolate some pages for migration */
...	...	@@ -369,7 +380,7 @@
369	380
370	381	trace_mm_compaction_isolate_migratepages(nr_scanned, nr_isolated);
371	382
372		- return cc->nr_migratepages;
	383	+ return ISOLATE_SUCCESS;
373	384	}
374	385
375	386	/*
376	387
...	...	@@ -535,8 +546,15 @@
535	546	unsigned long nr_migrate, nr_remaining;
536	547	int err;
537	548
538		- if (!isolate_migratepages(zone, cc))
	549	+ switch (isolate_migratepages(zone, cc)) {
	550	+ case ISOLATE_ABORT:
	551	+ ret = COMPACT_PARTIAL;
	552	+ goto out;
	553	+ case ISOLATE_NONE:
539	554	continue;
	555	+ case ISOLATE_SUCCESS:
	556	+ ;
	557	+ }
540	558
541	559	nr_migrate = cc->nr_migratepages;
542	560	err = migrate_pages(&cc->migratepages, compaction_alloc,
...	...	@@ -560,6 +578,7 @@
560	578
561	579	}
562	580
	581	+out:
563	582	/* Release free pages and check accounting */
564	583	cc->nr_freepages -= release_freepages(&cc->freepages);
565	584	VM_BUG_ON(cc->nr_freepages != 0);