On (15/11/07 02:39), Andrew Morton didst pronounce:
> On Thu, 15 Nov 2007 10:13:22 +0000 [EMAIL PROTECTED] (Mel Gorman) wrote:
>
> > This patch is a fix for 2.6.24.
> >
> > Ordinarily the size of a pageblock is determined at compile-time based on
> > the
> > hugepage size. On PPC64, the hugepage size is determined at runtime based on
> > what is supported by the machine. With legacy machines such as iSeries that
> > do
> > not support hugepages, HPAGE_SHIFT becomes 0. This results in
> > pageblock_order
> > being set to -PAGE_SHIFT and a crash results shortly afterwards.
> >
> > This patch sets HUGETLB_PAGE_SIZE_VARIABLE for PPC64 and adds a function
> > to select a sensible value for pageblock order by default. It checks that
> > HPAGE_SHIFT is a sensible value before using the hugepage size; if it is
> > not MAX_ORDER-1 is used.
> >
> > Credit goes to Stephen Rothwell for identifying the bug and testing
> > candidate
> > patches. Additional credit goes to Andy Whitcroft for spotting a problem
> > with respects to IA-64 before releasing. Additional credit to David Gibson
> > for testing with the libhugetlbfs test suite.
> >
> > Signed-off-by: Mel Gorman <[EMAIL PROTECTED]>
> > Tested-by: Stephen Rothwell <[EMAIL PROTECTED]>
> >
> > ---
> > arch/powerpc/Kconfig | 5 +++++
> > mm/page_alloc.c | 14 ++++++++++++--
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff -rup -X /usr/src/patchset-0.6/bin//dontdiff
> > linux-2.6.24-rc2-mm1-clean/arch/powerpc/Kconfig
> > linux-2.6.24-rc2-005_iSeries_fix/arch/powerpc/Kconfig
> > --- linux-2.6.24-rc2-mm1-clean/arch/powerpc/Kconfig 2007-11-14
> > 11:38:05.000000000 +0000
> > +++ linux-2.6.24-rc2-005_iSeries_fix/arch/powerpc/Kconfig 2007-11-14
> > 11:39:12.000000000 +0000
> > @@ -187,6 +187,11 @@ config FORCE_MAX_ZONEORDER
> > default "9" if PPC_64K_PAGES
> > default "13"
> >
> > +config HUGETLB_PAGE_SIZE_VARIABLE
> > + bool
> > + depends on HUGETLB_PAGE
> > + default y
> > +
> > config MATH_EMULATION
> > bool "Math emulation"
> > depends on 4xx || 8xx || E200 || PPC_MPC832x || E500
> > diff -rup -X /usr/src/patchset-0.6/bin//dontdiff
> > linux-2.6.24-rc2-mm1-clean/mm/page_alloc.c
> > linux-2.6.24-rc2-005_iSeries_fix/mm/page_alloc.c
> > --- linux-2.6.24-rc2-mm1-clean/mm/page_alloc.c 2007-11-14
> > 11:38:08.000000000 +0000
> > +++ linux-2.6.24-rc2-005_iSeries_fix/mm/page_alloc.c 2007-11-14
> > 13:45:19.000000000 +0000
> > @@ -3342,6 +3342,16 @@ static void inline setup_usemap(struct p
> > #endif /* CONFIG_SPARSEMEM */
> >
> > #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
> > +
> > +/* Return a sensible default order for the pageblock size. */
> > +static inline int __init 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)
> > {
> > @@ -3357,7 +3367,7 @@ static inline void __init set_pageblock_
> > }
> > #else /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
> >
> > -/* Defined this way to avoid accidently referencing HUGETLB_PAGE_ORDER */
> > +#define pageblock_default_order(x) (0)
>
> Shouldn't this have been HUGETLB_PAGE_ORDER?
>
As a #define, possibly but as a static inline - definitly not.
In this context, the define is not used because set_pageblock_order()
is a no-op when CONFIG_HUGETLB_PAGE_SIZE_VARIABLE is unset.
pageblock_default_order() is only defined for symmetry as set_pageblock_order()
is defined in both contexts. However, as a #define it might make more sense
to a casual reader to see HUGETLB_PAGE_ORDER even if it has no effect. I
can send a version of the patch that does this with a comment explaining
what is going on with set_pageblock_order() if you like.
However, in a follow-up fix, you make pageblock_default_order() a static
inline. If it tries to return HUGETLB_PAGE_ORDER, it will fail to compile
when CONFIG_HUGETLB_PAGE is not set.
Which would you prefer?
> > #define set_pageblock_order(x) do {} while (0)
> >
> > #endif /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
> > @@ -3442,7 +3452,7 @@ static void __meminit free_area_init_cor
> > if (!size)
> > continue;
> >
> > - set_pageblock_order(HUGETLB_PAGE_ORDER);
> > + set_pageblock_order(pageblock_default_order());
> > setup_usemap(pgdat, zone, size);
> > ret = init_currently_empty_zone(zone, zone_start_pfn,
> > size, MEMMAP_EARLY);
> > --
> > --
> > Mel Gorman
> > Part-time Phd Student Linux Technology Center
> > University of Limerick IBM Dublin Software Lab
>
--
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
