Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933712AbaLKDFz (ORCPT ); Wed, 10 Dec 2014 22:05:55 -0500 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:38966 "EHLO lgemrelse6q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933176AbaLKDFx (ORCPT ); Wed, 10 Dec 2014 22:05:53 -0500 X-Original-SENDERIP: 10.177.222.213 X-Original-MAILFROM: iamjoonsoo.kim@lge.com Date: Thu, 11 Dec 2014 12:09:48 +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 4/4] mm/compaction: stop the isolation when we isolate enough freepage Message-ID: <20141211030948.GB16381@js1304-P5Q-DELUXE> References: <1418022980-4584-1-git-send-email-iamjoonsoo.kim@lge.com> <1418022980-4584-5-git-send-email-iamjoonsoo.kim@lge.com> <54857675.5080400@suse.cz> <20141210070055.GE13371@js1304-P5Q-DELUXE> <54886471.9050306@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54886471.9050306@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 Wed, Dec 10, 2014 at 04:19:13PM +0100, Vlastimil Babka wrote: > On 12/10/2014 08:00 AM, Joonsoo Kim wrote: > >On Mon, Dec 08, 2014 at 10:59:17AM +0100, Vlastimil Babka wrote: > >>On 12/08/2014 08:16 AM, Joonsoo Kim wrote: > >>>From: Joonsoo Kim > >>> > >>>Currently, freepage isolation in one pageblock doesn't consider how many > >>>freepages we isolate. When I traced flow of compaction, compaction > >>>sometimes isolates more than 256 freepages to migrate just 32 pages. > >>> > >>>In this patch, freepage isolation is stopped at the point that we > >>>have more isolated freepage than isolated page for migration. This > >>>results in slowing down free page scanner and make compaction success > >>>rate higher. > >>> > >>>stress-highalloc test in mmtests with non movable order 7 allocation shows > >>>increase of compaction success rate and slight improvement of allocation > >>>success rate. > >>> > >>>Allocation success rate on phase 1 (%) > >>>62.70 : 64.00 > >>> > >>>Compaction success rate (Compaction success * 100 / Compaction stalls, %) > >>>35.13 : 41.50 > >> > >>This is weird. I could maybe understand that isolating too many > > > >In fact, I also didn't fully understand why it results in this > >result. :) > > > >>freepages and then returning them is a waste of time if compaction > >>terminates immediately after the following migration (otherwise we > >>would keep those free pages for the future migrations within same > >>compaction run). And wasting time could reduce success rates for > >>async compaction terminating prematurely due to cond_resched(), but > >>that should be all the difference, unless there's another subtle > >>bug, no? > > > >My guess is that there is bad effect when we release isolated > >freepages. In asynchronous compaction, this happens quite easily. > >In this case, freepages are returned to page allocator and, maybe, > >they are on pcp list or front of buddy list so they would be used by > >another user at first. This reduces freepages we can utilize so > >compaction is finished earlier. > > Hmm, some might even stay on the pcplists and we won't isolate them > again. So we will leave them behind. I wouldn't expect such big > difference here, but anyway... > It might be interesting to evaluate if a pcplists drain after > returning isolated freepages (unless the scanners have already met, > that's pointless) would make any difference. Yes, I will check it. > > >> > >>>pfn where both scanners meets on compaction complete > >>>(separate test due to enormous tracepoint buffer) > >>>(zone_start=4096, zone_end=1048576) > >>>586034 : 654378 > >> > >>The difference here suggests that there is indeed another subtle bug > >>related to where free scanner restarts, and we must be leaving the > >>excessively isolated (and then returned) freepages behind. Otherwise > >>I think the scanners should meet at the same place regardless of > >>your patch. > > > >I tried to find another subtle bug, but, can't find any critical one. > >Hmm... > > > >Anyway, regardless of the reason of result, this patch seems reasonable, > >because we don't need to waste time to isolate unneeded freepages. > > Right. > > >Thanks. > > > >> > >>>Signed-off-by: Joonsoo Kim > >>>--- > >>> mm/compaction.c | 17 ++++++++++------- > >>> 1 file changed, 10 insertions(+), 7 deletions(-) > >>> > >>>diff --git a/mm/compaction.c b/mm/compaction.c > >>>index 2fd5f79..12223b9 100644 > >>>--- a/mm/compaction.c > >>>+++ b/mm/compaction.c > >>>@@ -422,6 +422,13 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > >>> > >>> /* If a page was split, advance to the end of it */ > >>> if (isolated) { > >>>+ cc->nr_freepages += isolated; > >>>+ if (!strict && > >>>+ cc->nr_migratepages <= cc->nr_freepages) { > >>>+ blockpfn += isolated; > >>>+ break; > >>>+ } > >>>+ > >>> blockpfn += isolated - 1; > >>> cursor += isolated - 1; > >>> continue; > >>>@@ -831,7 +838,6 @@ static void isolate_freepages(struct compact_control *cc) > >>> unsigned long isolate_start_pfn; /* exact pfn we start at */ > >>> unsigned long block_end_pfn; /* end of current pageblock */ > >>> unsigned long low_pfn; /* lowest pfn scanner is able to scan */ > >>>- int nr_freepages = cc->nr_freepages; > >>> struct list_head *freelist = &cc->freepages; > >>> > >>> /* > >>>@@ -856,11 +862,11 @@ static void isolate_freepages(struct compact_control *cc) > >>> * pages on cc->migratepages. We stop searching if the migrate > >>> * and free page scanners meet or enough free pages are isolated. > >>> */ > >>>- for (; block_start_pfn >= low_pfn && cc->nr_migratepages > nr_freepages; > >>>+ for (; block_start_pfn >= low_pfn && > >>>+ cc->nr_migratepages > cc->nr_freepages; > >>> block_end_pfn = block_start_pfn, > >>> block_start_pfn -= pageblock_nr_pages, > >>> isolate_start_pfn = block_start_pfn) { > >>>- unsigned long isolated; > >>> > >>> /* > >>> * This can iterate a massively long zone without finding any > >>>@@ -885,9 +891,8 @@ static void isolate_freepages(struct compact_control *cc) > >>> continue; > >>> > >>> /* Found a block suitable for isolating free pages from. */ > >>>- isolated = isolate_freepages_block(cc, &isolate_start_pfn, > >>>+ isolate_freepages_block(cc, &isolate_start_pfn, > >>> block_end_pfn, freelist, false); > >>>- nr_freepages += isolated; > >>> > >>> /* > >>> * Remember where the free scanner should restart next time, > >>>@@ -919,8 +924,6 @@ static void isolate_freepages(struct compact_control *cc) > >>> */ > >>> if (block_start_pfn < low_pfn) > >>> cc->free_pfn = cc->migrate_pfn; > >>>- > >>>- cc->nr_freepages = nr_freepages; > >>> } > >>> > >>> /* > >>> > >> > >>-- > >>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, 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/