From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Yoknis Subject: Re: [PATCH] mm: memmap_init_zone() performance improvement Date: Tue, 06 Nov 2012 09:03:26 -0700 Message-ID: <1352217806.6504.19.camel@MikesLinux.fc.hp.com> References: <1349276174-8398-1-git-send-email-mike.yoknis@hp.com> <20121008151656.GM29125@suse.de> <1349794597.29752.10.camel@MikesLinux.fc.hp.com> <1350676398.1169.6.camel@MikesLinux.fc.hp.com> <20121020082858.GA2698@suse.de> <508FEECE.2070402@linux.vnet.ibm.com> Reply-To: mike.yoknis@hp.com Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <508FEECE.2070402@linux.vnet.ibm.com> Sender: linux-kbuild-owner@vger.kernel.org To: Dave Hansen Cc: Mel Gorman , "mingo@redhat.com" , "akpm@linux-foundation.org" , "linux-arch@vger.kernel.org" , "mmarek@suse.cz" , "tglx@linutronix.de" , "hpa@zytor.com" , "arnd@arndb.de" , "sam@ravnborg.org" , "minchan@kernel.org" , "kamezawa.hiroyu@jp.fujitsu.com" , "mhocko@suse.cz" , "linux-kbuild@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" List-Id: linux-arch.vger.kernel.org On Tue, 2012-10-30 at 09:14 -0600, Dave Hansen wrote: > On 10/20/2012 01:29 AM, Mel Gorman wrote: > > I'm travelling at the moment so apologies that I have not followed up on > > this. My problem is still the same with the patch - it changes more > > headers than is necessary and it is sparsemem specific. At minimum, try > > the suggestion of > > > > if (!early_pfn_valid(pfn)) { > > pfn = ALIGN(pfn + MAX_ORDER_NR_PAGES, MAX_ORDER_NR_PAGES) - 1; > > continue; > > } > > Sorry I didn't catch this until v2... > > Is that ALIGN() correct? If pfn=3, then it would expand to: > > (3+MAX_ORDER_NR_PAGES+MAX_ORDER_NR_PAGES-1) & ~(MAX_ORDER_NR_PAGES-1) > > You would end up skipping the current MAX_ORDER_NR_PAGES area, and then > one _extra_ because ALIGN() aligns up, and you're adding > MAX_ORDER_NR_PAGES too. It doesn't matter unless you run in to a > !early_valid_pfn() in the middle of a MAX_ORDER area, I guess. > > I think this would work, plus be a bit smaller: > > pfn = ALIGN(pfn + 1, MAX_ORDER_NR_PAGES) - 1; > Dave, I see your point about "rounding-up". But, I favor the way Mel suggested it. It more clearly shows the intent, which is to move up by MAX_ORDER_NR_PAGES. The "pfn+1" may suggest that there is some significance to the next pfn, but there is not. I find Mel's way easier to understand. Mike Y From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from g5t0008.atlanta.hp.com ([15.192.0.45]:44771 "EHLO g5t0008.atlanta.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752459Ab2KFQDj (ORCPT ); Tue, 6 Nov 2012 11:03:39 -0500 Message-ID: <1352217806.6504.19.camel@MikesLinux.fc.hp.com> Subject: Re: [PATCH] mm: memmap_init_zone() performance improvement From: Mike Yoknis Reply-To: mike.yoknis@hp.com Date: Tue, 06 Nov 2012 09:03:26 -0700 In-Reply-To: <508FEECE.2070402@linux.vnet.ibm.com> References: <1349276174-8398-1-git-send-email-mike.yoknis@hp.com> <20121008151656.GM29125@suse.de> <1349794597.29752.10.camel@MikesLinux.fc.hp.com> <1350676398.1169.6.camel@MikesLinux.fc.hp.com> <20121020082858.GA2698@suse.de> <508FEECE.2070402@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Dave Hansen Cc: Mel Gorman , "mingo@redhat.com" , "akpm@linux-foundation.org" , "linux-arch@vger.kernel.org" , "mmarek@suse.cz" , "tglx@linutronix.de" , "hpa@zytor.com" , "arnd@arndb.de" , "sam@ravnborg.org" , "minchan@kernel.org" , "kamezawa.hiroyu@jp.fujitsu.com" , "mhocko@suse.cz" , "linux-kbuild@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" Message-ID: <20121106160326.on30rVZYmlJ8us8SNll7LxCWvwSYMYpSm2WC2U6uzCQ@z> On Tue, 2012-10-30 at 09:14 -0600, Dave Hansen wrote: > On 10/20/2012 01:29 AM, Mel Gorman wrote: > > I'm travelling at the moment so apologies that I have not followed up on > > this. My problem is still the same with the patch - it changes more > > headers than is necessary and it is sparsemem specific. At minimum, try > > the suggestion of > > > > if (!early_pfn_valid(pfn)) { > > pfn = ALIGN(pfn + MAX_ORDER_NR_PAGES, MAX_ORDER_NR_PAGES) - 1; > > continue; > > } > > Sorry I didn't catch this until v2... > > Is that ALIGN() correct? If pfn=3, then it would expand to: > > (3+MAX_ORDER_NR_PAGES+MAX_ORDER_NR_PAGES-1) & ~(MAX_ORDER_NR_PAGES-1) > > You would end up skipping the current MAX_ORDER_NR_PAGES area, and then > one _extra_ because ALIGN() aligns up, and you're adding > MAX_ORDER_NR_PAGES too. It doesn't matter unless you run in to a > !early_valid_pfn() in the middle of a MAX_ORDER area, I guess. > > I think this would work, plus be a bit smaller: > > pfn = ALIGN(pfn + 1, MAX_ORDER_NR_PAGES) - 1; > Dave, I see your point about "rounding-up". But, I favor the way Mel suggested it. It more clearly shows the intent, which is to move up by MAX_ORDER_NR_PAGES. The "pfn+1" may suggest that there is some significance to the next pfn, but there is not. I find Mel's way easier to understand. Mike Y