Re: [PATCH] mm: setup pageblock_order before it's used by sparse

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

On Sat, Jun 30, 2012 at 2:07 AM, Jiang Liu <jiang.liu@huawei.com> wrote:
> From: Xishi Qiu <qiuxishi@huawei.com>
>
> On architectures with CONFIG_HUGETLB_PAGE_SIZE_VARIABLE set, such as Itanium,
> pageblock_order is a variable with default value of 0. It's set to the right
> value by set_pageblock_order() in function free_area_init_core().
>
> But pageblock_order may be used by sparse_init() before free_area_init_core()
> is called along path:
> sparse_init()
>    ->sparse_early_usemaps_alloc_node()
>        ->usemap_size()
>            ->SECTION_BLOCKFLAGS_BITS
>                ->((1UL << (PFN_SECTION_SHIFT - pageblock_order)) *
> NR_PAGEBLOCK_BITS)
>
> The uninitialized pageblock_size will cause memory wasting because usemap_size()
> returns a much bigger value then it's really needed.
>
> For example, on an Itanium platform,
> sparse_init() pageblock_order=0 usemap_size=24576
> free_area_init_core() before pageblock_order=0, usemap_size=24576
> free_area_init_core() after pageblock_order=12, usemap_size=8
>
> That means 24K memory has been wasted for each section, so fix it by calling
> set_pageblock_order() from sparse_init().
>

can you check attached patch?

That will kill more lines code instead.

Thanks

Yinghai

