Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756583AbaLJGml (ORCPT ); Wed, 10 Dec 2014 01:42:41 -0500 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:51469 "EHLO lgemrelse6q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753359AbaLJGmj (ORCPT ); Wed, 10 Dec 2014 01:42:39 -0500 X-Original-SENDERIP: 10.177.222.213 X-Original-MAILFROM: iamjoonsoo.kim@lge.com Date: Wed, 10 Dec 2014 15:46:32 +0900 From: Joonsoo Kim To: Vlastimil Babka Cc: Andrew Morton , Mel Gorman , David Rientjes , Rik van Riel , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] mm/compaction: enhance compaction finish condition Message-ID: <20141210064632.GD13371@js1304-P5Q-DELUXE> References: <1418022980-4584-1-git-send-email-iamjoonsoo.kim@lge.com> <1418022980-4584-4-git-send-email-iamjoonsoo.kim@lge.com> <5485708D.3070009@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5485708D.3070009@suse.cz> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Dec 08, 2014 at 10:34:05AM +0100, Vlastimil Babka wrote: > On 12/08/2014 08:16 AM, Joonsoo Kim wrote: > >Compaction has anti fragmentation algorithm. It is that freepage > >should be more than pageblock order to finish the compaction if we don't > >find any freepage in requested migratetype buddy list. This is for > >mitigating fragmentation, but, it is a lack of migratetype consideration > >and too excessive. > > > >At first, it doesn't consider migratetype so there would be false positive > >on compaction finish decision. For example, if allocation request is > >for unmovable migratetype, freepage in CMA migratetype doesn't help that > >allocation, so compaction should not be stopped. But, current logic > >considers it as compaction is no longer needed and stop the compaction. > > > >Secondly, it is too excessive. We can steal freepage from other migratetype > >and change pageblock migratetype on more relaxed conditions. In page > >allocator, there is another conditions that can succeed to steal without > >introducing fragmentation. > > > >To solve these problems, this patch borrows anti fragmentation logic from > >page allocator. It will reduce premature compaction finish in some cases > >and reduce excessive compaction work. > > > >stress-highalloc test in mmtests with non movable order 7 allocation shows > >in allocation success rate on phase 1 and compaction success rate. > > > >Allocation success rate on phase 1 (%) > >57.00 : 63.67 > > > >Compaction success rate (Compaction success * 100 / Compaction stalls, %) > >28.94 : 35.13 > > > >Signed-off-by: Joonsoo Kim > >--- > > include/linux/mmzone.h | 3 +++ > > mm/compaction.c | 31 +++++++++++++++++++++++++++++-- > > mm/internal.h | 1 + > > mm/page_alloc.c | 5 ++--- > > 4 files changed, 35 insertions(+), 5 deletions(-) > > > >diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > >index 2f0856d..87f5bb5 100644 > >--- a/include/linux/mmzone.h > >+++ b/include/linux/mmzone.h > >@@ -63,6 +63,9 @@ enum { > > MIGRATE_TYPES > > }; > > > >+#define FALLBACK_MIGRATETYPES (4) > >+extern int fallbacks[MIGRATE_TYPES][FALLBACK_MIGRATETYPES]; > >+ > > #ifdef CONFIG_CMA > > # define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA) > > #else > >diff --git a/mm/compaction.c b/mm/compaction.c > >index 1a5f465..2fd5f79 100644 > >--- a/mm/compaction.c > >+++ b/mm/compaction.c > >@@ -1054,6 +1054,30 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone, > > return cc->nr_migratepages ? ISOLATE_SUCCESS : ISOLATE_NONE; > > } > > > >+static bool can_steal_fallbacks(struct free_area *area, > >+ unsigned int order, int migratetype) > >+{ > >+ int i; > >+ int fallback_mt; > >+ > >+ if (area->nr_free == 0) > >+ return false; > >+ > >+ for (i = 0; i < FALLBACK_MIGRATETYPES; i++) { > >+ fallback_mt = fallbacks[migratetype][i]; > >+ if (fallback_mt == MIGRATE_RESERVE) > >+ break; > >+ > >+ if (list_empty(&area->free_list[fallback_mt])) > >+ continue; > >+ > >+ if (can_steal_freepages(order, migratetype, fallback_mt)) > >+ return true; > >+ } > >+ > >+ return false; > >+} > >+ > > static int __compact_finished(struct zone *zone, struct compact_control *cc, > > const int migratetype) > > { > >@@ -1104,8 +1128,11 @@ static int __compact_finished(struct zone *zone, struct compact_control *cc, > > if (!list_empty(&area->free_list[migratetype])) > > return COMPACT_PARTIAL; > > > >- /* Job done if allocation would set block type */ > >- if (order >= pageblock_order && area->nr_free) > > So, can_steal_fallbacks() -> can_steal_freepages() is quite involved > way if in the end we just realize that order >= pageblock_order and > we are stealing whole pageblock. Given that often compaction is done > for THP, it would be better to check order >= pageblock_order and > handle it upfront. This goes together with my comments on previous > patch that order >= pageblock_order is better handled separately. I'd like to keep this order check in can_steal_freepages(). At first, we should first check migratetype before order checking. If high order page is on CMA, we can't steal it. Secondly, I think that maintaining well defined function to check whether we can steal or not is better than separating logic. It would help future maintanance. Thanks. > > >+ /* > >+ * Job done if allocation would steal freepages from > >+ * other migratetype buddy lists. > >+ */ > >+ if (can_steal_fallbacks(area, order, migratetype)) > > return COMPACT_PARTIAL; > > } > > > >diff --git a/mm/internal.h b/mm/internal.h > >index efad241..7028d83 100644 > >--- a/mm/internal.h > >+++ b/mm/internal.h > >@@ -179,6 +179,7 @@ unsigned long > > isolate_migratepages_range(struct compact_control *cc, > > unsigned long low_pfn, unsigned long end_pfn); > > > >+bool can_steal_freepages(unsigned int order, int start_mt, int fallback_mt); > > #endif > > > > /* > >diff --git a/mm/page_alloc.c b/mm/page_alloc.c > >index 7b4c9aa..dcb8523 100644 > >--- a/mm/page_alloc.c > >+++ b/mm/page_alloc.c > >@@ -1031,7 +1031,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order, > > * This array describes the order lists are fallen back to when > > * the free lists for the desirable migrate type are depleted > > */ > >-static int fallbacks[MIGRATE_TYPES][4] = { > >+int fallbacks[MIGRATE_TYPES][FALLBACK_MIGRATETYPES] = { > > [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE }, > > [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE }, > > #ifdef CONFIG_CMA > >@@ -1161,8 +1161,7 @@ static void try_to_steal_freepages(struct zone *zone, struct page *page, > > } > > } > > > >-static bool can_steal_freepages(unsigned int order, > >- int start_mt, int fallback_mt) > >+bool can_steal_freepages(unsigned int order, int start_mt, int fallback_mt) > > { > > /* > > * When borrowing from MIGRATE_CMA, we need to release the excess > > > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: email@kvack.org -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/