[2.6 patch] fix mm/mm_init.c compilation

[2.6 patch] fix mm/mm_init.c compilation

[Adrian Bunk]

This patch fixes the following compile error with gcc 3.2:

<--  snip  -->

...
  CC      mm/mm_init.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:77:1: directives may not be used inside a macro argument
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:76:47: unterminated argument list invoking macro "mminit_dprintk"
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c: In function `mminit_verify_pageflags_layout':
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: `mminit_dprintk' undeclared (first use in this function)
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: (Each undeclared identifier is reported only once
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: for each function it appears in.)
/home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: syntax error before numeric constant
make[2]: *** [mm/mm_init.o] Error 1

<--  snip  -->

Reported-by: Adrian Bunk <bunk@kernel.org>
Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

 mm/mm_init.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -63,6 +63,13 @@ void __init mminit_verify_pageflags_layout(void)
 {
 	int shift, width;
 	unsigned long or_mask, add_mask;
+	int sections_shift;
+
+#ifdef SECTIONS_SHIFT
+	sections_shift = SECTIONS_SHIFT;
+#else
+	sections_shift = 0;
+#endif
 
 	shift = 8 * sizeof(unsigned long);
 	width = shift - SECTIONS_WIDTH - NODES_WIDTH - ZONES_WIDTH;
@@ -74,11 +81,7 @@ void __init mminit_verify_pageflags_layout(void)
 		NR_PAGEFLAGS);
 	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
 		"Section %d Node %d Zone %d\n",
-#ifdef SECTIONS_SHIFT
-		SECTIONS_SHIFT,
-#else
-		0,
-#endif
+		sections_shift,
 		NODES_SHIFT,
 		ZONES_SHIFT);
 	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",

Re: [2.6 patch] fix mm/mm_init.c compilation

[Andrew Morton]

On Mon, 4 Aug 2008 21:19:15 +0300
Adrian Bunk <bunk@kernel.org> wrote:

> This patch fixes the following compile error with gcc 3.2:
> 
> <--  snip  -->
> 
> ...
>   CC      mm/mm_init.o
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:77:1: directives may not be used inside a macro argument
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:76:47: unterminated argument list invoking macro "mminit_dprintk"
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c: In function `mminit_verify_pageflags_layout':
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: `mminit_dprintk' undeclared (first use in this function)
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: (Each undeclared identifier is reported only once
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: for each function it appears in.)
> /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: syntax error before numeric constant
> make[2]: *** [mm/mm_init.o] Error 1
> 
> <--  snip  -->
> 
> 
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -63,6 +63,13 @@ void __init mminit_verify_pageflags_layout(void)
>  {
>  	int shift, width;
>  	unsigned long or_mask, add_mask;
> +	int sections_shift;
> +
> +#ifdef SECTIONS_SHIFT
> +	sections_shift = SECTIONS_SHIFT;
> +#else
> +	sections_shift = 0;
> +#endif
>  
>  	shift = 8 * sizeof(unsigned long);
>  	width = shift - SECTIONS_WIDTH - NODES_WIDTH - ZONES_WIDTH;
> @@ -74,11 +81,7 @@ void __init mminit_verify_pageflags_layout(void)
>  		NR_PAGEFLAGS);
>  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
>  		"Section %d Node %d Zone %d\n",
> -#ifdef SECTIONS_SHIFT
> -		SECTIONS_SHIFT,
> -#else
> -		0,
> -#endif
> +		sections_shift,
>  		NODES_SHIFT,
>  		ZONES_SHIFT);
>  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",

OK, ifdefs inside macro expansion aren't a great idea.

Mel, is there any erason why we shouldn't do it this way?

 include/linux/mmzone.h |    3 ++-
 mm/mm_init.c           |    4 ----

 2 files changed, 2 insertions(+), 5 deletions(-)