[-- Attachment #2: kill_set_pageblock_order.patch --]
[-- Type: application/octet-stream, Size: 2426 bytes --]

Subject: [PATCH] mm: set pageblock_order in compiling time

That is initial setting, and could be override by command line.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 mm/page_alloc.c |   45 ++++++---------------------------------------
 1 file changed, 6 insertions(+), 39 deletions(-)

Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c
+++ linux-2.6/mm/page_alloc.c
@@ -147,7 +147,12 @@ bool pm_suspended_storage(void)
 #endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
-int pageblock_order __read_mostly;
+/*
+ * Assume the largest contiguous order of interest is a huge page.
+ * This value may be variable depending on boot parameters on IA64 and
+ * powerpc.
+ */
+int pageblock_order = ((HPAGE_SHIT > PAGE_SHIFT) ? HUGETLB_PAGE_ORDER : (MAX_ORDER - 1)) __read_mostly;
 #endif
 
 static void __free_pages_ok(struct page *page, unsigned int order);
@@ -4298,43 +4303,6 @@ static inline void setup_usemap(struct p
 				struct zone *zone, unsigned long zonesize) {}
 #endif /* CONFIG_SPARSEMEM */
 
-#ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
-
-/* Initialise the number of pages represented by NR_PAGEBLOCK_BITS */
-static inline void __init set_pageblock_order(void)
-{
-	unsigned int order;
-
-	/* Check that pageblock_nr_pages has not already been setup */
-	if (pageblock_order)
-		return;
-
-	if (HPAGE_SHIFT > PAGE_SHIFT)
-		order = HUGETLB_PAGE_ORDER;
-	else
-		order = MAX_ORDER - 1;
-
-	/*
-	 * Assume the largest contiguous order of interest is a huge page.
-	 * This value may be variable depending on boot parameters on IA64 and
-	 * powerpc.
-	 */
-	pageblock_order = order;
-}
-#else /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
-
-/*
- * When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not set, set_pageblock_order()
- * is unused as pageblock_order is set at compile-time. See
- * include/linux/pageblock-flags.h for the values of pageblock_order based on
- * the kernel config
- */
-static inline void set_pageblock_order(void)
-{
-}
-
-#endif /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
-
 /*
  * Set up the zone data structures:
  *   - mark all pages reserved
@@ -4413,7 +4381,6 @@ static void __paginginit free_area_init_
 		if (!size)
 			continue;
 
-		set_pageblock_order();
 		setup_usemap(pgdat, zone, size);
 		ret = init_currently_empty_zone(zone, zone_start_pfn,
 						size, MEMMAP_EARLY);

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Jiang Liu]

Hi Yinghai,
	The patch fails compilation as below:
mm/page_alloc.c:151: error: initializer element is not constant
mm/page_alloc.c:151: error: expected �,� or �;� before �__attribute__�

On IA64, HUGETLB_PAGE_ORDER has dependency on variable hpage_shift.
# define HUGETLB_PAGE_ORDER        (HPAGE_SHIFT - PAGE_SHIFT)
# define HPAGE_SHIFT               hpage_shift

And hpage_shift could be changed by early parameter "hugepagesz".
So seems will still need to keep function set_pageblock_order().

Thanks!
Gerry

On 2012-7-1 4:15, Yinghai Lu wrote:
> On Sat, Jun 30, 2012 at 2:07 AM, Jiang Liu <jiang.liu@huawei.com> wrote:
>> From: Xishi Qiu <qiuxishi@huawei.com>
>>
>> On architectures with CONFIG_HUGETLB_PAGE_SIZE_VARIABLE set, such as Itanium,
>> pageblock_order is a variable with default value of 0. It's set to the right
>> value by set_pageblock_order() in function free_area_init_core().
>>
>> But pageblock_order may be used by sparse_init() before free_area_init_core()
>> is called along path:
>> sparse_init()
>>    ->sparse_early_usemaps_alloc_node()
>>        ->usemap_size()
>>            ->SECTION_BLOCKFLAGS_BITS
>>                ->((1UL << (PFN_SECTION_SHIFT - pageblock_order)) *
>> NR_PAGEBLOCK_BITS)
>>
>> The uninitialized pageblock_size will cause memory wasting because usemap_size()
>> returns a much bigger value then it's really needed.
>>
>> For example, on an Itanium platform,
>> sparse_init() pageblock_order=0 usemap_size=24576
>> free_area_init_core() before pageblock_order=0, usemap_size=24576
>> free_area_init_core() after pageblock_order=12, usemap_size=8
>>
>> That means 24K memory has been wasted for each section, so fix it by calling
>> set_pageblock_order() from sparse_init().
>>
> 
> can you check attached patch?
> 
> That will kill more lines code instead.
> 
> Thanks
> 
> Yinghai

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

On Sun, Jul 1, 2012 at 7:01 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
> Hi Yinghai,
>         The patch fails compilation as below:
> mm/page_alloc.c:151: error: initializer element is not constant
> mm/page_alloc.c:151: error: expected ‘,’ or ‘;’ before ‘__attribute__’
>
> On IA64, HUGETLB_PAGE_ORDER has dependency on variable hpage_shift.
> # define HUGETLB_PAGE_ORDER        (HPAGE_SHIFT - PAGE_SHIFT)
> # define HPAGE_SHIFT               hpage_shift
>
> And hpage_shift could be changed by early parameter "hugepagesz".
> So seems will still need to keep function set_pageblock_order().

ah,  then use use _DEFAULT instead and later could update that in earlyparam.

So attached -v2 should  work.

Thanks

Yinghai

[-- Attachment #2: kill_set_pageblock_order_v2.patch --]
[-- Type: application/octet-stream, Size: 3159 bytes --]

Subject: [PATCH] mm: set pageblock_order in compiling time

That is initial setting, and could be override by command line.

-v2: use HPAGE_SHIFT_DEFAULT by default and set that again when hpage_shift
     get updated again.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/ia64/mm/hugetlbpage.c |    4 ++++
 mm/page_alloc.c            |   45 ++++++---------------------------------------
 2 files changed, 10 insertions(+), 39 deletions(-)

Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c
+++ linux-2.6/mm/page_alloc.c
@@ -147,7 +147,12 @@ bool pm_suspended_storage(void)
 #endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
-int pageblock_order __read_mostly;
+/*
+ * Assume the largest contiguous order of interest is a huge page.
+ * This value may be variable depending on boot parameters on IA64 and
+ * powerpc.
+ */
+int pageblock_order = ((HPAGE_SHIFT_DEFAULT > PAGE_SHIFT) ? (HPAGE_SHIFT_DEFAULT - PAGE_SHIFT) : (MAX_ORDER - 1)) __read_mostly;
 #endif
 
 static void __free_pages_ok(struct page *page, unsigned int order);
@@ -4298,43 +4303,6 @@ static inline void setup_usemap(struct p
 				struct zone *zone, unsigned long zonesize) {}
 #endif /* CONFIG_SPARSEMEM */
 
-#ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
-
-/* Initialise the number of pages represented by NR_PAGEBLOCK_BITS */
-static inline void __init set_pageblock_order(void)
-{
-	unsigned int order;
-
-	/* Check that pageblock_nr_pages has not already been setup */
-	if (pageblock_order)
-		return;
-
-	if (HPAGE_SHIFT > PAGE_SHIFT)
-		order = HUGETLB_PAGE_ORDER;
-	else
-		order = MAX_ORDER - 1;
-
-	/*
-	 * Assume the largest contiguous order of interest is a huge page.
-	 * This value may be variable depending on boot parameters on IA64 and
-	 * powerpc.
-	 */
-	pageblock_order = order;
-}
-#else /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
-
-/*
- * When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not set, set_pageblock_order()
- * is unused as pageblock_order is set at compile-time. See
- * include/linux/pageblock-flags.h for the values of pageblock_order based on
- * the kernel config
- */
-static inline void set_pageblock_order(void)
-{
-}
-
-#endif /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
-
 /*
  * Set up the zone data structures:
  *   - mark all pages reserved
@@ -4413,7 +4381,6 @@ static void __paginginit free_area_init_
 		if (!size)
 			continue;
 
-		set_pageblock_order();
 		setup_usemap(pgdat, zone, size);
 		ret = init_currently_empty_zone(zone, zone_start_pfn,
 						size, MEMMAP_EARLY);
Index: linux-2.6/arch/ia64/mm/hugetlbpage.c
===================================================================
--- linux-2.6.orig/arch/ia64/mm/hugetlbpage.c
+++ linux-2.6/arch/ia64/mm/hugetlbpage.c
@@ -202,6 +202,10 @@ static int __init hugetlb_setup_sz(char
 	 * override here with new page shift.
 	 */
 	ia64_set_rr(HPAGE_REGION_BASE, hpage_shift << 2);
+
+	/* update pageblock_order accordingly */
+	pageblock_order = ((HPAGE_SHIFT > PAGE_SHIFT) ? HUGETLB_PAGE_ORDER : (MAX_ORDER - 1))
+
 	return 0;
 }
 early_param("hugepagesz", hugetlb_setup_sz);

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Jiang Liu]

