Re: [PATCH 18/22] lmb: Add__free/__reserve/__clear_lmb_reserved_region_array()

From: Benjamin Herrenschmidt
Date: Mon May 10 2010 - 02:36:43 EST


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 <yinghai@xxxxxxxxxx>
> ---
> 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
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/