diff -puN include/linux/mmzone.h~a include/linux/mmzone.h
--- a/include/linux/mmzone.h~a
+++ a/include/linux/mmzone.h
@@ -831,7 +831,7 @@ static inline unsigned long early_pfn_to
 #ifdef CONFIG_SPARSEMEM
 
 /*
- * SECTION_SHIFT    		#bits space required to store a section #
+ * SECTIONS_SHIFT    		#bits space required to store a section #
  *
  * PA_SECTION_SHIFT		physical address to/from section number
  * PFN_SECTION_SHIFT		pfn to/from section number
@@ -973,6 +973,7 @@ static inline int pfn_present(unsigned l
 #define early_pfn_valid(pfn)	pfn_valid(pfn)
 void sparse_init(void);
 #else
+#define SECTIONS_SHIFT		0
 #define sparse_init()	do {} while (0)
 #define sparse_index_init(_sec, _nid)  do {} while (0)
 #endif /* CONFIG_SPARSEMEM */
diff -puN mm/mm_init.c~a mm/mm_init.c
--- a/mm/mm_init.c~a
+++ a/mm/mm_init.c
@@ -74,11 +74,7 @@ void __init mminit_verify_pageflags_layo
 		NR_PAGEFLAGS);
 	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
 		"Section %d Node %d Zone %d\n",
-#ifdef SECTIONS_SHIFT
 		SECTIONS_SHIFT,
-#else
-		0,
-#endif
 		NODES_SHIFT,
 		ZONES_SHIFT);
 	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",
_

Re: [2.6 patch] fix mm/mm_init.c compilation

[Mel Gorman]

On (04/08/08 14:05), Andrew Morton didst pronounce:
> On Mon, 4 Aug 2008 21:19:15 +0300
> Adrian Bunk <bunk@kernel.org> wrote:
> 
> > This patch fixes the following compile error with gcc 3.2:
> > 
> > <--  snip  -->
> > 
> > ...
> >   CC      mm/mm_init.o
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:77:1: directives may not be used inside a macro argument
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:76:47: unterminated argument list invoking macro "mminit_dprintk"
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c: In function `mminit_verify_pageflags_layout':
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: `mminit_dprintk' undeclared (first use in this function)
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: (Each undeclared identifier is reported only once
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: for each function it appears in.)
> > /home/bunk/linux/kernel-2.6/git/linux-2.6/mm/mm_init.c:80: syntax error before numeric constant
> > make[2]: *** [mm/mm_init.o] Error 1
> > 
> > <--  snip  -->
> > 
> > 
> > --- a/mm/mm_init.c
> > +++ b/mm/mm_init.c
> > @@ -63,6 +63,13 @@ void __init mminit_verify_pageflags_layout(void)
> >  {
> >  	int shift, width;
> >  	unsigned long or_mask, add_mask;
> > +	int sections_shift;
> > +
> > +#ifdef SECTIONS_SHIFT
> > +	sections_shift = SECTIONS_SHIFT;
> > +#else
> > +	sections_shift = 0;
> > +#endif
> >  
> >  	shift = 8 * sizeof(unsigned long);
> >  	width = shift - SECTIONS_WIDTH - NODES_WIDTH - ZONES_WIDTH;
> > @@ -74,11 +81,7 @@ void __init mminit_verify_pageflags_layout(void)
> >  		NR_PAGEFLAGS);
> >  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
> >  		"Section %d Node %d Zone %d\n",
> > -#ifdef SECTIONS_SHIFT
> > -		SECTIONS_SHIFT,
> > -#else
> > -		0,
> > -#endif
> > +		sections_shift,
> >  		NODES_SHIFT,
> >  		ZONES_SHIFT);
> >  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",
> 
> OK, ifdefs inside macro expansion aren't a great idea.
> 
> Mel, is there any erason why we shouldn't do it this way?
> 

The only minor nit is SECTIONS_SHIFT is defined outside of SPARSEMEM even
though it has no meaning there. There is no problem with this as such but
someone being silly might use SECTIONS_SHIFT accidently. If this sort of
behaviour was considered a possibility, we could add something like this to
mm/mm_init.c instead?

#ifndef SECTIONS_SHIFT
#define SECTIONS_SHIFT 0
#endif /* SECTIONS_SHIFT */

