From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Subject: Re: [PATCH 18/22] lmb: Add __free/__reserve/__clear_lmb_reserved_region_array() Date: Mon, 10 May 2010 16:18:10 +1000 Message-ID: <1273472290.23699.35.camel@pasglop> References: <1273331860-14325-1-git-send-email-yinghai@kernel.org> <1273331860-14325-19-git-send-email-yinghai@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: Received: from gate.crashing.org ([63.228.1.57]:46398 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752890Ab0EJGge (ORCPT ); Mon, 10 May 2010 02:36:34 -0400 In-Reply-To: <1273331860-14325-19-git-send-email-yinghai@kernel.org> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Yinghai Lu Cc: Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Andrew Morton , David Miller , Linus Torvalds , Johannes Weiner , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org On Sat, 2010-05-08 at 08:17 -0700, Yinghai Lu wrote: > Seperate those three functions and could be shared by related callers. You just created those functions, so why don't you create them the "right way" in the first place ? I still have issues with that anyways, but we'll come to that one thing at a time. Cheers, Ben. > Signed-off-by: Yinghai Lu > --- > mm/lmb.c | 63 +++++++++++++++++++++++++++++++++++-------------------------- > 1 files changed, 36 insertions(+), 27 deletions(-) > > diff --git a/mm/lmb.c b/mm/lmb.c > index c2c6bff..20befd3 100644 > --- a/mm/lmb.c > +++ b/mm/lmb.c > @@ -684,16 +684,41 @@ static __init struct range *find_range_array(int count) > } > > #ifdef CONFIG_NO_BOOTMEM > -static void __init subtract_lmb_reserved(struct range *range, int az) > +static void __init __free_lmb_reserved_region_array(void) > { > - int i, count; > - u64 final_start, final_end; > - > #ifdef ARCH_DISCARD_LMB > /* Take out region array itself at first*/ > if (lmb.reserved.region != lmb_reserved_region) > lmb_free(__pa(lmb.reserved.region), sizeof(struct lmb_property) * lmb.reserved.nr_regions); > #endif > +} > +static void __init __reserve_lmb_reserved_region_array(void) > +{ > +#ifdef ARCH_DISCARD_LMB > + /* Put region array back ? */ > + if (lmb.reserved.region != lmb_reserved_region) > + lmb_reserve(__pa(lmb.reserved.region), sizeof(struct lmb_property) * lmb.reserved.nr_regions); > +#endif > +} > + > +static void __init __clear_lmb_reserved_region_array(void) > +{ > +#ifdef ARCH_DISCARD_LMB > + memset(&lmb.reserved.region[0], 0, sizeof(struct lmb_property) * lmb.reserved.nr_regions); > + lmb.reserved.region = NULL; > + lmb.reserved.nr_regions = 0; > + lmb.reserved.cnt = 0; > +#endif > +} > + > +static void __init subtract_lmb_reserved(struct range *range, int az) > +{ > + int i, count; > + u64 final_start, final_end; > + > + /* Take out region array itself at first*/ > + __free_lmb_reserved_region_array(); > + > count = lmb.reserved.cnt; > > if (lmb_debug) > @@ -709,11 +734,9 @@ static void __init subtract_lmb_reserved(struct range *range, int az) > continue; > subtract_range(range, az, final_start, final_end); > } > -#ifdef ARCH_DISCARD_LMB > + > /* Put region array back ? */ > - if (lmb.reserved.region != lmb_reserved_region) > - lmb_reserve(__pa(lmb.reserved.region), sizeof(struct lmb_property) * lmb.reserved.nr_regions); > -#endif > + __reserve_lmb_reserved_region_array(); > } > > int __init get_free_all_memory_range(struct range **rangep, int nodeid) > @@ -738,15 +761,9 @@ int __init get_free_all_memory_range(struct range **rangep, int nodeid) > subtract_lmb_reserved(range, count); > nr_range = clean_sort_range(range, count); > > -#ifdef ARCH_DISCARD_LMB > /* Need to clear it ? */ > - if (nodeid == MAX_NUMNODES) { > - memset(&lmb.reserved.region[0], 0, sizeof(struct lmb_property) * lmb.reserved.nr_regions); > - lmb.reserved.region = NULL; > - lmb.reserved.nr_regions = 0; > - lmb.reserved.cnt = 0; > - } > -#endif > + if (nodeid == MAX_NUMNODES) > + __clear_lmb_reserved_region_array(); > > *rangep = range; > return nr_range; > @@ -757,11 +774,8 @@ void __init lmb_to_bootmem(u64 start, u64 end) > int i, count; > u64 final_start, final_end; > > -#ifdef ARCH_DISCARD_LMB > /* Take out region array itself */ > - if (lmb.reserved.region != lmb_reserved_region) > - lmb_free(__pa(lmb.reserved.region), sizeof(struct lmb_property) * lmb.reserved.nr_regions); > -#endif > + __free_lmb_reserved_region_array(); > > count = lmb.reserved.cnt; > if (lmb_debug) > @@ -782,13 +796,8 @@ void __init lmb_to_bootmem(u64 start, u64 end) > reserve_bootmem_generic(final_start, final_end - final_start, BOOTMEM_DEFAULT); > } > > -#ifdef ARCH_DISCARD_LMB > - /* Clear them to avoid misusing ? */ > - memset(&lmb.reserved.region[0], 0, sizeof(struct lmb_property) * lmb.reserved.nr_regions); > - lmb.reserved.region = NULL; > - lmb.reserved.nr_regions = 0; > - lmb.reserved.cnt = 0; > -#endif > + /* Assume lmb_to_bootmem is only called one time */ > + __clear_lmb_reserved_region_array(); > } > #endif >