From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754169Ab2GNWiS (ORCPT ); Sat, 14 Jul 2012 18:38:18 -0400 Received: from perches-mx.perches.com ([206.117.179.246]:58646 "EHLO labridge.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752229Ab2GNWiK (ORCPT ); Sat, 14 Jul 2012 18:38:10 -0400 Message-ID: <1342305489.8377.47.camel@joe2Laptop> Subject: Re: [PATCH 2/3] x86/mm: Simplify for-loop in free_init_pages() From: Joe Perches To: Pekka Enberg Cc: Yinghai Lu , mingo@kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, Tejun Heo Date: Sat, 14 Jul 2012 15:38:09 -0700 In-Reply-To: References: <1342273267-2108-1-git-send-email-penberg@kernel.org> <1342273267-2108-2-git-send-email-penberg@kernel.org> <1342298616.8377.37.camel@joe2Laptop> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.2- Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 2012-07-14 at 23:48 +0300, Pekka Enberg wrote: > On Sat, Jul 14, 2012 at 11:43 PM, Joe Perches wrote: > > On Sat, 2012-07-14 at 13:36 -0700, Yinghai Lu wrote: > >> On Sat, Jul 14, 2012 at 6:41 AM, Pekka Enberg wrote: > >> > As a cleanup, move initial "addr" assignment to the for-loop construct > >> > in free_init_pages(). > > > > Not really a good idea. > > Of course it's a good idea. The current code flow makes no sense. > > >> > diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c > > [] > >> > @@ -349,8 +349,6 @@ void free_init_pages(char *what, unsigned long begin, unsigned long end) > >> > if (begin >= end) > >> > return; > >> > > >> > - addr = begin; > >> > - > >> > /* > >> > * If debugging page accesses then do not free this memory but > >> > * mark them not present - any buggy init-section access will > >> > @@ -371,7 +369,7 @@ void free_init_pages(char *what, unsigned long begin, unsigned long end) > >> > > >> > printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10); > >> > > >> > - for (; addr < end; addr += PAGE_SIZE) { > >> > + for (addr = begin; addr < end; addr += PAGE_SIZE) { > >> > ClearPageReserved(virt_to_page(addr)); > >> > init_page_count(virt_to_page(addr)); > >> > memset((void *)addr, POISON_FREE_INITMEM, PAGE_SIZE); > >> > -- > > > > Now there's an unused variable warning when CONFIG_DEBUG_PAGEALLOC > > Sure, that needs fixing. It'd probably be simplest to introduce a > __free_init_pages() helper. > Perhaps: arch/x86/mm/init.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index bc4e9d8..1023484 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -330,7 +330,6 @@ int devmem_is_allowed(unsigned long pagenr) void free_init_pages(char *what, unsigned long begin, unsigned long end) { - unsigned long addr; unsigned long begin_aligned, end_aligned; /* Make sure boundaries are page aligned */ @@ -345,8 +344,6 @@ void free_init_pages(char *what, unsigned long begin, unsigned long end) if (begin >= end) return; - addr = begin; - /* * If debugging page accesses then do not free this memory but * mark them not present - any buggy init-section access will @@ -367,12 +364,13 @@ void free_init_pages(char *what, unsigned long begin, unsigned long end) printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10); - for (; addr < end; addr += PAGE_SIZE) { - ClearPageReserved(virt_to_page(addr)); - init_page_count(virt_to_page(addr)); - memset((void *)addr, POISON_FREE_INITMEM, PAGE_SIZE); - free_page(addr); + while (begin < end) { + ClearPageReserved(virt_to_page(begin)); + init_page_count(virt_to_page(begin)); + memset((void *)begin, POISON_FREE_INITMEM, PAGE_SIZE); + free_page(begin); totalram_pages++; + begin += PAGE_SIZE; } #endif }