linux-kernel - Re: [2.6 patch] fix mm/mm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20080805021141.0b060e04.akpm@linux-foundation.org>
Date:	Tue, 5 Aug 2008 02:11:41 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Mel Gorman <mel@....ul.ie>
Cc:	Adrian Bunk <bunk@...nel.org>, torvalds@...ux-foundation.org,
	linux-kernel@...r.kernel.org, apw@...dowen.org
Subject: Re: [2.6 patch] fix mm/mm_init.c compilation

On Tue, 5 Aug 2008 08:51:23 +0100 Mel Gorman <mel@....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",
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/