On 2012-7-3 4:43, Yinghai Lu wrote:
> On Sun, Jul 1, 2012 at 7:01 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
>> Hi Yinghai,
>>         The patch fails compilation as below:
>> mm/page_alloc.c:151: error: initializer element is not constant
>> mm/page_alloc.c:151: error: expected �,� or �;� before �__attribute__�
>>
>> On IA64, HUGETLB_PAGE_ORDER has dependency on variable hpage_shift.
>> # define HUGETLB_PAGE_ORDER        (HPAGE_SHIFT - PAGE_SHIFT)
>> # define HPAGE_SHIFT               hpage_shift
>>
>> And hpage_shift could be changed by early parameter "hugepagesz".
>> So seems will still need to keep function set_pageblock_order().
> 
> ah,  then use use _DEFAULT instead and later could update that in earlyparam.
> 
> So attached -v2 should  work.
Hi Yinghai,

I'm afraid the v2 will break powerpc. Currently only IA64 and PowerPC
supports variable hugetlb size. 

HPAGE_SHIFT is a variable default to 0 on powerpc. But seems PowerPC 
is doing something wrong here, according to it's mm initialization 
sequence as below:
start_kernel()
	setup_arch()
		paging_init()
			free_area_init_node()
				set_pageblock_order()
					refer to HPAGE_SHIFT (still 0)
	init_rest()	
		do_initcalls()
			hugetlbpage_init()
				setup HPAGE_SHIFT
That means pageblock_order is always set to "MAX_ORDER - 1", not sure
whether this is intended. And it has the same issue as IA64 of wasting
memory if CONFIG_SPARSE is enabled.

So it would be better to keep function set_pageblock_order(), it will
fix the memory wasting on both IA64 and PowerPC.

Thanks!
Gerry

> 
> Thanks
> 
> Yinghai

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Yinghai Lu]

