mm, migration: add destination page freeing callback

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Signed-off-by David Rientjes <[email protected]>

---
 mm/compaction.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

mm/compaction.c +0 -0

@@ -797,6 +797,19 @@ static struct page *compaction_alloc(struct page *migratepage,
 }
 
 /*

Quoted 6 lines

mm/compaction.c +0 -0

@@ -797,6 +797,19 @@ static struct page *compaction_alloc(struct page *migratepage,
 }
 

mm/compaction.c static struct page *compaction_alloc(struct page *migratepage, · line 809

Quoted 11 lines

mm/compaction.c +10 -0

 /*
+ * This is a migrate-callback that "frees" freepages back to the isolated
+ * freelist.  All pages on the freelist are from the same zone, so there is no
+ * special handling needed for NUMA.
+ */
+static void compaction_free(struct page *page, unsigned long data)
+{
+	struct compact_control *cc = (struct compact_control *)data;
+
+	list_add(&page->lru, &cc->freepages);
+	cc->nr_freepages++;

With this change, migration_page() handles cc->nr_freepages consistently, so we don't have to run over freelist to update this count in update_nr_listpages()?

Memory migration uses a callback defined by the caller to determine how to allocate destination pages. When migration fails for a source page, however, it frees the destination page back to the system.

This patch adds a memory migration callback defined by the caller to determine how to free destination pages. If a caller, such as memory compaction, builds its own freelist for migration targets, this can reuse already freed memory instead of scanning additional memory.

If the caller provides a function to handle freeing of destination pages, it is called when page migration fails. Otherwise, it may pass NULL and freeing back to the system will be handled as usual. This patch introduces no functional change.

Signed-off-by David Rientjes <[email protected]>

mm/migrate.c +2 -2

@@ -1056,20 +1059,30 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
 	if (!page_mapped(hpage))
 		rc = move_to_new_page(new_hpage, hpage, 1, mode);
 
-	if (rc)
+	if (rc != MIGRATEPAGE_SUCCESS)
 		remove_migration_ptes(hpage, hpage);
 
 	if (anon_vma)
 		put_anon_vma(anon_vma);
 
-	if (!rc)
+	if (rc == MIGRATEPAGE_SUCCESS)
 		hugetlb_cgroup_migrate(hpage, new_hpage);
 
 	unlock_page(hpage);

Looks good to me.

Reviewed-by Naoya Horiguchi <[email protected]>

I have one comment below ...

[snip]

Quoted 15 lines

mm/migrate.c +2 -2

@@ -1056,20 +1059,30 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
 	if (!page_mapped(hpage))
 		rc = move_to_new_page(new_hpage, hpage, 1, mode);
 
-	if (rc)
+	if (rc != MIGRATEPAGE_SUCCESS)
 		remove_migration_ptes(hpage, hpage);
 
 	if (anon_vma)
 		put_anon_vma(anon_vma);
 
-	if (!rc)
+	if (rc == MIGRATEPAGE_SUCCESS)
 		hugetlb_cgroup_migrate(hpage, new_hpage);
 

mm/migrate.c static int unmap_and_move_huge_page(new_page_t get_new_page, · line 1080

Quoted 11 lines

mm/migrate.c +6 -1

 	unlock_page(hpage);
 out:
 	if (rc != -EAGAIN)
 		putback_active_hugepage(hpage);
-	put_page(new_hpage);
+
+	/*
+	 * If migration was not successful and there's a freeing callback, use
+	 * it.  Otherwise, put_page() will drop the reference grabbed during
+	 * isolation.
+	 */

This comment is true both for normal page and huge page, and people more likely to see unmap_and_move() at first, so this had better be (also) in unmap_and_move().

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Signed-off-by David Rientjes <[email protected]>

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Signed-off-by David Rientjes <[email protected]>

---
  mm/compaction.c | 27 ++++++++++++++++++---------
  1 file changed, 18 insertions(+), 9 deletions(-)

mm/compaction.c +6 -3

@@ -797,23 +797,32 @@ static struct page *compaction_alloc(struct page *migratepage,
  }
 
  /*
- * We cannot control nr_migratepages and nr_freepages fully when migration is
- * running as migrate_pages() has no knowledge of compact_control. When
- * migration is complete, we count the number of pages on the lists by hand.
+ * This is a migrate-callback that "frees" freepages back to the isolated
+ * freelist.  All pages on the freelist are from the same zone, so there is no
+ * special handling needed for NUMA.
+ */
+static void compaction_free(struct page *page, unsigned long data)
+{

mm/compaction.c +0 -4

   */
  static void update_nr_listpages(struct compact_control *cc)
  {
  	int nr_migratepages = 0;
-	int nr_freepages = 0;
  	struct page *page;
 
  	list_for_each_entry(page, &cc->migratepages, lru)
  		nr_migratepages++;
-	list_for_each_entry(page, &cc->freepages, lru)
-		nr_freepages++;
 
  	cc->nr_migratepages = nr_migratepages;
-	cc->nr_freepages = nr_freepages;
  }
 
  /* possible outcome of isolate_migratepages */

During compaction, update_nr_listpages() has been used to count remaining non-migrated and free pages after a call to migrage_pages(). The freepages counting has become unneccessary, and it turns out that migratepages counting is also unnecessary in most cases.

The only situation when it's needed to count cc->migratepages is when migrate_pages() returns with a negative error code. Otherwise, the non-negative return value is the number of pages that were not migrated, which is exactly the count of remaining pages in the cc->migratepages list.

Furthermore, any non-zero count is only interesting for the tracepoint of mm_compaction_migratepages events, because after that all remaining unmigrated pages are put back and their count is set to 0.

This patch therefore removes update_nr_listpages() completely, and changes the tracepoint definition so that the manual counting is done only when the tracepoint is enabled, and only when migrate_pages() returns a negative error code.

Furthermore, migrate_pages() and the tracepoints won't be called when there's nothing to migrate. This potentially avoids some wasted cycles and reduces the volume of uninteresting mm_compaction_migratepages events where "nr_migrated=0 nr_failed=0". In the stress-highalloc mmtest, this was about 75% of the events. The mm_compaction_isolate_migratepages event is better for determining that nothing was isolated for migration, and this one was just duplicating the info.

Signed-off-by Vlastimil Babka <[email protected]>

Cc Minchan Kim <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Naoya Horiguchi <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Rik van Riel <[email protected]>

---
 include/trace/events/compaction.h | 26 ++++++++++++++++++++++----
 mm/compaction.c                   | 30 ++++++++----------------------
 2 files changed, 30 insertions(+), 26 deletions(-)

include/trace/events/compaction.h +2 -0

index 06f544e..bbd5e1f 100644
@@ -5,7 +5,9 @@
 #define _TRACE_COMPACTION_H
 
 #include <linux/types.h>
+#include <linux/list.h>
 #include <linux/tracepoint.h>
+#include <linux/mm_types.h>
 #include <trace/events/gfpflags.h>
 
 DECLARE_EVENT_CLASS(mm_compaction_isolate_template,

include/trace/events/compaction.h +4 -2

-	TP_ARGS(nr_migrated, nr_failed),
+	TP_ARGS(nr_all, migrate_rc, migratepages),
 
 	TP_STRUCT__entry(
 		__field(unsigned long, nr_migrated)

@@ -58,7 +61,22 @@ TRACE_EVENT(mm_compaction_migratepages,
 	),
 
 	TP_fast_assign(
-		__entry->nr_migrated = nr_migrated;
+		unsigned long nr_failed = 0;
+		struct page *page;
+

include/trace/events/compaction.h +3 -0

+				nr_failed++;
+
+		__entry->nr_migrated = nr_all - nr_failed;
 		__entry->nr_failed = nr_failed;
 	),
 

mm/compaction.c +5 -22

index ae1d0ae..873d7de 100644
@@ -799,22 +799,6 @@ static void compaction_free(struct page *page, unsigned long data)
 	cc->nr_freepages++;
 }
 
-/*
- * We cannot control nr_migratepages fully when migration is running as
- * migrate_pages() has no knowledge of of compact_control.  When migration is
- * complete, we count the number of pages on the list by hand.
- */
-static void update_nr_listpages(struct compact_control *cc)
-{
-	int nr_migratepages = 0;
-	struct page *page;
-
-	list_for_each_entry(page, &cc->migratepages, lru)
-		nr_migratepages++;
-
-	cc->nr_migratepages = nr_migratepages;
-}
-
 /* possible outcome of isolate_migratepages */
 typedef enum {
 	ISOLATE_ABORT,		/* Abort compaction now */

@@ -1006,7 +990,6 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
 	migrate_prep_local();
 
 	while ((ret = compact_finished(zone, cc)) == COMPACT_CONTINUE) {
-		unsigned long nr_migrate, nr_remaining;
 		int err;
 
 		switch (isolate_migratepages(zone, cc)) {

@@ -1021,16 +1004,16 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
 			;
 		}
 
-		nr_migrate = cc->nr_migratepages;
+		if (!cc->nr_migratepages)
+			continue;
+
 		err = migrate_pages(&cc->migratepages, compaction_alloc,
 				compaction_free, (unsigned long)cc,
 				cc->sync ? MIGRATE_SYNC_LIGHT : MIGRATE_ASYNC,
 				MR_COMPACTION);
-		update_nr_listpages(cc);
-		nr_remaining = cc->nr_migratepages;
 
-		trace_mm_compaction_migratepages(nr_migrate - nr_remaining,
-						nr_remaining);
+		trace_mm_compaction_migratepages(cc->nr_migratepages, err,
+							&cc->migratepages);
 
 		/* Release isolated pages not migrated */
 		if (err) {

Quoted 19 lines · Vlastimil Babka

During compaction, update_nr_listpages() has been used to count remaining non-migrated and free pages after a call to migrage_pages(). The freepages counting has become unneccessary, and it turns out that migratepages counting is also unnecessary in most cases.

The only situation when it's needed to count cc->migratepages is when migrate_pages() returns with a negative error code. Otherwise, the non-negative return value is the number of pages that were not migrated, which is exactly the count of remaining pages in the cc->migratepages list.

Furthermore, any non-zero count is only interesting for the tracepoint of mm_compaction_migratepages events, because after that all remaining unmigrated pages are put back and their count is set to 0.

This patch therefore removes update_nr_listpages() completely, and changes the tracepoint definition so that the manual counting is done only when the tracepoint is enabled, and only when migrate_pages() returns a negative error code.

Quoted 6 lines

Furthermore, migrate_pages() and the tracepoints won't be called when there's nothing to migrate. This potentially avoids some wasted cycles and reduces the volume of uninteresting mm_compaction_migratepages events where "nr_migrated=0 nr_failed=0". In the stress-highalloc mmtest, this was about 75% of the events. The mm_compaction_isolate_migratepages event is better for determining that nothing was isolated for migration, and this one was just duplicating the info.

I agree with this patch.

Reviewed-by Naoya Horiguchi <[email protected]>

A few nitpicks below...

Quoted 27 lines

Signed-off-by Vlastimil Babka <[email protected]>

Cc Minchan Kim <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Naoya Horiguchi <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Rik van Riel <[email protected]>

---
  include/trace/events/compaction.h | 26 ++++++++++++++++++++++----
  mm/compaction.c                   | 30 ++++++++----------------------
  2 files changed, 30 insertions(+), 26 deletions(-)

include/trace/events/compaction.h +2 -0

index 06f544e..bbd5e1f 100644
@@ -5,7 +5,9 @@
  #define _TRACE_COMPACTION_H
 
  #include <linux/types.h>
+#include <linux/list.h>
  #include <linux/tracepoint.h>
+#include <linux/mm_types.h>
  #include <trace/events/gfpflags.h>
 
  DECLARE_EVENT_CLASS(mm_compaction_isolate_template,

include/trace/events/compaction.h DEFINE_EVENT(mm_compaction_isolate_template, mm_compaction_isolate_freepages, · line 54

Quoted 8 lines

include/trace/events/compaction.h +3 -2

@@ -47,10 +49,11 @@ DEFINE_EVENT(mm_compaction_isolate_template, mm_compaction_isolate_freepages,
 
  TRACE_EVENT(mm_compaction_migratepages,
 
-	TP_PROTO(unsigned long nr_migrated,
-		unsigned long nr_failed),
+	TP_PROTO(unsigned long nr_all,
+		int migrate_rc,
+		struct list_head * migratepages),

checkpatch.pl shows code violation message for this line. (ERROR: "foo * bar" should be "foo *bar")

Quoted 18 lines

include/trace/events/compaction.h +8 -2

-	TP_ARGS(nr_migrated, nr_failed),
+	TP_ARGS(nr_all, migrate_rc, migratepages),
 
  	TP_STRUCT__entry(
  		__field(unsigned long, nr_migrated)

@@ -58,7 +61,22 @@ TRACE_EVENT(mm_compaction_migratepages,
  	),
 
  	TP_fast_assign(
-		__entry->nr_migrated = nr_migrated;
+		unsigned long nr_failed = 0;
+		struct page *page;
+
+		/*
+		 * migrate_pages() returns either a non-negative number
+		 * with the number of pages that failed migration, or an
+		 * error code, in which case we need to count the remaining

include/trace/events/compaction.h TRACE_EVENT(mm_compaction_migratepages, · line 76

Quoted 6 lines

include/trace/events/compaction.h +6 -0

+		 * pages manually
+		 */
+		if (migrate_rc >= 0)
+			nr_failed = migrate_rc;
+		else
+		        list_for_each_entry(page, migratepages, lru)

This line contains whitespace indent.

Quoted 61 lines

include/trace/events/compaction.h +3 -0

+				nr_failed++;
+
+		__entry->nr_migrated = nr_all - nr_failed;
  		__entry->nr_failed = nr_failed;
  	),
 

mm/compaction.c +5 -22

index ae1d0ae..873d7de 100644
@@ -799,22 +799,6 @@ static void compaction_free(struct page *page, unsigned long data)
  	cc->nr_freepages++;
  }
 
-/*
- * We cannot control nr_migratepages fully when migration is running as
- * migrate_pages() has no knowledge of of compact_control.  When migration is
- * complete, we count the number of pages on the list by hand.
- */
-static void update_nr_listpages(struct compact_control *cc)
-{
-	int nr_migratepages = 0;
-	struct page *page;
-
-	list_for_each_entry(page, &cc->migratepages, lru)
-		nr_migratepages++;
-
-	cc->nr_migratepages = nr_migratepages;
-}
-
  /* possible outcome of isolate_migratepages */
  typedef enum {
  	ISOLATE_ABORT,		/* Abort compaction now */

@@ -1006,7 +990,6 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  	migrate_prep_local();
 
  	while ((ret = compact_finished(zone, cc)) == COMPACT_CONTINUE) {
-		unsigned long nr_migrate, nr_remaining;
  		int err;
 
  		switch (isolate_migratepages(zone, cc)) {

@@ -1021,16 +1004,16 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  			;
  		}
 
-		nr_migrate = cc->nr_migratepages;
+		if (!cc->nr_migratepages)
+			continue;
+
  		err = migrate_pages(&cc->migratepages, compaction_alloc,
  				compaction_free, (unsigned long)cc,
  				cc->sync ? MIGRATE_SYNC_LIGHT : MIGRATE_ASYNC,
  				MR_COMPACTION);
-		update_nr_listpages(cc);
-		nr_remaining = cc->nr_migratepages;
 
-		trace_mm_compaction_migratepages(nr_migrate - nr_remaining,
-						nr_remaining);
+		trace_mm_compaction_migratepages(cc->nr_migratepages, err,
+							&cc->migratepages);
 

mm/compaction.c static int compact_zone(struct zone *zone, struct compact_control *cc) · line 1032

Quoted 9 lines

mm/compaction.c +3 -0

  		/* Release isolated pages not migrated */
  		if (err) {

@@ -1044,6 +1027,9 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  				ret = COMPACT_PARTIAL;
  				goto out;
  			}
+		} else {
+			/* All pages were successfully migrated */
+			cc->nr_migratepages = 0;

cc->nr_migratepages = 0 is also done in err != 0, so can it be done in common path?

Thanks,
Naoya Horiguchi

The compaction free scanner in isolate_freepages() currently remembers PFN of the highest pageblock where it successfully isolates, to be used as the starting pageblock for the next invocation. The rationale behind this is that page migration might return free pages to the allocator when migration fails and we don't want to skip them if the compaction continues.

Since migration now returns free pages back to compaction code where they can be reused, this is no longer a concern. This patch changes isolate_freepages() so that the PFN for restarting is updated with each pageblock where isolation is attempted. Using stress-highalloc from mmtests, this resulted in 10% reduction of the pages scanned by the free scanner.

Note that the somewhat similar functionality that records highest successful pageblock in zone->compact_cached_free_pfn, remains unchanged. This cache is used when the whole compaction is restarted, not for multiple invocations of the free scanner during single compaction.

Signed-off-by Vlastimil Babka <[email protected]>

Cc Minchan Kim <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Naoya Horiguchi <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Rik van Riel <[email protected]>

---
 mm/compaction.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

mm/compaction.c +1 -9

index 873d7de..1967850 100644
@@ -686,12 +686,6 @@ static void isolate_freepages(struct zone *zone,
 	low_pfn = ALIGN(cc->migrate_pfn + 1, pageblock_nr_pages);
 
 	/*
-	 * If no pages are isolated, the block_start_pfn < low_pfn check
-	 * will kick in.
-	 */
-	next_free_pfn = 0;
-
-	/*
 	 * Isolate free pages until enough are available to migrate the
 	 * pages on cc->migratepages. We stop searching if the migrate
 	 * and free page scanners meet or enough free pages are isolated.

@@ -731,19 +725,19 @@ static void isolate_freepages(struct zone *zone,
 			continue;
 
 		/* Found a block suitable for isolating free pages from */
+		next_free_pfn = block_start_pfn;
 		isolated = isolate_freepages_block(cc, block_start_pfn,
 					block_end_pfn, freelist, false);
 		nr_freepages += isolated;
 
 		/*
-		 * Record the highest PFN we isolated pages from. When next
-		 * looking for free pages, the search will restart here as
-		 * page migration may have returned some pages to the allocator

Quoted 54 lines · Vlastimil Babka

The compaction free scanner in isolate_freepages() currently remembers PFN of the highest pageblock where it successfully isolates, to be used as the starting pageblock for the next invocation. The rationale behind this is that page migration might return free pages to the allocator when migration fails and we don't want to skip them if the compaction continues.

Since migration now returns free pages back to compaction code where they can be reused, this is no longer a concern. This patch changes isolate_freepages() so that the PFN for restarting is updated with each pageblock where isolation is attempted. Using stress-highalloc from mmtests, this resulted in 10% reduction of the pages scanned by the free scanner.

Note that the somewhat similar functionality that records highest successful pageblock in zone->compact_cached_free_pfn, remains unchanged. This cache is used when the whole compaction is restarted, not for multiple invocations of the free scanner during single compaction.

Signed-off-by Vlastimil Babka <[email protected]>

Cc Minchan Kim <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Naoya Horiguchi <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Rik van Riel <[email protected]>

---
  mm/compaction.c | 18 ++++++------------
  1 file changed, 6 insertions(+), 12 deletions(-)

mm/compaction.c +1 -6

index 873d7de..1967850 100644
@@ -686,12 +686,6 @@ static void isolate_freepages(struct zone *zone,
  	low_pfn = ALIGN(cc->migrate_pfn + 1, pageblock_nr_pages);
 
  	/*
-	 * If no pages are isolated, the block_start_pfn < low_pfn check
-	 * will kick in.
-	 */
-	next_free_pfn = 0;
-
-	/*
  	 * Isolate free pages until enough are available to migrate the
  	 * pages on cc->migratepages. We stop searching if the migrate
  	 * and free page scanners meet or enough free pages are isolated.

@@ -731,19 +725,19 @@ static void isolate_freepages(struct zone *zone,
  			continue;
 
  		/* Found a block suitable for isolating free pages from */
+		next_free_pfn = block_start_pfn;
  		isolated = isolate_freepages_block(cc, block_start_pfn,
  					block_end_pfn, freelist, false);
  		nr_freepages += isolated;
 

Quoted 4 lines

mm/compaction.c +0 -3

  		/*
-		 * Record the highest PFN we isolated pages from. When next
-		 * looking for free pages, the search will restart here as
-		 * page migration may have returned some pages to the allocator

mm/compaction.c static void isolate_freepages(struct zone *zone, · line 746

Quoted 10 lines

mm/compaction.c +5 -3

+		 * Set a flag that we successfully isolated in this pageblock.
+		 * In the next loop iteration, zone->compact_cached_free_pfn
+		 * will not be updated and thus it will effectively contain the
+		 * highest pageblock we isolated pages from.
  		 */
-		if (isolated && next_free_pfn == 0) {
+		if (isolated)
  			cc->finished_update_free = true;
-			next_free_pfn = block_start_pfn;
-		}

Why don't you completely remove next_free_pfn and update cc->free_pfn directly?

Thanks,
Naoya Horiguchi

Each zone has a cached migration scanner pfn for memory compaction so that subsequent calls to memory compaction can start where the previous call left off.

Currently, the compaction migration scanner only updates the per-zone cached pfn when pageblocks were not skipped for async compaction. This creates a dependency on calling sync compaction to avoid having subsequent calls to async compaction from scanning an enormous amount of non-MOVABLE pageblocks each time it is called. On large machines, this could be potentially very expensive.

This patch adds a per-zone cached migration scanner pfn only for async compaction. It is updated everytime a pageblock has been scanned in its entirety and when no pages from it were successfully isolated. The cached migration scanner pfn for sync compaction is updated only when called for sync compaction.

Signed-off-by David Rientjes <[email protected]>

---
  include/linux/mmzone.h |  5 +++--
  mm/compaction.c        | 55 ++++++++++++++++++++++++++------------------------
  2 files changed, 32 insertions(+), 28 deletions(-)

include/linux/mmzone.h +3 -2

@@ -360,9 +360,10 @@ struct zone {
  	/* Set to true when the PG_migrate_skip bits should be cleared */
  	bool			compact_blockskip_flush;
 
-	/* pfns where compaction scanners should start */
+	/* pfn where compaction free scanner should start */
  	unsigned long		compact_cached_free_pfn;
-	unsigned long		compact_cached_migrate_pfn;
+	/* pfn where async and sync compaction migration scanner should start */
+	unsigned long		compact_cached_migrate_pfn[2];
  #endif
  #ifdef CONFIG_MEMORY_HOTPLUG
  	/* see spanned/present_pages for more description */

mm/compaction.c +27 -25

@@ -89,7 +89,8 @@ static void __reset_isolation_suitable(struct zone *zone)
  	unsigned long end_pfn = zone_end_pfn(zone);
  	unsigned long pfn;
 
-	zone->compact_cached_migrate_pfn = start_pfn;
+	zone->compact_cached_migrate_pfn[0] = start_pfn;
+	zone->compact_cached_migrate_pfn[1] = start_pfn;
  	zone->compact_cached_free_pfn = end_pfn;
  	zone->compact_blockskip_flush = false;
 

@@ -134,6 +135,7 @@ static void update_pageblock_skip(struct compact_control *cc,
  			bool migrate_scanner)
  {
  	struct zone *zone = cc->zone;
+	unsigned long pfn;
 
  	if (cc->ignore_skip_hint)
  		return;

@@ -141,20 +143,25 @@ static void update_pageblock_skip(struct compact_control *cc,
  	if (!page)
  		return;
 
-	if (!nr_isolated) {
-		unsigned long pfn = page_to_pfn(page);
-		set_pageblock_skip(page);
-
-		/* Update where compaction should restart */
-		if (migrate_scanner) {
-			if (!cc->finished_update_migrate &&
-			    pfn > zone->compact_cached_migrate_pfn)
-				zone->compact_cached_migrate_pfn = pfn;
-		} else {
-			if (!cc->finished_update_free &&
-			    pfn < zone->compact_cached_free_pfn)
-				zone->compact_cached_free_pfn = pfn;
-		}
+	if (nr_isolated)
+		return;
+
+	set_pageblock_skip(page);
+	pfn = page_to_pfn(page);
+
+	/* Update where async and sync compaction should restart */
+	if (migrate_scanner) {
+		if (cc->finished_update_migrate)
+			return;
+		if (pfn > zone->compact_cached_migrate_pfn[0])
+			zone->compact_cached_migrate_pfn[0] = pfn;
+		if (cc->sync && pfn > zone->compact_cached_migrate_pfn[1])
+			zone->compact_cached_migrate_pfn[1] = pfn;
+	} else {
+		if (cc->finished_update_free)
+			return;
+		if (pfn < zone->compact_cached_free_pfn)
+			zone->compact_cached_free_pfn = pfn;
  	}
  }
  #else

@@ -464,7 +471,6 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  	unsigned long flags;
  	bool locked = false;
  	struct page *page = NULL, *valid_page = NULL;
-	bool skipped_async_unsuitable = false;
  	const isolate_mode_t mode = (!cc->sync ? ISOLATE_ASYNC_MIGRATE : 0) |
  				    (unevictable ? ISOLATE_UNEVICTABLE : 0);
 

@@ -540,11 +546,8 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  			 * the minimum amount of work satisfies the allocation
  			 */
  			mt = get_pageblock_migratetype(page);
-			if (!cc->sync && !migrate_async_suitable(mt)) {
-				cc->finished_update_migrate = true;
-				skipped_async_unsuitable = true;
+			if (!cc->sync && !migrate_async_suitable(mt))
  				goto next_pageblock;
-			}
  		}
 
  		/*

@@ -646,10 +649,8 @@ next_pageblock:
  	/*
  	 * Update the pageblock-skip information and cached scanner pfn,
  	 * if the whole pageblock was scanned without isolating any page.
-	 * This is not done when pageblock was skipped due to being unsuitable
-	 * for async compaction, so that eventual sync compaction can try.
  	 */
-	if (low_pfn == end_pfn && !skipped_async_unsuitable)
+	if (low_pfn == end_pfn)
  		update_pageblock_skip(cc, valid_page, nr_isolated, true);
 
  	trace_mm_compaction_isolate_migratepages(nr_scanned, nr_isolated);

@@ -875,7 +876,8 @@ static int compact_finished(struct zone *zone,
  	/* Compaction run completes if the migrate and free scanner meet */
  	if (cc->free_pfn <= cc->migrate_pfn) {
  		/* Let the next compaction start anew. */
-		zone->compact_cached_migrate_pfn = zone->zone_start_pfn;
+		zone->compact_cached_migrate_pfn[0] = zone->zone_start_pfn;
+		zone->compact_cached_migrate_pfn[1] = zone->zone_start_pfn;
  		zone->compact_cached_free_pfn = zone_end_pfn(zone);
 
  		/*

@@ -1000,7 +1002,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  	 * information on where the scanners should start but check that it
  	 * is initialised by ensuring the values are within zone boundaries.
  	 */
-	cc->migrate_pfn = zone->compact_cached_migrate_pfn;
+	cc->migrate_pfn = zone->compact_cached_migrate_pfn[cc->sync];
  	cc->free_pfn = zone->compact_cached_free_pfn;
  	if (cc->free_pfn < start_pfn || cc->free_pfn > end_pfn) {
  		cc->free_pfn = end_pfn & ~(pageblock_nr_pages-1);

OK that's due to my commit 50b5b094e6 ("mm: compaction: do not mark unmovable pageblocks as skipped in async compaction") and the intention was to avoid marking pageblocks as to-be-skipped just because they were ignored by async compaction, which would make the following sync compaction ignore them as well. However it's true that update_pageblock_skip() also updates the cached pfn's and not updating them is a sideeffect of this change.

I didn't think that would be a problem as skipping whole pageblocks due to being non-movable should be fast and without taking locks. But if your testing shows that this is a problem, then OK.

Quoted 6 lines

This patch adds a per-zone cached migration scanner pfn only for async compaction. It is updated everytime a pageblock has been scanned in its entirety and when no pages from it were successfully isolated. The cached migration scanner pfn for sync compaction is updated only when called for sync
compaction.

I think this might be an overkill and maybe just decoupling the cached pfn update from the update_pageblock_skip() would be enough, i.e. restore pre-50b5b094e6 behavior for the cached pfn (but not for the skip bits)? I wonder if your new sync migration scanner would make any difference. Presumably when async compaction finishes without success by having the scanners meet, compact_finished() will reset the cached pfn's and the sync compaction will not have a chance to use any previously cached value anyway?

Quoted 7 lines · Vlastimil Babka

OK that's due to my commit 50b5b094e6 ("mm: compaction: do not mark unmovable pageblocks as skipped in async compaction") and the intention was to avoid marking pageblocks as to-be-skipped just because they were ignored by async compaction, which would make the following sync compaction ignore them as well. However it's true that update_pageblock_skip() also updates the cached pfn's and not updating them is a sideeffect of this change.

It's not necessary just that commit, update_pageblock_skip() won't do anything if cc->finished_update_migrate is true which still happens before the commit. This issue was noticed on a kernel without your commit.

Quoted 4 lines

I didn't think that would be a problem as skipping whole pageblocks due to being non-movable should be fast and without taking locks. But if your testing shows that this is a problem, then OK.

Async compaction terminates early when lru_lock is contended or need_resched() and on zones that are so large for a 128GB machine, this happens often. A thp allocation returns immediately because of contended_compaction in the page allocator. When the next thp is allocated, async compaction starts from where the former iteration started because we don't do any caching of the pfns and nothing called sync compaction. It's simply unnecessary overhead that can be prevented on the next call and it leaves a potentially large part of the zone unscanned if we continuously fail because of contention. This patch fixes that.

Quoted 7 lines

Quoted 6 lines

This patch adds a per-zone cached migration scanner pfn only for async compaction. It is updated everytime a pageblock has been scanned in its entirety and when no pages from it were successfully isolated. The cached migration scanner pfn for sync compaction is updated only when called for sync
compaction.

Quoted 8 lines

I think this might be an overkill and maybe just decoupling the cached pfn update from the update_pageblock_skip() would be enough, i.e. restore pre-50b5b094e6 behavior for the cached pfn (but not for the skip bits)? I wonder if your new sync migration scanner would make any difference. Presumably when async compaction finishes without success by having the scanners meet, compact_finished() will reset the cached pfn's and the sync compaction will not have a chance to use any previously cached value anyway?

When a zone has 32GB or 64GB to scan, as is in this case (and will become larger in the future), async compaction will always terminate early. It may never cause a migration destination page to even be allocated, the freeing scanner may never move and there's no guarantee they will ever meet if we never call sync compaction.

The logic presented in this patch will avoid rescanning the non-movable pageblocks, for example, for async compaction until all other memory has been scanned.

I see, although I would still welcome some numbers to back such change.

What I still don't like is the removal of the intent of commit 50b5b094e6. You now again call set_pageblock_skip() unconditionally, thus also on pageblocks that async compaction skipped due to being non-MOVABLE. The sync compaction will thus ignore them.

Quoted 1 line · Vlastimil Babka

I see, although I would still welcome some numbers to back such change.

It's pretty difficult to capture numbers for this in real-world scenarios since it happens rarely (and when it happens, it's very significant latency) and without an instrumented kernel that will determine how many pageblocks have been skipped. I could create a synthetic example of it in the kernel and get numbers for a worst-case scenario with a 64GB zone if you'd like, I'm not sure how representative it will be.

Quoted 5 lines

What I still don't like is the removal of the intent of commit 50b5b094e6. You now again call set_pageblock_skip() unconditionally, thus also on pageblocks that async compaction skipped due to being non-MOVABLE. The sync compaction will thus ignore them.

I'm not following you, with this patch there are two cached pfns for the migration scanner: one is used for sync and one is used for async. When cc->sync == true, both cached pfns are updated (async is not going to succeed for a pageblock when sync failed for that pageblock); when cc->sync == false, the async cached pfn is updated only and we pick up again where we left off for subsequent async compactions. Sync compaction will still begin where it last left off and consider these non-MOVABLE pageblocks.

Synchronous memory compaction can be very expensive: it can iterate an enormous amount of memory without aborting, constantly rescheduling, waiting on page locks and lru_lock, etc, if a pageblock cannot be defragmented.

Unfortunately, it's too expensive for pagefault for transparent hugepages and it's much better to simply fallback to pages. On 128GB machines, we find that synchronous memory compaction can take O(seconds) for a single thp fault.

Now that async compaction remembers where it left off without strictly relying on sync compaction, this makes thp allocations best-effort without causing egregious latency during pagefault.

Signed-off-by David Rientjes <[email protected]>

Quoted 8 lines

Synchronous memory compaction can be very expensive: it can iterate an enormous amount of memory without aborting, constantly rescheduling, waiting on page locks and lru_lock, etc, if a pageblock cannot be defragmented.

Unfortunately, it's too expensive for pagefault for transparent hugepages and it's much better to simply fallback to pages. On 128GB machines, we find that synchronous memory compaction can take O(seconds) for a single thp fault.

Quoted 5 lines

Now that async compaction remembers where it left off without strictly relying on sync compaction, this makes thp allocations best-effort without causing egregious latency during pagefault.

Signed-off-by David Rientjes <[email protected]>

Compaction uses MIGRATE_SYNC_LIGHT which in the current implementation avoids calling ->writepage to clean dirty page in the fallback migrate case

static int fallback_migrate_page(struct address_space *mapping,
struct page *newpage, struct page *page, enum migrate_mode mode)

{
        if (PageDirty(page)) {
                /* Only writeback pages in full synchronous migration */
                if (mode != MIGRATE_SYNC)
                        return -EBUSY;
                return writeout(mapping, page);
        }

or waiting on page writeback in other cases

                /*
                 * Only in the case of a full synchronous migration is it
                 * necessary to wait for PageWriteback. In the async case,
                 * the retry loop is too short and in the sync-light case,
                 * the overhead of stalling is too much
                 */
                if (mode != MIGRATE_SYNC) {
                        rc = -EBUSY;
                        goto uncharge;
                }

or on acquiring the page lock for unforced migrations.

However, buffers still get locked in the SYNC_LIGHT case causing stalls in buffer_migrate_lock_buffers which may be undesirable or maybe you are hitting some other case.

It would be preferable to identify what is
getting stalled in SYNC_LIGHT compaction and fix that rather than disabling it entirely. You may also want to distinguish between a direct compaction by a process and collapsing huge pages as done by khugepaged.

Quoted 41 lines · Mel Gorman

Quoted 8 lines

Synchronous memory compaction can be very expensive: it can iterate an enormous amount of memory without aborting, constantly rescheduling, waiting on page locks and lru_lock, etc, if a pageblock cannot be defragmented.

Unfortunately, it's too expensive for pagefault for transparent hugepages and it's much better to simply fallback to pages. On 128GB machines, we find that synchronous memory compaction can take O(seconds) for a single thp fault.

Quoted 5 lines

Now that async compaction remembers where it left off without strictly relying on sync compaction, this makes thp allocations best-effort without causing egregious latency during pagefault.

Signed-off-by David Rientjes <[email protected]>

Compaction uses MIGRATE_SYNC_LIGHT which in the current implementation avoids calling ->writepage to clean dirty page in the fallback migrate case

static int fallback_migrate_page(struct address_space *mapping,
struct page *newpage, struct page *page, enum migrate_mode mode)

{
        if (PageDirty(page)) {
                /* Only writeback pages in full synchronous migration */
                if (mode != MIGRATE_SYNC)
                        return -EBUSY;
                return writeout(mapping, page);
        }

or waiting on page writeback in other cases

                /*
                 * Only in the case of a full synchronous migration is it
                 * necessary to wait for PageWriteback. In the async case,
                 * the retry loop is too short and in the sync-light case,
                 * the overhead of stalling is too much
                 */
                if (mode != MIGRATE_SYNC) {
                        rc = -EBUSY;
                        goto uncharge;
                }

Quoted 2 lines

or on acquiring the page lock for unforced migrations.

The page locks I'm referring to is the lock_page() in __unmap_and_move() that gets called for sync compaction after the migrate_pages() iteration makes a few passes and unsuccessfully grabs it. This becomes a forced migration since __unmap_and_move() returns -EAGAIN when the trylock fails.

Quoted 3 lines

However, buffers still get locked in the SYNC_LIGHT case causing stalls in buffer_migrate_lock_buffers which may be undesirable or maybe you are hitting some other case.

We have perf profiles from one workload in particular that shows contention on i_mmap_mutex (anon isn't interesting since the vast majority of memory on this workload [120GB on a 128GB machine] is has a gup pin and doesn't get isolated because of 119d6d59dcc0 ("mm, compaction: avoid isolating pinned pages")) between cpus all doing memory compaction trying to fault thp memory.

That's one example that we've seen, but the fact remains that at times sync compaction will iterate the entire 128GB machine and not allow an order-9 page to be allocated and there's nothing to preempt it like the need_resched() or lock contention checks that async compaction has.

(I
say "at times" because deferred compaction does help if it's triggered and the cached pfn values help, but the worst case scenario is when the entire machine is iterated.) Would it make sense when HPAGE_PMD_ORDER == pageblock_order, which is usually the case, that we don't even try to migrate pages unless the entire pageblock is successfully isolated and gfp_mask & __GFP_NO_KSWAPD?

That's the only other thing that I can think that will avoid such a problem other than this patch.

We're struggling to find the logic that ensures sync compaction does not become too expensive at pagefault and unless that can be guaranteed, it seemed best to avoid it entirely.

Quoted 5 lines

The page locks I'm referring to is the lock_page() in __unmap_and_move() that gets called for sync compaction after the migrate_pages() iteration makes a few passes and unsuccessfully grabs it. This becomes a forced migration since __unmap_and_move() returns -EAGAIN when the trylock fails.

Can that be fixed then instead of disabling it entirely?

Quoted 7 lines

We have perf profiles from one workload in particular that shows contention on i_mmap_mutex (anon isn't interesting since the vast majority of memory on this workload [120GB on a 128GB machine] is has a gup pin and doesn't get isolated because of 119d6d59dcc0 ("mm, compaction: avoid isolating pinned pages")) between cpus all doing memory compaction trying to fault thp memory.

Abort SYNC_LIGHT compaction if the mutex is contended.

Quoted 4 lines

That's one example that we've seen, but the fact remains that at times sync compaction will iterate the entire 128GB machine and not allow an order-9 page to be allocated and there's nothing to preempt it like the need_resched() or lock contention checks that async compaction has.

Make compact_control->sync the same enum field and check for contention on the async/sync_light case but leave it for sync if compacting via the proc interface?

Quoted 8 lines · Mel Gorman

Quoted 5 lines

The page locks I'm referring to is the lock_page() in __unmap_and_move() that gets called for sync compaction after the migrate_pages() iteration makes a few passes and unsuccessfully grabs it. This becomes a forced migration since __unmap_and_move() returns -EAGAIN when the trylock fails.

Can that be fixed then instead of disabling it entirely?

We could return -EAGAIN when the trylock_page() fails for MIGRATE_SYNC_LIGHT. It would become a forced migration but we ignore that currently for MIGRATE_ASYNC, and I could extend it to be ignored for MIGRATE_SYNC_LIGHT as well.

Quoted 10 lines

Quoted 7 lines

We have perf profiles from one workload in particular that shows contention on i_mmap_mutex (anon isn't interesting since the vast majority of memory on this workload [120GB on a 128GB machine] is has a gup pin and doesn't get isolated because of 119d6d59dcc0 ("mm, compaction: avoid isolating pinned pages")) between cpus all doing memory compaction trying to fault thp memory.

Abort SYNC_LIGHT compaction if the mutex is contended.

Yeah, I have patches for that as well but we're waiting to see if they are actually needed when sync compaction is disabled for thp. If we aren't actually going to disable it entirely, then I can revive those patches if the contention becomes such an issue.

Quoted 9 lines

Quoted 4 lines

That's one example that we've seen, but the fact remains that at times sync compaction will iterate the entire 128GB machine and not allow an order-9 page to be allocated and there's nothing to preempt it like the need_resched() or lock contention checks that async compaction has.

Make compact_control->sync the same enum field and check for contention on the async/sync_light case but leave it for sync if compacting via the proc interface?

Ok, that certainly can be done, I wasn't sure you would be happy with such a change. I'm not sure there's so much of a difference between the new compact_control->sync == MIGRATE_ASYNC and == MIGRATE_SYNC_LIGHT now, though. Would it make sense to remove MIGRATE_SYNC_LIGHT entirely from the page allocator, i.e. remove sync_migration entirely, and just retry with a second call to compaction before failing instead?

Quoted 8 lines · Mel Gorman

Quoted 5 lines

The page locks I'm referring to is the lock_page() in __unmap_and_move() that gets called for sync compaction after the migrate_pages() iteration makes a few passes and unsuccessfully grabs it. This becomes a forced migration since __unmap_and_move() returns -EAGAIN when the trylock fails.

Can that be fixed then instead of disabling it entirely?

We could return -EAGAIN when the trylock_page() fails for MIGRATE_SYNC_LIGHT. It would become a forced migration but we ignore that currently for MIGRATE_ASYNC, and I could extend it to be ignored for MIGRATE_SYNC_LIGHT as well.

Quoted 10 lines

Quoted 7 lines

We have perf profiles from one workload in particular that shows contention on i_mmap_mutex (anon isn't interesting since the vast majority of memory on this workload [120GB on a 128GB machine] is has a gup pin and doesn't get isolated because of 119d6d59dcc0 ("mm, compaction: avoid isolating pinned pages")) between cpus all doing memory compaction trying to fault thp memory.

Abort SYNC_LIGHT compaction if the mutex is contended.

Yeah, I have patches for that as well but we're waiting to see if they are actually needed when sync compaction is disabled for thp. If we aren't actually going to disable it entirely, then I can revive those patches if the contention becomes such an issue.

Quoted 5 lines

Quoted 4 lines

That's one example that we've seen, but the fact remains that at times sync compaction will iterate the entire 128GB machine and not allow an order-9 page to be allocated and there's nothing to preempt it like the need_resched() or lock contention checks that async compaction has.

Quoted 4 lines

Make compact_control->sync the same enum field and check for contention on the async/sync_light case but leave it for sync if compacting via the proc interface?

Ok, that certainly can be done, I wasn't sure you would be happy with such a change.

I'm not sure there's so much of a difference between the new compact_control->sync == MIGRATE_ASYNC and == MIGRATE_SYNC_LIGHT now, though. Would it make sense to remove MIGRATE_SYNC_LIGHT entirely from the page allocator, i.e. remove sync_migration entirely, and just retry with a second call to compaction before failing instead?

Quoted 37 lines · David Rientjes

Quoted 8 lines · Mel Gorman

Quoted 5 lines

The page locks I'm referring to is the lock_page() in __unmap_and_move() that gets called for sync compaction after the migrate_pages() iteration makes a few passes and unsuccessfully grabs it. This becomes a forced migration since __unmap_and_move() returns -EAGAIN when the trylock fails.

Can that be fixed then instead of disabling it entirely?

We could return -EAGAIN when the trylock_page() fails for MIGRATE_SYNC_LIGHT. It would become a forced migration but we ignore that currently for MIGRATE_ASYNC, and I could extend it to be ignored for MIGRATE_SYNC_LIGHT as well.

Quoted 10 lines

Quoted 7 lines

We have perf profiles from one workload in particular that shows contention on i_mmap_mutex (anon isn't interesting since the vast majority of memory on this workload [120GB on a 128GB machine] is has a gup pin and doesn't get isolated because of 119d6d59dcc0 ("mm, compaction: avoid isolating pinned pages")) between cpus all doing memory compaction trying to fault thp memory.

Abort SYNC_LIGHT compaction if the mutex is contended.

Yeah, I have patches for that as well but we're waiting to see if they are actually needed when sync compaction is disabled for thp. If we aren't actually going to disable it entirely, then I can revive those patches if the contention becomes such an issue.

Quoted 5 lines

Quoted 4 lines

That's one example that we've seen, but the fact remains that at times sync compaction will iterate the entire 128GB machine and not allow an order-9 page to be allocated and there's nothing to preempt it like the need_resched() or lock contention checks that async compaction has.

Quoted 7 lines

Quoted 4 lines

Make compact_control->sync the same enum field and check for contention on the async/sync_light case but leave it for sync if compacting via the proc interface?

Ok, that certainly can be done, I wasn't sure you would be happy with such a change.

I'm not super-keen as the success rates are already very poor for allocations under pressure. It's something Vlastimil is working on so it may be possible to get some of the success rates back. It has always been the case that compaction should not severely impact overall performance as THP gains must offset compaction. While I'm not happy to reduce the success rates further I do not think we should leave a known performance impact on 128G machines wait on Vlastimil's series.

Vlastimil, what do you think?

Quoted 5 lines

I'm not sure there's so much of a difference between the new compact_control->sync == MIGRATE_ASYNC and == MIGRATE_SYNC_LIGHT now, though. Would it make sense to remove MIGRATE_SYNC_LIGHT entirely from the page allocator, i.e. remove sync_migration entirely, and just retry with a second call to compaction before failing instead?

Would it be possible if only khugepaged entered SYNC_LIGHT migration and kswapd and direct THP allocations used only MIGRATE_ASYNC? That would allow khugepaged to continue locking pages and buffers in a slow path while still not allowing it to issue IO or wait on writeback. It would also give a chance for Vlastimil's series to shake out a bit without him having to reintroduce SYNC_LIGHT as part of that series.

Memory migration uses a callback defined by the caller to determine how to allocate destination pages. When migration fails for a source page, however, it frees the destination page back to the system.

This patch adds a memory migration callback defined by the caller to determine how to free destination pages. If a caller, such as memory compaction, builds its own freelist for migration targets, this can reuse already freed memory instead of scanning additional memory.

If the caller provides a function to handle freeing of destination pages, it is called when page migration fails. Otherwise, it may pass NULL and freeing back to the system will be handled as usual. This patch introduces no functional change.

Reviewed-by Naoya Horiguchi <[email protected]>

Signed-off-by David Rientjes <[email protected]>

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Acked-by Mel Gorman <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

Signed-off-by David Rientjes <[email protected]>

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Quoted 9 lines

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

Quoted 5 lines

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

What did Greg actually report? IOW, what if any observable problem is being fixed here?

Quoted 15 lines · Andrew Morton

Quoted 14 lines

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Quoted 3 lines

What did Greg actually report? IOW, what if any observable problem is being fixed here?

Greg reported by code inspection that he found isolated free pages were returned back to the VM rather than the compaction freelist. This will cause holes behind the free scanner and cause it to reallocate additional memory if necessary later.

Quoted 9 lines · On Tue, 6 May 2014 19:22:43 -0700 (PDT) David Rientjes <[email protected]>…

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

Quoted 5 lines

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

What did Greg actually report? IOW, what if any observable problem is being fixed here?

Quoted 17 lines · Andrew Morton

Quoted 14 lines · On Tue, 6 May 2014 19:22:43 -0700 (PDT) David Rientjes <[email protected]>…

Memory compaction works by having a "freeing scanner" scan from one end of a zone which isolates pages as migration targets while another "migrating scanner" scans from the other end of the same zone which isolates pages for migration.

When page migration fails for an isolated page, the target page is returned to the system rather than the freelist built by the freeing scanner. This may require the freeing scanner to continue scanning memory after suitable migration targets have already been returned to the system needlessly.

This patch returns destination pages to the freeing scanner freelist when page migration fails. This prevents unnecessary work done by the freeing scanner but also encourages memory to be as compacted as possible at the end of the zone.

Reported-by Greg Thelen <[email protected]>

Quoted 2 lines

What did Greg actually report? IOW, what if any observable problem is being fixed here?

I detected the problem at runtime seeing that ext4 metadata pages (esp the ones read by "sbi->s_group_desc[i] = sb_bread(sb, block)") were constantly visited by compaction calls of migrate_pages(). These pages had a non-zero b_count which caused fallback_migrate_page() -> try_to_release_page() -> try_to_free_buffers() to fail.

Each zone has a cached migration scanner pfn for memory compaction so that subsequent calls to memory compaction can start where the previous call left off.

Currently, the compaction migration scanner only updates the per-zone cached pfn when pageblocks were not skipped for async compaction. This creates a dependency on calling sync compaction to avoid having subsequent calls to async compaction from scanning an enormous amount of non-MOVABLE pageblocks each time it is called. On large machines, this could be potentially very expensive.

This patch adds a per-zone cached migration scanner pfn only for async compaction. It is updated everytime a pageblock has been scanned in its entirety and when no pages from it were successfully isolated. The cached migration scanner pfn for sync compaction is updated only when called for sync compaction.

Signed-off-by David Rientjes <[email protected]>

---
v3: do not update pageblock skip metadata when skipped due to async per
Vlastimil.

  include/linux/mmzone.h |  5 ++--
  mm/compaction.c        | 66 ++++++++++++++++++++++++++++++--------------------
  2 files changed, 43 insertions(+), 28 deletions(-)

include/linux/mmzone.h +3 -2

@@ -360,9 +360,10 @@ struct zone {
  	/* Set to true when the PG_migrate_skip bits should be cleared */
  	bool			compact_blockskip_flush;
 
-	/* pfns where compaction scanners should start */
+	/* pfn where compaction free scanner should start */
  	unsigned long		compact_cached_free_pfn;
-	unsigned long		compact_cached_migrate_pfn;
+	/* pfn where async and sync compaction migration scanner should start */
+	unsigned long		compact_cached_migrate_pfn[2];
  #endif
  #ifdef CONFIG_MEMORY_HOTPLUG
  	/* see spanned/present_pages for more description */

mm/compaction.c +38 -25

@@ -89,7 +89,8 @@ static void __reset_isolation_suitable(struct zone *zone)
  	unsigned long end_pfn = zone_end_pfn(zone);
  	unsigned long pfn;
 
-	zone->compact_cached_migrate_pfn = start_pfn;
+	zone->compact_cached_migrate_pfn[0] = start_pfn;
+	zone->compact_cached_migrate_pfn[1] = start_pfn;
  	zone->compact_cached_free_pfn = end_pfn;
  	zone->compact_blockskip_flush = false;
 

@@ -131,9 +132,10 @@ void reset_isolation_suitable(pg_data_t *pgdat)
   */
  static void update_pageblock_skip(struct compact_control *cc,
  			struct page *page, unsigned long nr_isolated,
-			bool migrate_scanner)
+			bool set_unsuitable, bool migrate_scanner)
  {
  	struct zone *zone = cc->zone;
+	unsigned long pfn;
 
  	if (cc->ignore_skip_hint)
  		return;

@@ -141,20 +143,31 @@ static void update_pageblock_skip(struct compact_control *cc,
  	if (!page)
  		return;
 
-	if (!nr_isolated) {
-		unsigned long pfn = page_to_pfn(page);
+	if (nr_isolated)
+		return;
+
+	/*
+	 * Only skip pageblocks when all forms of compaction will be known to
+	 * fail in the near future.
+	 */
+	if (set_unsuitable)
  		set_pageblock_skip(page);
 
-		/* Update where compaction should restart */
-		if (migrate_scanner) {
-			if (!cc->finished_update_migrate &&
-			    pfn > zone->compact_cached_migrate_pfn)
-				zone->compact_cached_migrate_pfn = pfn;
-		} else {
-			if (!cc->finished_update_free &&
-			    pfn < zone->compact_cached_free_pfn)
-				zone->compact_cached_free_pfn = pfn;
-		}
+	pfn = page_to_pfn(page);
+
+	/* Update where async and sync compaction should restart */
+	if (migrate_scanner) {
+		if (cc->finished_update_migrate)
+			return;
+		if (pfn > zone->compact_cached_migrate_pfn[0])
+			zone->compact_cached_migrate_pfn[0] = pfn;
+		if (cc->sync && pfn > zone->compact_cached_migrate_pfn[1])
+			zone->compact_cached_migrate_pfn[1] = pfn;
+	} else {
+		if (cc->finished_update_free)
+			return;
+		if (pfn < zone->compact_cached_free_pfn)
+			zone->compact_cached_free_pfn = pfn;
  	}
  }
  #else

@@ -166,7 +179,7 @@ static inline bool isolation_suitable(struct compact_control *cc,
 
  static void update_pageblock_skip(struct compact_control *cc,
  			struct page *page, unsigned long nr_isolated,
-			bool migrate_scanner)
+			bool set_unsuitable, bool migrate_scanner)
  {
  }
  #endif /* CONFIG_COMPACTION */

@@ -329,7 +342,8 @@ isolate_fail:
 
  	/* Update the pageblock-skip if the whole pageblock was scanned */
  	if (blockpfn == end_pfn)
-		update_pageblock_skip(cc, valid_page, total_isolated, false);
+		update_pageblock_skip(cc, valid_page, total_isolated, true,
+				      false);
 
  	count_compact_events(COMPACTFREE_SCANNED, nr_scanned);
  	if (total_isolated)

@@ -464,7 +478,7 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  	unsigned long flags;
  	bool locked = false;
  	struct page *page = NULL, *valid_page = NULL;
-	bool skipped_async_unsuitable = false;
+	bool set_unsuitable = true;
  	const isolate_mode_t mode = (!cc->sync ? ISOLATE_ASYNC_MIGRATE : 0) |
  				    (unevictable ? ISOLATE_UNEVICTABLE : 0);
 

@@ -541,8 +555,7 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  			 */
  			mt = get_pageblock_migratetype(page);
  			if (!cc->sync && !migrate_async_suitable(mt)) {
-				cc->finished_update_migrate = true;
-				skipped_async_unsuitable = true;
+				set_unsuitable = false;
  				goto next_pageblock;
  			}
  		}

@@ -646,11 +659,10 @@ next_pageblock:
  	/*
  	 * Update the pageblock-skip information and cached scanner pfn,
  	 * if the whole pageblock was scanned without isolating any page.
-	 * This is not done when pageblock was skipped due to being unsuitable
-	 * for async compaction, so that eventual sync compaction can try.
  	 */
-	if (low_pfn == end_pfn && !skipped_async_unsuitable)
-		update_pageblock_skip(cc, valid_page, nr_isolated, true);
+	if (low_pfn == end_pfn)
+		update_pageblock_skip(cc, valid_page, nr_isolated,
+				      set_unsuitable, true);
 
  	trace_mm_compaction_isolate_migratepages(nr_scanned, nr_isolated);
 

@@ -877,7 +889,8 @@ static int compact_finished(struct zone *zone,
  	/* Compaction run completes if the migrate and free scanner meet */
  	if (cc->free_pfn <= cc->migrate_pfn) {
  		/* Let the next compaction start anew. */
-		zone->compact_cached_migrate_pfn = zone->zone_start_pfn;
+		zone->compact_cached_migrate_pfn[0] = zone->zone_start_pfn;
+		zone->compact_cached_migrate_pfn[1] = zone->zone_start_pfn;
  		zone->compact_cached_free_pfn = zone_end_pfn(zone);
 
  		/*

@@ -1002,7 +1015,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  	 * information on where the scanners should start but check that it
  	 * is initialised by ensuring the values are within zone boundaries.
  	 */
-	cc->migrate_pfn = zone->compact_cached_migrate_pfn;
+	cc->migrate_pfn = zone->compact_cached_migrate_pfn[cc->sync];
  	cc->free_pfn = zone->compact_cached_free_pfn;
  	if (cc->free_pfn < start_pfn || cc->free_pfn > end_pfn) {
  		cc->free_pfn = end_pfn & ~(pageblock_nr_pages-1);

Each zone has a cached migration scanner pfn for memory compaction so that subsequent calls to memory compaction can start where the previous call left off.

Currently, the compaction migration scanner only updates the per-zone cached pfn when pageblocks were not skipped for async compaction. This creates a dependency on calling sync compaction to avoid having subsequent calls to async compaction from scanning an enormous amount of non-MOVABLE pageblocks each time it is called. On large machines, this could be potentially very expensive.

This patch adds a per-zone cached migration scanner pfn only for async compaction. It is updated everytime a pageblock has been scanned in its entirety and when no pages from it were successfully isolated. The cached migration scanner pfn for sync compaction is updated only when called for sync compaction.

Signed-off-by David Rientjes <[email protected]>

---
v3: do not update pageblock skip metadata when skipped due to async per
Vlastimil.

We're going to want to manipulate the migration mode for compaction in the page allocator, and currently compact_control's sync field is only a bool.

Currently, we only do MIGRATE_ASYNC or MIGRATE_SYNC_LIGHT compaction depending on the value of this bool. Convert the bool to enum migrate_mode and pass the migration mode in directly. Later, we'll want to avoid MIGRATE_SYNC_LIGHT for thp allocations in the pagefault patch to avoid unnecessary latency.

This also alters compaction triggered from sysfs, either for the entire system or for a node, to force MIGRATE_SYNC.

Suggested-by Mel Gorman <[email protected]>

Signed-off-by David Rientjes <[email protected]>

---
  include/linux/compaction.h |  4 ++--
  mm/compaction.c            | 36 +++++++++++++++++++-----------------
  mm/internal.h              |  2 +-
  mm/page_alloc.c            | 37 ++++++++++++++++---------------------
  4 files changed, 38 insertions(+), 41 deletions(-)

include/linux/compaction.h +1 -1

  extern void compact_pgdat(pg_data_t *pgdat, int order);
  extern void reset_isolation_suitable(pg_data_t *pgdat);
  extern unsigned long compaction_suitable(struct zone *zone, int order);

@@ -91,7 +91,7 @@ static inline bool compaction_restarting(struct zone *zone, int order)
  #else
  static inline unsigned long try_to_compact_pages(struct zonelist *zonelist,
  			int order, gfp_t gfp_mask, nodemask_t *nodemask,
-			bool sync, bool *contended)
+			enum migrate_mode sync, bool *contended)
  {
  	return COMPACT_CONTINUE;
  }

mm/compaction.c +19 -17

@@ -161,7 +161,8 @@ static void update_pageblock_skip(struct compact_control *cc,
  			return;
  		if (pfn > zone->compact_cached_migrate_pfn[0])
  			zone->compact_cached_migrate_pfn[0] = pfn;
-		if (cc->sync && pfn > zone->compact_cached_migrate_pfn[1])
+		if (cc->mode != MIGRATE_ASYNC &&
+		    pfn > zone->compact_cached_migrate_pfn[1])
  			zone->compact_cached_migrate_pfn[1] = pfn;
  	} else {
  		if (cc->finished_update_free)

@@ -208,7 +209,7 @@ static bool compact_checklock_irqsave(spinlock_t *lock, unsigned long *flags,
  		}
 
  		/* async aborts if taking too long or contended */
-		if (!cc->sync) {
+		if (cc->mode == MIGRATE_ASYNC) {
  			cc->contended = true;
  			return false;
  		}

@@ -479,7 +480,8 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  	bool locked = false;
  	struct page *page = NULL, *valid_page = NULL;
  	bool set_unsuitable = true;
-	const isolate_mode_t mode = (!cc->sync ? ISOLATE_ASYNC_MIGRATE : 0) |
+	const isolate_mode_t mode = (cc->mode == MIGRATE_ASYNC ?
+					ISOLATE_ASYNC_MIGRATE : 0) |
  				    (unevictable ? ISOLATE_UNEVICTABLE : 0);
 
  	/*

@@ -489,7 +491,7 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  	 */
  	while (unlikely(too_many_isolated(zone))) {
  		/* async migration should just abort */
-		if (!cc->sync)
+		if (cc->mode == MIGRATE_ASYNC)
  			return 0;
 
  		congestion_wait(BLK_RW_ASYNC, HZ/10);

@@ -554,7 +556,8 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
  			 * the minimum amount of work satisfies the allocation
  			 */
  			mt = get_pageblock_migratetype(page);
-			if (!cc->sync && !migrate_async_suitable(mt)) {
+			if (cc->mode == MIGRATE_ASYNC &&
+			    !migrate_async_suitable(mt)) {
  				set_unsuitable = false;
  				goto next_pageblock;
  			}

@@ -990,6 +993,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  	int ret;
  	unsigned long start_pfn = zone->zone_start_pfn;
  	unsigned long end_pfn = zone_end_pfn(zone);
+	const bool sync = cc->mode != MIGRATE_ASYNC;
 
  	ret = compaction_suitable(zone, cc->order);
  	switch (ret) {

@@ -1015,7 +1019,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
  	 * information on where the scanners should start but check that it
  	 * is initialised by ensuring the values are within zone boundaries.
  	 */
-	cc->migrate_pfn = zone->compact_cached_migrate_pfn[cc->sync];
+	cc->migrate_pfn = zone->compact_cached_migrate_pfn[sync];
  	cc->free_pfn = zone->compact_cached_free_pfn;
  	if (cc->free_pfn < start_pfn || cc->free_pfn > end_pfn) {
  		cc->free_pfn = end_pfn & ~(pageblock_nr_pages-1);

@@ -1049,8 +1053,7 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
 
  		nr_migrate = cc->nr_migratepages;
  		err = migrate_pages(&cc->migratepages, compaction_alloc,
-				compaction_free, (unsigned long)cc,
-				cc->sync ? MIGRATE_SYNC_LIGHT : MIGRATE_ASYNC,
+				compaction_free, (unsigned long)cc, cc->mode,
  				MR_COMPACTION);
  		update_nr_listpages(cc);
  		nr_remaining = cc->nr_migratepages;

@@ -1083,9 +1086,8 @@ out:
  	return ret;
  }
 
-static unsigned long compact_zone_order(struct zone *zone,
-				 int order, gfp_t gfp_mask,
-				 bool sync, bool *contended)
+static unsigned long compact_zone_order(struct zone *zone, int order,
+		gfp_t gfp_mask, enum migrate_mode mode, bool *contended)
  {
  	unsigned long ret;
  	struct compact_control cc = {

@@ -1094,7 +1096,7 @@ static unsigned long compact_zone_order(struct zone *zone,
  		.order = order,
  		.migratetype = allocflags_to_migratetype(gfp_mask),
  		.zone = zone,
-		.sync = sync,
+		.mode = mode,
  	};
  	INIT_LIST_HEAD(&cc.freepages);
  	INIT_LIST_HEAD(&cc.migratepages);

@@ -1116,7 +1118,7 @@ int sysctl_extfrag_threshold = 500;
   * @order: The order of the current allocation
   * @gfp_mask: The GFP mask of the current allocation
   * @nodemask: The allowed nodes to allocate from
- * @sync: Whether migration is synchronous or not
+ * @sync: The migration mode for async, sync light, or sync migration
   * @contended: Return value that is true if compaction was aborted due to lock contention
   * @page: Optionally capture a free page of the requested order during compaction
   *

@@ -1124,7 +1126,7 @@ int sysctl_extfrag_threshold = 500;
   */
  unsigned long try_to_compact_pages(struct zonelist *zonelist,
  			int order, gfp_t gfp_mask, nodemask_t *nodemask,
-			bool sync, bool *contended)
+			enum migrate_mode sync, bool *contended)
  {
  	enum zone_type high_zoneidx = gfp_zone(gfp_mask);
  	int may_enter_fs = gfp_mask & __GFP_FS;

@@ -1189,7 +1191,7 @@ static void __compact_pgdat(pg_data_t *pgdat, struct compact_control *cc)
  						low_wmark_pages(zone), 0, 0))
  				compaction_defer_reset(zone, cc->order, false);
  			/* Currently async compaction is never deferred. */
-			else if (cc->sync)
+			else if (cc->mode != MIGRATE_ASYNC)
  				defer_compaction(zone, cc->order);
  		}
 

@@ -1202,7 +1204,7 @@ void compact_pgdat(pg_data_t *pgdat, int order)
  {
  	struct compact_control cc = {
  		.order = order,
-		.sync = false,
+		.mode = MIGRATE_ASYNC,
  	};
 
  	if (!order)

@@ -1215,7 +1217,7 @@ static void compact_node(int nid)
  {
  	struct compact_control cc = {
  		.order = -1,
-		.sync = true,
+		.mode = MIGRATE_SYNC,
  		.ignore_skip_hint = true,
  	};
 

mm/internal.h +1 -1

@@ -134,7 +134,7 @@ struct compact_control {
  	unsigned long nr_migratepages;	/* Number of pages to migrate */
  	unsigned long free_pfn;		/* isolate_freepages search base */
  	unsigned long migrate_pfn;	/* isolate_migratepages search base */
-	bool sync;			/* Synchronous migration */
+	enum migrate_mode mode;		/* Async or sync migration mode */
  	bool ignore_skip_hint;		/* Scan blocks even if marked skip */
  	bool finished_update_free;	/* True when the zone cached pfns are
  					 * no longer being updated

mm/page_alloc.c +15 -20

@@ -2226,7 +2226,7 @@ static struct page *
  __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
  	struct zonelist *zonelist, enum zone_type high_zoneidx,
  	nodemask_t *nodemask, int alloc_flags, struct zone *preferred_zone,
-	int migratetype, bool sync_migration,
+	int migratetype, enum migrate_mode mode,
  	bool *contended_compaction, bool *deferred_compaction,
  	unsigned long *did_some_progress)
  {

@@ -2240,7 +2240,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 
  	current->flags |= PF_MEMALLOC;
  	*did_some_progress = try_to_compact_pages(zonelist, order, gfp_mask,
-						nodemask, sync_migration,
+						nodemask, mode,
  						contended_compaction);
  	current->flags &= ~PF_MEMALLOC;
 

@@ -2273,7 +2273,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
  		 * As async compaction considers a subset of pageblocks, only
  		 * defer if the failure was a sync compaction failure.
  		 */
-		if (sync_migration)
+		if (mode != MIGRATE_ASYNC)
  			defer_compaction(preferred_zone, order);
 
  		cond_resched();

@@ -2286,9 +2286,8 @@ static inline struct page *
  __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
  	struct zonelist *zonelist, enum zone_type high_zoneidx,
  	nodemask_t *nodemask, int alloc_flags, struct zone *preferred_zone,
-	int migratetype, bool sync_migration,
-	bool *contended_compaction, bool *deferred_compaction,
-	unsigned long *did_some_progress)
+	int migratetype, enum migrate_mode mode, bool *contended_compaction,
+	bool *deferred_compaction, unsigned long *did_some_progress)
  {
  	return NULL;
  }

@@ -2483,7 +2482,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
  	int alloc_flags;
  	unsigned long pages_reclaimed = 0;
  	unsigned long did_some_progress;
-	bool sync_migration = false;
+	enum migrate_mode migration_mode = MIGRATE_ASYNC;
  	bool deferred_compaction = false;
  	bool contended_compaction = false;
 

@@ -2577,17 +2576,15 @@ rebalance:
  	 * Try direct compaction. The first pass is asynchronous. Subsequent
  	 * attempts after direct reclaim are synchronous
  	 */
-	page = __alloc_pages_direct_compact(gfp_mask, order,
-					zonelist, high_zoneidx,
-					nodemask,
-					alloc_flags, preferred_zone,
-					migratetype, sync_migration,
-					&contended_compaction,
+	page = __alloc_pages_direct_compact(gfp_mask, order, zonelist,
+					high_zoneidx, nodemask, alloc_flags,
+					preferred_zone, migratetype,
+					migration_mode, &contended_compaction,
  					&deferred_compaction,
  					&did_some_progress);
  	if (page)
  		goto got_pg;
-	sync_migration = true;
+	migration_mode = MIGRATE_SYNC_LIGHT;
 
  	/*
  	 * If compaction is deferred for high-order allocations, it is because

@@ -2662,12 +2659,10 @@ rebalance:
  		 * direct reclaim and reclaim/compaction depends on compaction
  		 * being called after reclaim so call directly if necessary
  		 */
-		page = __alloc_pages_direct_compact(gfp_mask, order,
-					zonelist, high_zoneidx,
-					nodemask,
-					alloc_flags, preferred_zone,
-					migratetype, sync_migration,
-					&contended_compaction,
+		page = __alloc_pages_direct_compact(gfp_mask, order, zonelist,
+					high_zoneidx, nodemask, alloc_flags,
+					preferred_zone, migratetype,
+					migration_mode, &contended_compaction,
  					&deferred_compaction,
  					&did_some_progress);
  		if (page)