>  include/linux/mmzone.h |    3 ++-
>  mm/mm_init.c           |    4 ----
> 
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff -puN include/linux/mmzone.h~a include/linux/mmzone.h
> --- a/include/linux/mmzone.h~a
> +++ a/include/linux/mmzone.h
> @@ -831,7 +831,7 @@ static inline unsigned long early_pfn_to
>  #ifdef CONFIG_SPARSEMEM
>  
>  /*
> - * SECTION_SHIFT    		#bits space required to store a section #
> + * SECTIONS_SHIFT    		#bits space required to store a section #
>   *
>   * PA_SECTION_SHIFT		physical address to/from section number
>   * PFN_SECTION_SHIFT		pfn to/from section number
> @@ -973,6 +973,7 @@ static inline int pfn_present(unsigned l
>  #define early_pfn_valid(pfn)	pfn_valid(pfn)
>  void sparse_init(void);
>  #else
> +#define SECTIONS_SHIFT		0
>  #define sparse_init()	do {} while (0)
>  #define sparse_index_init(_sec, _nid)  do {} while (0)
>  #endif /* CONFIG_SPARSEMEM */
> diff -puN mm/mm_init.c~a mm/mm_init.c
> --- a/mm/mm_init.c~a
> +++ a/mm/mm_init.c
> @@ -74,11 +74,7 @@ void __init mminit_verify_pageflags_layo
>  		NR_PAGEFLAGS);
>  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
>  		"Section %d Node %d Zone %d\n",
> -#ifdef SECTIONS_SHIFT
>  		SECTIONS_SHIFT,
> -#else
> -		0,
> -#endif
>  		NODES_SHIFT,
>  		ZONES_SHIFT);
>  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",
> _
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

Re: [2.6 patch] fix mm/mm_init.c compilation

[Andrew Morton]

On Tue, 5 Aug 2008 08:51:23 +0100 Mel Gorman <mel@csn.ul.ie> wrote:

> > > --- a/mm/mm_init.c
> > > +++ b/mm/mm_init.c
> > > @@ -63,6 +63,13 @@ void __init mminit_verify_pageflags_layout(void)
> > >  {
> > >  	int shift, width;
> > >  	unsigned long or_mask, add_mask;
> > > +	int sections_shift;
> > > +
> > > +#ifdef SECTIONS_SHIFT
> > > +	sections_shift = SECTIONS_SHIFT;
> > > +#else
> > > +	sections_shift = 0;
> > > +#endif
> > >  
> > >  	shift = 8 * sizeof(unsigned long);
> > >  	width = shift - SECTIONS_WIDTH - NODES_WIDTH - ZONES_WIDTH;
> > > @@ -74,11 +81,7 @@ void __init mminit_verify_pageflags_layout(void)
> > >  		NR_PAGEFLAGS);
> > >  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
> > >  		"Section %d Node %d Zone %d\n",
> > > -#ifdef SECTIONS_SHIFT
> > > -		SECTIONS_SHIFT,
> > > -#else
> > > -		0,
> > > -#endif
> > > +		sections_shift,
> > >  		NODES_SHIFT,
> > >  		ZONES_SHIFT);
> > >  	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",
> > 
> > OK, ifdefs inside macro expansion aren't a great idea.
> > 
> > Mel, is there any erason why we shouldn't do it this way?
> > 
> 
> The only minor nit is SECTIONS_SHIFT is defined outside of SPARSEMEM even
> though it has no meaning there. There is no problem with this as such but
> someone being silly might use SECTIONS_SHIFT accidently. If this sort of
> behaviour was considered a possibility, we could add something like this to
> mm/mm_init.c instead?

That sounds sane.

diff -puN mm/mm_init.c~mm_initc-avoid-ifdef-inside-macro-expansion mm/mm_init.c
--- a/mm/mm_init.c~mm_initc-avoid-ifdef-inside-macro-expansion
+++ a/mm/mm_init.c
@@ -14,6 +14,10 @@
 #ifdef CONFIG_DEBUG_MEMORY_INIT
 int __meminitdata mminit_loglevel;
 
+#ifndef SECTIONS_SHIFT
+#define SECTIONS_SHIFT	0
+#endif
+
 /* The zonelists are simply reported, validation is manual. */
 void mminit_verify_zonelist(void)
 {
@@ -74,11 +78,7 @@ void __init mminit_verify_pageflags_layo
 		NR_PAGEFLAGS);
 	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_shifts",
 		"Section %d Node %d Zone %d\n",
-#ifdef SECTIONS_SHIFT
 		SECTIONS_SHIFT,
-#else
-		0,
-#endif
 		NODES_SHIFT,
 		ZONES_SHIFT);
 	mminit_dprintk(MMINIT_TRACE, "pageflags_layout_offsets",
_