On Mon, Jul 2, 2012 at 7:54 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
> On 2012-7-3 4:43, Yinghai Lu wrote:
>> On Sun, Jul 1, 2012 at 7:01 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
>>> Hi Yinghai,
>>>         The patch fails compilation as below:
>>> mm/page_alloc.c:151: error: initializer element is not constant
>>> mm/page_alloc.c:151: error: expected �,� or �;� before �__attribute__�
>>>
>>> On IA64, HUGETLB_PAGE_ORDER has dependency on variable hpage_shift.
>>> # define HUGETLB_PAGE_ORDER        (HPAGE_SHIFT - PAGE_SHIFT)
>>> # define HPAGE_SHIFT               hpage_shift
>>>
>>> And hpage_shift could be changed by early parameter "hugepagesz".
>>> So seems will still need to keep function set_pageblock_order().
>>
>> ah,  then use use _DEFAULT instead and later could update that in earlyparam.
>>
>> So attached -v2 should  work.
> Hi Yinghai,
>
> I'm afraid the v2 will break powerpc. Currently only IA64 and PowerPC
> supports variable hugetlb size.
>
> HPAGE_SHIFT is a variable default to 0 on powerpc. But seems PowerPC
> is doing something wrong here, according to it's mm initialization
> sequence as below:
> start_kernel()
>         setup_arch()
>                 paging_init()
>                         free_area_init_node()
>                                 set_pageblock_order()
>                                         refer to HPAGE_SHIFT (still 0)
>         init_rest()
>                 do_initcalls()
>                         hugetlbpage_init()
>                                 setup HPAGE_SHIFT
> That means pageblock_order is always set to "MAX_ORDER - 1", not sure
> whether this is intended. And it has the same issue as IA64 of wasting
> memory if CONFIG_SPARSE is enabled.

adding BenH, need to know if it is powerpc intended.

>
> So it would be better to keep function set_pageblock_order(), it will
> fix the memory wasting on both IA64 and PowerPC.

Should setup pageblock_order as early as possible to avoid confusing.

Thanks

Yinghai

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Jiang Liu]

>> Hi Yinghai,
>>
>> I'm afraid the v2 will break powerpc. Currently only IA64 and PowerPC
>> supports variable hugetlb size.
>>
>> HPAGE_SHIFT is a variable default to 0 on powerpc. But seems PowerPC
>> is doing something wrong here, according to it's mm initialization
>> sequence as below:
>> start_kernel()
>>         setup_arch()
>>                 paging_init()
>>                         free_area_init_node()
>>                                 set_pageblock_order()
>>                                         refer to HPAGE_SHIFT (still 0)
>>         init_rest()
>>                 do_initcalls()
>>                         hugetlbpage_init()
>>                                 setup HPAGE_SHIFT
>> That means pageblock_order is always set to "MAX_ORDER - 1", not sure
>> whether this is intended. And it has the same issue as IA64 of wasting
>> memory if CONFIG_SPARSE is enabled.
> 
> adding BenH, need to know if it is powerpc intended.
> 
>>
>> So it would be better to keep function set_pageblock_order(), it will
>> fix the memory wasting on both IA64 and PowerPC.
> 
> Should setup pageblock_order as early as possible to avoid confusing.
OK, waiting response from PPC. If we could find some ways to set HPAGE_SIZE
early on PPC too, we can setup pageblock_order in arch instead of page_alloc.c
as early as possible.

Thanks!
Gerry

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Greg KH]

On Sat, Jun 30, 2012 at 05:07:54PM +0800, Jiang Liu wrote:
> From: Xishi Qiu <qiuxishi@huawei.com>
> 
> On architectures with CONFIG_HUGETLB_PAGE_SIZE_VARIABLE set, such as Itanium,
> pageblock_order is a variable with default value of 0. It's set to the right
> value by set_pageblock_order() in function free_area_init_core().
> 
> But pageblock_order may be used by sparse_init() before free_area_init_core()
> is called along path:
> sparse_init()
>     ->sparse_early_usemaps_alloc_node()
> 	->usemap_size()
> 	    ->SECTION_BLOCKFLAGS_BITS
> 		->((1UL << (PFN_SECTION_SHIFT - pageblock_order)) *
> NR_PAGEBLOCK_BITS)
> 
> The uninitialized pageblock_size will cause memory wasting because usemap_size()
> returns a much bigger value then it's really needed.
> 
> For example, on an Itanium platform,
> sparse_init() pageblock_order=0 usemap_size=24576
> free_area_init_core() before pageblock_order=0, usemap_size=24576
> free_area_init_core() after pageblock_order=12, usemap_size=8
> 
> That means 24K memory has been wasted for each section, so fix it by calling
> set_pageblock_order() from sparse_init().
> 
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> ---
>  mm/internal.h   |    2 ++
>  mm/page_alloc.c |    4 ++--
>  mm/sparse.c     |    3 +++
>  3 files changed, 7 insertions(+), 2 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Mel Gorman]

