mm: compaction: partially revert capture of suitable high-order page

Eric Wong reported on 3.7 and 3.8-rc2 that ppoll() got stuck when
waiting for POLLIN on a local TCP socket.  It was easier to trigger if
there was disk IO and dirty pages at the same time and he bisected it to
commit 1fb3f8ca0e92 ("mm: compaction: capture a suitable high-order page
immediately when it is made available").

The intention of that patch was to improve high-order allocations under
memory pressure after changes made to reclaim in 3.6 drastically hurt
THP allocations but the approach was flawed.  For Eric, the problem was
that page->pfmemalloc was not being cleared for captured pages leading
to a poor interaction with swap-over-NFS support causing the packets to
be dropped.  However, I identified a few more problems with the patch
including the fact that it can increase contention on zone->lock in some
cases which could result in async direct compaction being aborted early.

In retrospect the capture patch took the wrong approach.  What it should
have done is mark the pageblock being migrated as MIGRATE_ISOLATE if it
was allocating for THP and avoided races that way.  While the patch was
showing to improve allocation success rates at the time, the benefit is
marginal given the relative complexity and it should be revisited from
scratch in the context of the other reclaim-related changes that have
taken place since the patch was first written and tested.  This patch
partially reverts commit 1fb3f8ca0e92 ("mm: compaction: capture a
suitable high-order page immediately when it is made available").

Reported-and-tested-by: Eric Wong <normalperson@yhbt.net>
Tested-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Mel Gorman <mgorman@suse.de>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 8 insertions, 27 deletions

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c957805a7f0e..df2022ff0c8a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1384,14 +1384,8 @@ void split_page(struct page *page, unsigned int order)
                 set_page_refcounted(page + i);
 }
 
-/*
- * Similar to the split_page family of functions except that the page
- * required at the given order and being isolated now to prevent races
- * with parallel allocators
- */
-int capture_free_page(struct page *page, int alloc_order, int migratetype)
+static int __isolate_free_page(struct page *page, unsigned int order)
 {
-        unsigned int order;
         unsigned long watermark;
         struct zone *zone;
         int mt;
@@ -1399,7 +1393,6 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
         BUG_ON(!PageBuddy(page));
 
         zone = page_zone(page);
-        order = page_order(page);
         mt = get_pageblock_migratetype(page);
 
         if (mt != MIGRATE_ISOLATE) {
@@ -1408,7 +1401,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
                 if (!zone_watermark_ok(zone, 0, watermark, 0, 0))
                         return 0;
 
-                __mod_zone_freepage_state(zone, -(1UL << alloc_order), mt);
+                __mod_zone_freepage_state(zone, -(1UL << order), mt);
         }
 
         /* Remove page from free list */
@@ -1416,11 +1409,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
         zone->free_area[order].nr_free--;
         rmv_page_order(page);
 
-        if (alloc_order != order)
-                expand(zone, page, alloc_order, order,
-                        &zone->free_area[order], migratetype);
-
-        /* Set the pageblock if the captured page is at least a pageblock */
+        /* Set the pageblock if the isolated page is at least a pageblock */
         if (order >= pageblock_order - 1) {
                 struct page *endpage = page + (1 << order) - 1;
                 for (; page < endpage; page += pageblock_nr_pages) {
@@ -1431,7 +1420,7 @@ int capture_free_page(struct page *page, int alloc_order, int migratetype)
                 }
         }
 
-        return 1UL << alloc_order;
+        return 1UL << order;
 }
 
 /*
@@ -1449,10 +1438,9 @@ int split_free_page(struct page *page)
         unsigned int order;
         int nr_pages;
 
-        BUG_ON(!PageBuddy(page));
         order = page_order(page);
 
-        nr_pages = capture_free_page(page, order, 0);
+        nr_pages = __isolate_free_page(page, order);
         if (!nr_pages)
                 return 0;
 
@@ -2136,8 +2124,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
         bool *contended_compaction, bool *deferred_compaction,
         unsigned long *did_some_progress)
 {
-        struct page *page = NULL;
-
         if (!order)
                 return NULL;
 
@@ -2149,16 +2135,12 @@ __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,
-                                                contended_compaction, &page);
+                                                contended_compaction);
         current->flags &= ~PF_MEMALLOC;
 
-        /* If compaction captured a page, prep and use it */
-        if (page) {
-                prep_new_page(page, order, gfp_mask);
-                goto got_page;
-        }
-
         if (*did_some_progress != COMPACT_SKIPPED) {
+                struct page *page;
+
                 /* Page migration frees to the PCP lists but we want merging */
                 drain_pages(get_cpu());
                 put_cpu();
@@ -2168,7 +2150,6 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
                                 alloc_flags & ~ALLOC_NO_WATERMARKS,
                                 preferred_zone, migratetype);
                 if (page) {
-got_page:
                         preferred_zone->compact_blockskip_flush = false;
                         preferred_zone->compact_considered = 0;
                         preferred_zone->compact_defer_shift = 0;