Re: inux-next: Tree for Apr 27 (uml + mm/memcontrol.c)

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 27 Apr 2012 11:44:24 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> On Fri, 27 Apr 2012, Randy Dunlap wrote:
> 
> > On 04/26/2012 11:11 PM, Stephen Rothwell wrote:
> > 
> > > Hi all,
> > > 
> > > Changes since 20120424:
> > 
> > 
> > 
> > uml on x86_64 (defconfig):
> > 
> > mm/memcontrol.c:256:30: error: 'HUGE_MAX_HSTATE' undeclared here (not in a function)
> > 
> 
> This is because of "memcg: add HugeTLB extension" from -mm.
> 
> This patch is legal in C99, I wonder if Andrew would let us get away with 
> something like this so there's no a dozen #ifdefs in mm/memcontrol.c?
> ---
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -39,13 +39,13 @@
>  #ifndef __ASSEMBLY__
>  #ifdef CONFIG_HUGETLB_PAGE
>  extern unsigned int HPAGE_SHIFT;
> +#define HUGE_MAX_HSTATE		(MMU_PAGE_COUNT-1)
>  #else
>  #define HPAGE_SHIFT PAGE_SHIFT
>  #endif
>  #define HPAGE_SIZE		((1UL) << HPAGE_SHIFT)
>  #define HPAGE_MASK		(~(HPAGE_SIZE - 1))
>  #define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
> -#define HUGE_MAX_HSTATE		(MMU_PAGE_COUNT-1)
>  #endif
>  
>  /* We do define AT_SYSINFO_EHDR but don't use the gate mechanism */
> diff --git a/arch/tile/include/asm/page.h b/arch/tile/include/asm/page.h
> --- a/arch/tile/include/asm/page.h
> +++ b/arch/tile/include/asm/page.h
> @@ -136,9 +136,8 @@ static inline __attribute_const__ int get_order(unsigned long size)
>  
>  #define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
>  
> -#define HUGE_MAX_HSTATE		6
> -
>  #ifdef CONFIG_HUGETLB_PAGE
> +#define HUGE_MAX_HSTATE		6
>  #define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>  #endif
>  
> diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
> --- a/arch/x86/include/asm/page_types.h
> +++ b/arch/x86/include/asm/page_types.h
> @@ -25,7 +25,9 @@
>  #define HPAGE_MASK		(~(HPAGE_SIZE - 1))
>  #define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
>  
> +#ifdef CONFIG_HUGETLB_PAGE
>  #define HUGE_MAX_HSTATE 2
> +#endif
>  
>  #define PAGE_OFFSET		((unsigned long)__PAGE_OFFSET)
>  
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -324,6 +324,7 @@ static inline int hstate_index(struct hstate *h)
>  
>  #else
>  struct hstate {};
> +#define HUGE_MAX_HSTATE 0
>  #define alloc_huge_page_node(h, nid) NULL
>  #define alloc_bootmem_huge_page(h) NULL
>  #define hstate_file(f) NULL

Seems reasonable.  But the CONFIG_HUGETLB_PAGE=y,
CONFIG_MEM_RES_CTLR_HUGETLB=n combination will cause unneeded code
generation and space consumption in memcontrol.c.

I wonder if we can additionally do, within memcontrol.c:

/*
 * Nice comment goes here
 */
#ifdef CONFIG_MEM_RES_CTLR_HUGETLB
#define HUGE_MAX_HSTATE_FOO HUGE_MAX_HSTATE
#else
#define HUGE_MAX_HSTATE_FOO 0
#endif

and s/HUGE_MAX_HSTATE/HUGE_MAX_HSTATE_FOO/ in that file.