mm, compaction: skip buddy pages by their order in the migrate scanner

The migration scanner skips PageBuddy pages, but does not consider their
order as checking page_order() is generally unsafe without holding the
zone->lock, and acquiring the lock just for the check wouldn't be a good
tradeoff.

Still, this could avoid some iterations over the rest of the buddy page,
and if we are careful, the race window between PageBuddy() check and
page_order() is small, and the worst thing that can happen is that we skip
too much and miss some isolation candidates.  This is not that bad, as
compaction can already fail for many other reasons like parallel
allocations, and those have much larger race window.

This patch therefore makes the migration scanner obtain the buddy page
order and use it to skip the whole buddy page, if the order appears to be
in the valid range.

It's important that the page_order() is read only once, so that the value
used in the checks and in the pfn calculation is the same.  But in theory
the compiler can replace the local variable by multiple inlines of
page_order().  Therefore, the patch introduces page_order_unsafe() that
uses ACCESS_ONCE to prevent this.

Testing with stress-highalloc from mmtests shows a 15% reduction in number
of pages scanned by migration scanner.  The reduction is >60% with
__GFP_NO_KSWAPD allocations, along with success rates better by few
percent.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: Mel Gorman <mgorman@suse.de>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Rik van Riel <riel@redhat.com>
Acked-by: David Rientjes <rientjes@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 31 insertions, 5 deletions

diff --git a/mm/compaction.c b/mm/compaction.c
index b69b7dac0361..b9cf751cc00e 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -313,8 +313,15 @@ static inline bool compact_should_abort(struct compact_control *cc)
 static bool suitable_migration_target(struct page *page)
 {
         /* If the page is a large free page, then disallow migration */
-        if (PageBuddy(page) && page_order(page) >= pageblock_order)
-                return false;
+        if (PageBuddy(page)) {
+                /*
+                 * We are checking page_order without zone->lock taken. But
+                 * the only small danger is that we skip a potentially suitable
+                 * pageblock, so it's not worth to check order for valid range.
+                 */
+                if (page_order_unsafe(page) >= pageblock_order)
+                        return false;
+        }
 
         /* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */
         if (migrate_async_suitable(get_pageblock_migratetype(page)))
@@ -608,11 +615,23 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
                         valid_page = page;
 
                 /*
-                 * Skip if free. page_order cannot be used without zone->lock
-                 * as nothing prevents parallel allocations or buddy merging.
+                 * Skip if free. We read page order here without zone lock
+                 * which is generally unsafe, but the race window is small and
+                 * the worst thing that can happen is that we skip some
+                 * potential isolation targets.
                  */
-                if (PageBuddy(page))
+                if (PageBuddy(page)) {
+                        unsigned long freepage_order = page_order_unsafe(page);
+
+                        /*
+                         * Without lock, we cannot be sure that what we got is
+                         * a valid page order. Consider only values in the
+                         * valid order range to prevent low_pfn overflow.
+                         */
+                        if (freepage_order > 0 && freepage_order < MAX_ORDER)
+                                low_pfn += (1UL << freepage_order) - 1;
                         continue;
+                }
 
                 /*
                  * Check may be lockless but that's ok as we recheck later.
@@ -698,6 +717,13 @@ isolate_success:
                 }
         }
 
+        /*
+         * The PageBuddy() check could have potentially brought us outside
+         * the range to be scanned.
+         */
+        if (unlikely(low_pfn > end_pfn))
+                low_pfn = end_pfn;
+
         if (locked)
                 spin_unlock_irqrestore(&zone->lru_lock, flags);