On Sat, Jun 30, 2012 at 05:07:54PM +0800, Jiang Liu wrote:
> From: Xishi Qiu <qiuxishi@huawei.com>
> 
> On architectures with CONFIG_HUGETLB_PAGE_SIZE_VARIABLE set, such as Itanium,
> pageblock_order is a variable with default value of 0. It's set to the right
> value by set_pageblock_order() in function free_area_init_core().
> 
> But pageblock_order may be used by sparse_init() before free_area_init_core()
> is called along path:
> sparse_init()
>     ->sparse_early_usemaps_alloc_node()
> 	->usemap_size()
> 	    ->SECTION_BLOCKFLAGS_BITS
> 		->((1UL << (PFN_SECTION_SHIFT - pageblock_order)) *
> NR_PAGEBLOCK_BITS)
> 
> The uninitialized pageblock_size will cause memory wasting because usemap_size()
> returns a much bigger value then it's really needed.
> 
> For example, on an Itanium platform,
> sparse_init() pageblock_order=0 usemap_size=24576
> free_area_init_core() before pageblock_order=0, usemap_size=24576
> free_area_init_core() after pageblock_order=12, usemap_size=8
> 
> That means 24K memory has been wasted for each section, so fix it by calling
> set_pageblock_order() from sparse_init().
> 
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> Signed-off-by: Jiang Liu <liuj97@gmail.com>

Looks all right.

Acked-by: Mel Gorman <mgorman@suse.de>

This should be considered a stable candidate. Add

Cc: stable <stable@vger.kernel.org>

above your Signed-off-by and it'll get picked up if the patch is merged
to mainline.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Andrew Morton]

On Sat, 30 Jun 2012 17:07:54 +0800
Jiang Liu <jiang.liu@huawei.com> wrote:

