From patchwork Tue Jan 24 16:28:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 719248 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3v7DCk4CVfz9t14; Wed, 25 Jan 2017 03:29:22 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1cW3yB-0005wz-6z; Tue, 24 Jan 2017 16:29:19 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1cW3xt-0005pk-27 for kernel-team@lists.ubuntu.com; Tue, 24 Jan 2017 16:29:01 +0000 Received: from [187.74.228.207] (helo=calabresa.lan) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1cW3xs-0008UG-B4 for kernel-team@lists.ubuntu.com; Tue, 24 Jan 2017 16:29:00 +0000 From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [Xenial PATCH 08/11] mm: consider compaction feedback also for costly allocation Date: Tue, 24 Jan 2017 14:28:36 -0200 Message-Id: <20170124162839.6355-9-cascardo@canonical.com> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20170124162839.6355-1-cascardo@canonical.com> References: <20170124162839.6355-1-cascardo@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: Michal Hocko BugLink: https://bugs.launchpad.net/bugs/1655842 PAGE_ALLOC_COSTLY_ORDER retry logic is mostly handled inside should_reclaim_retry currently where we decide to not retry after at least order worth of pages were reclaimed or the watermark check for at least one zone would succeed after reclaiming all pages if the reclaim hasn't made any progress. Compaction feedback is mostly ignored and we just try to make sure that the compaction did at least something before giving up. The first condition was added by a41f24ea9fd6 ("page allocator: smarter retry of costly-order allocations) and it assumed that lumpy reclaim could have created a page of the sufficient order. Lumpy reclaim, has been removed quite some time ago so the assumption doesn't hold anymore. Remove the check for the number of reclaimed pages and rely on the compaction feedback solely. should_reclaim_retry now only makes sure that we keep retrying reclaim for high order pages only if they are hidden by watermaks so order-0 reclaim makes really sense. should_compact_retry now keeps retrying even for the costly allocations. The number of retries is reduced wrt. !costly requests because they are less important and harder to grant and so their pressure shouldn't cause contention for other requests or cause an over reclaim. We also do not reset no_progress_loops for costly request to make sure we do not keep reclaiming too agressively. This has been tested by running a process which fragments memory: - compact memory - mmap large portion of the memory (1920M on 2GRAM machine with 2G of swapspace) - MADV_DONTNEED single page in PAGE_SIZE*((1UL< /proc/sys/vm/overcommit_memory;echo 1 > /proc/sys/vm/compact_memory; ./fragment-mem-and-run /root/alloc_hugepages.sh 1920M 250M Node 0, zone DMA 31 28 31 10 2 0 2 1 2 3 1 Node 0, zone DMA32 437 319 171 50 28 25 20 16 16 14 437 * This is the /proc/buddyinfo after the compaction Done fragmenting. size=2013265920 freed=262144000 Node 0, zone DMA 165 48 3 1 2 0 2 2 2 2 0 Node 0, zone DMA32 35109 14575 185 51 41 12 6 0 0 0 0 * /proc/buddyinfo after memory got fragmented Executing "/root/alloc_hugepages.sh" Eating some pagecache 508623+0 records in 508623+0 records out 2083319808 bytes (2.1 GB) copied, 11.7292 s, 178 MB/s Node 0, zone DMA 3 5 3 1 2 0 2 2 2 2 0 Node 0, zone DMA32 111 344 153 20 24 10 3 0 0 0 0 * /proc/buddyinfo after page cache got eaten Trying to allocate 129 129 * 129 hugepages requested and all of them granted. Node 0, zone DMA 3 5 3 1 2 0 2 2 2 2 0 Node 0, zone DMA32 127 97 30 99 11 6 2 1 4 0 0 * /proc/buddyinfo after hugetlb allocation. 10 runs will behave as follows: Trying to allocate 130 130 --- Trying to allocate 129 129 -- Trying to allocate 128 128 -- Trying to allocate 129 129 -- Trying to allocate 128 128 -- Trying to allocate 129 129 -- Trying to allocate 132 132 -- Trying to allocate 129 129 -- Trying to allocate 128 128 -- Trying to allocate 129 129 So basically 100% success for all 10 attempts. Without the patch numbers looked much worse: Trying to allocate 128 12 -- Trying to allocate 129 14 -- Trying to allocate 129 7 -- Trying to allocate 129 16 -- Trying to allocate 129 30 -- Trying to allocate 129 38 -- Trying to allocate 129 19 -- Trying to allocate 129 37 -- Trying to allocate 129 28 -- Trying to allocate 129 37 Just for completness the base kernel without oom detection rework looks as follows: Trying to allocate 127 30 -- Trying to allocate 129 12 -- Trying to allocate 129 52 -- Trying to allocate 128 32 -- Trying to allocate 129 12 -- Trying to allocate 129 10 -- Trying to allocate 129 32 -- Trying to allocate 128 14 -- Trying to allocate 128 16 -- Trying to allocate 129 8 As we can see the success rate is much more volatile and smaller without this patch. So the patch not only makes the retry logic for costly requests more sensible the success rate is even higher. Signed-off-by: Michal Hocko Acked-by: Vlastimil Babka Acked-by: Hillf Danton Cc: David Rientjes Cc: Johannes Weiner Cc: Joonsoo Kim Cc: Mel Gorman Cc: Tetsuo Handa Cc: Vladimir Davydov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds (cherry picked from commit 7854ea6c28c6076050e24773eeb78e2925bd7411) Signed-off-by: Thadeu Lima de Souza Cascardo --- mm/page_alloc.c | 63 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 1b293da..e97acfa 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2851,6 +2851,8 @@ should_compact_retry(unsigned int order, enum compact_result compact_result, enum migrate_mode *migrate_mode, int compaction_retries) { + int max_retries = MAX_COMPACT_RETRIES; + if (!order) return false; @@ -2868,17 +2870,24 @@ should_compact_retry(unsigned int order, enum compact_result compact_result, } /* - * !costly allocations are really important and we have to make sure - * the compaction wasn't deferred or didn't bail out early due to locks - * contention before we go OOM. Still cap the reclaim retry loops with - * progress to prevent from looping forever and potential trashing. + * make sure the compaction wasn't deferred or didn't bail out early + * due to locks contention before we declare that we should give up. */ - if (order <= PAGE_ALLOC_COSTLY_ORDER) { - if (compaction_withdrawn(compact_result)) - return true; - if (compaction_retries <= MAX_COMPACT_RETRIES) - return true; - } + if (compaction_withdrawn(compact_result)) + return true; + + /* + * !costly requests are much more important than __GFP_REPEAT + * costly ones because they are de facto nofail and invoke OOM + * killer to move on while costly can fail and users are ready + * to cope with that. 1/4 retries is rather arbitrary but we + * would need much more detailed feedback from compaction to + * make a better decision. + */ + if (order > PAGE_ALLOC_COSTLY_ORDER) + max_retries /= 4; + if (compaction_retries <= max_retries) + return true; return false; } @@ -3062,18 +3071,17 @@ static inline bool is_thp_gfp_mask(gfp_t gfp_mask) * Checks whether it makes sense to retry the reclaim to make a forward progress * for the given allocation request. * The reclaim feedback represented by did_some_progress (any progress during - * the last reclaim round), pages_reclaimed (cumulative number of reclaimed - * pages) and no_progress_loops (number of reclaim rounds without any progress - * in a row) is considered as well as the reclaimable pages on the applicable - * zone list (with a backoff mechanism which is a function of no_progress_loops). + * the last reclaim round) and no_progress_loops (number of reclaim rounds without + * any progress in a row) is considered as well as the reclaimable pages on the + * applicable zone list (with a backoff mechanism which is a function of + * no_progress_loops). * * Returns true if a retry is viable or false to enter the oom path. */ static inline bool should_reclaim_retry(gfp_t gfp_mask, unsigned order, struct alloc_context *ac, int alloc_flags, - bool did_some_progress, unsigned long pages_reclaimed, - int no_progress_loops) + bool did_some_progress, int no_progress_loops) { struct zone *zone; struct zoneref *z; @@ -3085,14 +3093,6 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order, if (no_progress_loops > MAX_RECLAIM_RETRIES) return false; - if (order > PAGE_ALLOC_COSTLY_ORDER) { - if (pages_reclaimed >= (1< PAGE_ALLOC_COSTLY_ORDER && !(gfp_mask & __GFP_REPEAT)) goto noretry; - if (did_some_progress) { + /* + * Costly allocations might have made a progress but this doesn't mean + * their order will become available due to high fragmentation so + * always increment the no progress counter for them + */ + if (did_some_progress && order <= PAGE_ALLOC_COSTLY_ORDER) no_progress_loops = 0; - pages_reclaimed += did_some_progress; - } else { + else no_progress_loops++; - } if (should_reclaim_retry(gfp_mask, order, ac, alloc_flags, - did_some_progress > 0, pages_reclaimed, - no_progress_loops)) + did_some_progress > 0, no_progress_loops)) goto retry; /*