2012-05-03 17:15:53

by rajman mekaco

[permalink] [raw]
Subject: [PATCH 1/1] page_alloc.c: remove argument to pageblock_default_order

When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined, then
pageblock_default_order has an argument to it.

However, free_area_init_core will call it without any argument
anyway.

Remove the argument to pageblock_default_order when
CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined.

Signed-off-by: rajman mekaco <[email protected]>
---
mm/page_alloc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a712fb9..4b95412 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4274,7 +4274,7 @@ static inline void __init set_pageblock_order(unsigned int order)
* at compile-time. See include/linux/pageblock-flags.h for the values of
* pageblock_order based on the kernel config
*/
-static inline int pageblock_default_order(unsigned int order)
+static inline int pageblock_default_order(void)
{
return MAX_ORDER-1;
}
--
1.7.5.4


2012-05-03 23:37:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/1] page_alloc.c: remove argument to pageblock_default_order

On Thu, 3 May 2012 22:45:12 +0530
rajman mekaco <[email protected]> wrote:

> When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined, then
> pageblock_default_order has an argument to it.
>
> However, free_area_init_core will call it without any argument
> anyway.
>
> Remove the argument to pageblock_default_order when
> CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined.
>
> Signed-off-by: rajman mekaco <[email protected]>
> ---
> mm/page_alloc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a712fb9..4b95412 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4274,7 +4274,7 @@ static inline void __init set_pageblock_order(unsigned int order)
> * at compile-time. See include/linux/pageblock-flags.h for the values of
> * pageblock_order based on the kernel config
> */
> -static inline int pageblock_default_order(unsigned int order)
> +static inline int pageblock_default_order(void)
> {
> return MAX_ORDER-1;
> }

Interesting. It has been that way since at least 3.1.

It didn't break the build because pageblock_default_order() is only
ever invoked by set_pageblock_order(), with:

set_pageblock_order(pageblock_default_order());

and set_pageblock_order() is a macro:

#define set_pageblock_order(x) do {} while (0)


There's yet another reason not to use macros, dammit - they hide bugs.


Mel, can you have a think about this please? Can we just kill off
pageblock_default_order() and fold its guts into
set_pageblock_order(void)? Only ia64 and powerpc can define
CONFIG_HUGETLB_PAGE_SIZE_VARIABLE.

--- a/mm/page_alloc.c~a
+++ a/mm/page_alloc.c
@@ -4300,25 +4300,24 @@ static inline void setup_usemap(struct p

#ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE

-/* Return a sensible default order for the pageblock size. */
-static inline int pageblock_default_order(void)
-{
- if (HPAGE_SHIFT > PAGE_SHIFT)
- return HUGETLB_PAGE_ORDER;
-
- return MAX_ORDER-1;
-}
-
/* Initialise the number of pages represented by NR_PAGEBLOCK_BITS */
-static inline void __init set_pageblock_order(unsigned int order)
+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
+ * This value may be variable depending on boot parameters on IA64 and
+ * powerpc.
*/
pageblock_order = order;
}
@@ -4326,15 +4325,13 @@ static inline void __init set_pageblock_

/*
* When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not set, set_pageblock_order()
- * and pageblock_default_order() are 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
+ * 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 int pageblock_default_order(unsigned int order)
+static inline void set_pageblock_order(void)
{
- return MAX_ORDER-1;
}
-#define set_pageblock_order(x) do {} while (0)

#endif /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */

@@ -4422,7 +4419,7 @@ static void __paginginit free_area_init_
if (!size)
continue;

- set_pageblock_order(pageblock_default_order());
+ set_pageblock_order();
setup_usemap(pgdat, zone, size);
ret = init_currently_empty_zone(zone, zone_start_pfn,
size, MEMMAP_EARLY);
_

2012-05-10 13:35:00

by Mel Gorman

[permalink] [raw]
Subject: Re: [PATCH 1/1] page_alloc.c: remove argument to pageblock_default_order

On Thu, May 03, 2012 at 04:37:49PM -0700, Andrew Morton wrote:
> On Thu, 3 May 2012 22:45:12 +0530
> rajman mekaco <[email protected]> wrote:
>
> > When CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined, then
> > pageblock_default_order has an argument to it.
> >
> > However, free_area_init_core will call it without any argument
> > anyway.
> >
> > Remove the argument to pageblock_default_order when
> > CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is not defined.
> >
> > Signed-off-by: rajman mekaco <[email protected]>
> > ---
> > mm/page_alloc.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index a712fb9..4b95412 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -4274,7 +4274,7 @@ static inline void __init set_pageblock_order(unsigned int order)
> > * at compile-time. See include/linux/pageblock-flags.h for the values of
> > * pageblock_order based on the kernel config
> > */
> > -static inline int pageblock_default_order(unsigned int order)
> > +static inline int pageblock_default_order(void)
> > {
> > return MAX_ORDER-1;
> > }
>
> Interesting. It has been that way since at least 3.1.
>

/me slaps self

> It didn't break the build because pageblock_default_order() is only
> ever invoked by set_pageblock_order(), with:
>
> set_pageblock_order(pageblock_default_order());
>
> and set_pageblock_order() is a macro:
>
> #define set_pageblock_order(x) do {} while (0)
>
> There's yet another reason not to use macros, dammit - they hide bugs.
>
>
> Mel, can you have a think about this please? Can we just kill off
> pageblock_default_order() and fold its guts into
> set_pageblock_order(void)? Only ia64 and powerpc can define
> CONFIG_HUGETLB_PAGE_SIZE_VARIABLE.
>

This looks reasonable to me.

--
Mel Gorman
SUSE Labs