> From: Xishi Qiu <qiuxishi@huawei.com>
> 
> On architectures with CONFIG_HUGETLB_PAGE_SIZE_VARIABLE set, such as Itanium,
> pageblock_order is a variable with default value of 0. It's set to the right
> value by set_pageblock_order() in function free_area_init_core().
> 
> But pageblock_order may be used by sparse_init() before free_area_init_core()
> is called along path:
> sparse_init()
>     ->sparse_early_usemaps_alloc_node()
> 	->usemap_size()
> 	    ->SECTION_BLOCKFLAGS_BITS
> 		->((1UL << (PFN_SECTION_SHIFT - pageblock_order)) *
> NR_PAGEBLOCK_BITS)
> 
> The uninitialized pageblock_size will cause memory wasting because usemap_size()
> returns a much bigger value then it's really needed.
> 
> For example, on an Itanium platform,
> sparse_init() pageblock_order=0 usemap_size=24576
> free_area_init_core() before pageblock_order=0, usemap_size=24576
> free_area_init_core() after pageblock_order=12, usemap_size=8
> 
> That means 24K memory has been wasted for each section, so fix it by calling
> set_pageblock_order() from sparse_init().
> 
> ...
>
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -485,6 +485,9 @@ void __init sparse_init(void)
>  	struct page **map_map;
>  #endif
>  
> +	/* Setup pageblock_order for HUGETLB_PAGE_SIZE_VARIABLE */
> +	set_pageblock_order();
> +
>  	/*
>  	 * map is using big page (aka 2M in x86 64 bit)
>  	 * usemap is less one page (aka 24 bytes)

It's a bit ugly calling set_pageblock_order() from both sparse_init()
and from free_area_init_core().  Can we find a single place from which
to call it?  It looks like here:

--- a/init/main.c~a
+++ a/init/main.c
@@ -514,6 +514,7 @@ asmlinkage void __init start_kernel(void
 		   __stop___param - __start___param,
 		   -1, -1, &unknown_bootoption);
 
+	set_pageblock_order();
 	jump_label_init();
 
 	/*

would do the trick?

(free_area_init_core is __paging_init and set_pageblock_order() is
__init.  I'm too lazy to work out if that's wrong)

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Jiang Liu]

> It's a bit ugly calling set_pageblock_order() from both sparse_init()
> and from free_area_init_core().  Can we find a single place from which
> to call it?  It looks like here:
> 
> --- a/init/main.c~a
> +++ a/init/main.c
> @@ -514,6 +514,7 @@ asmlinkage void __init start_kernel(void
>  		   __stop___param - __start___param,
>  		   -1, -1, &unknown_bootoption);
>  
> +	set_pageblock_order();
>  	jump_label_init();
>  
>  	/*
> 
> would do the trick?
> 
> (free_area_init_core is __paging_init and set_pageblock_order() is
> __init.  I'm too lazy to work out if that's wrong)

Hi Andrew,
	Thanks for you comments. Yes, this's an issue. 
And we are trying to find a way to setup  pageorder_block as 
early as possible. Yinghai has suggested a good way for IA64,
but we still need help from PPC experts because PPC has the 
same issue and I'm not familiar with PPC architecture. 
We will submit another patch once we find an acceptable
solution here.
	Thanks!
	Gerry

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Mel Gorman]

On Wed, Jul 04, 2012 at 09:40:30AM +0800, Jiang Liu wrote:
> > It's a bit ugly calling set_pageblock_order() from both sparse_init()
> > and from free_area_init_core().  Can we find a single place from which
> > to call it?  It looks like here:
> > 
> > --- a/init/main.c~a
> > +++ a/init/main.c
> > @@ -514,6 +514,7 @@ asmlinkage void __init start_kernel(void
> >  		   __stop___param - __start___param,
> >  		   -1, -1, &unknown_bootoption);
> >  
> > +	set_pageblock_order();
> >  	jump_label_init();
> >  
> >  	/*
> > 
> > would do the trick?
> > 
> > (free_area_init_core is __paging_init and set_pageblock_order() is
> > __init.  I'm too lazy to work out if that's wrong)
> 
> Hi Andrew,
> 	Thanks for you comments. Yes, this's an issue. 
> And we are trying to find a way to setup  pageorder_block as 
> early as possible. Yinghai has suggested a good way for IA64,
> but we still need help from PPC experts because PPC has the 
> same issue and I'm not familiar with PPC architecture. 
> We will submit another patch once we find an acceptable
> solution here.

I think it's overkill to try and do this on a per-architecture basis unless
you are aware of a case where the per-architecture code cares about the
value of pageblock_order. I find it implausible that the architecture
needs to know the value very early in boot as pageblock_order is part of
the arch-independent memory model. Andrew's suggestion seems reasonable
to me once the section mess is figured out.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Yinghai Lu]

On Wed, Jul 4, 2012 at 2:20 AM, Mel Gorman <mgorman@suse.de> wrote:
> On Wed, Jul 04, 2012 at 09:40:30AM +0800, Jiang Liu wrote:
>> > It's a bit ugly calling set_pageblock_order() from both sparse_init()
>> > and from free_area_init_core().  Can we find a single place from which
>> > to call it?  It looks like here:
>> >
>> > --- a/init/main.c~a
>> > +++ a/init/main.c
>> > @@ -514,6 +514,7 @@ asmlinkage void __init start_kernel(void
>> >                __stop___param - __start___param,
>> >                -1, -1, &unknown_bootoption);
>> >
>> > +   set_pageblock_order();
>> >     jump_label_init();
>> >
>> >     /*
>> >
>> > would do the trick?
>> >
>> > (free_area_init_core is __paging_init and set_pageblock_order() is
>> > __init.  I'm too lazy to work out if that's wrong)
>>
>> Hi Andrew,
>>       Thanks for you comments. Yes, this's an issue.
>> And we are trying to find a way to setup  pageorder_block as
>> early as possible. Yinghai has suggested a good way for IA64,
>> but we still need help from PPC experts because PPC has the
>> same issue and I'm not familiar with PPC architecture.
>> We will submit another patch once we find an acceptable
>> solution here.
>
> I think it's overkill to try and do this on a per-architecture basis unless
> you are aware of a case where the per-architecture code cares about the
> value of pageblock_order. I find it implausible that the architecture
> needs to know the value very early in boot as pageblock_order is part of
> the arch-independent memory model. Andrew's suggestion seems reasonable
> to me once the section mess is figured out.

cma, dma_continugous_reserve is referring pageblock_order very early too.
just after init_memory_mapping() for x86's setup_arch.

so set pageblock_order early looks like my -v2 patch is right way.

current question: need to powerpc guys to check who to set that early.

Thanks

Yinghai

Re: [PATCH] mm: setup pageblock_order before it's used by sparse

[Benjamin Herrenschmidt]

On Thu, 2012-07-05 at 18:00 -0700, Yinghai Lu wrote:
> cma, dma_continugous_reserve is referring pageblock_order very early
> too.
> just after init_memory_mapping() for x86's setup_arch.
> 
> so set pageblock_order early looks like my -v2 patch is right way.
> 
> current question: need to powerpc guys to check who to set that early.

I missed the beginning of that discussion, I'll try to dig a bit,
might take me til next week though as I'm about to be off for
the week-end.

Cheers,
Ben.