2.6.12-rc5-mm1 - missing #define SECTIONS_SHIFT in sparsemem

2.6.12-rc5-mm1 - missing #define SECTIONS_SHIFT in sparsemem

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

sparsemem-memory-model.patch references SECTIONS_SHIFT without defining it.

Caught this while compiling with -Wundef, causes lots of warnings
when it gets used in include/linux/mm.h.  The appended patch Works For Me,
although I wonder if the *real* problem isn't a missing '#ifdef CONFIG_SPARSEMEM'
around the code that uses it in mm.h.  
 
Signed-Off-By: valdis.kletnieks@vt.edu

--- linux-2.6.12-rc5-mm1/include/linux/mmzone.h.ifdef	2005-05-27 15:12:26.000000000 -0400
+++ linux-2.6.12-rc5-mm1/include/linux/mmzone.h	2005-05-27 16:26:40.000000000 -0400
@@ -568,6 +568,7 @@ static inline int pfn_valid(unsigned lon
 void sparse_init(void);
 #else
 #define sparse_init()	do {} while (0)
+#define SECTIONS_SHIFT	0
 #endif /* CONFIG_SPARSEMEM */
 
 #ifdef CONFIG_NODES_SPAN_OTHER_NODES


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.12-rc5-mm1 - missing #define SECTIONS_SHIFT in sparsemem

[Andy Whitcroft]

Valdis.Kletnieks@vt.edu wrote:
> sparsemem-memory-model.patch references SECTIONS_SHIFT without defining it.
> 
> Caught this while compiling with -Wundef, causes lots of warnings
> when it gets used in include/linux/mm.h.  The appended patch Works For Me,
> although I wonder if the *real* problem isn't a missing '#ifdef CONFIG_SPARSEMEM'
> around the code that uses it in mm.h.  
>  
> Signed-Off-By: valdis.kletnieks@vt.edu
> 
> --- linux-2.6.12-rc5-mm1/include/linux/mmzone.h.ifdef	2005-05-27 15:12:26.000000000 -0400
> +++ linux-2.6.12-rc5-mm1/include/linux/mmzone.h	2005-05-27 16:26:40.000000000 -0400
> @@ -568,6 +568,7 @@ static inline int pfn_valid(unsigned lon
>  void sparse_init(void);
>  #else
>  #define sparse_init()	do {} while (0)
> +#define SECTIONS_SHIFT	0
>  #endif /* CONFIG_SPARSEMEM */
>  
>  #ifdef CONFIG_NODES_SPAN_OTHER_NODES

Odd.  I guess there must be a reference from an unused function to this
define when SPARSEMEM is off.  Can you send me your .config please and
I'll have a look.

-apw

Re: 2.6.12-rc5-mm1 - missing #define SECTIONS_SHIFT in sparsemem

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

On Mon, 30 May 2005 00:27:19 BST, Andy Whitcroft said:
> Valdis.Kletnieks@vt.edu wrote:
> > sparsemem-memory-model.patch references SECTIONS_SHIFT without defining it.
> > 
> > Caught this while compiling with -Wundef, causes lots of warnings
> > when it gets used in include/linux/mm.h.  The appended patch Works For Me,
> > although I wonder if the *real* problem isn't a missing '#ifdef CONFIG_SPARSEMEM'
> > around the code that uses it in mm.h.  
> >  
> > Signed-Off-By: valdis.kletnieks@vt.edu
> > 
> > --- linux-2.6.12-rc5-mm1/include/linux/mmzone.h.ifdef	2005-05-27 15:1
2:26.000000000 -0400
> > +++ linux-2.6.12-rc5-mm1/include/linux/mmzone.h	2005-05-27 16:26:40.000
000000 -0400
> > @@ -568,6 +568,7 @@ static inline int pfn_valid(unsigned lon
> >  void sparse_init(void);
> >  #else
> >  #define sparse_init()	do {} while (0)
> > +#define SECTIONS_SHIFT	0
> >  #endif /* CONFIG_SPARSEMEM */
> >  
> >  #ifdef CONFIG_NODES_SPAN_OTHER_NODES
> 
> Odd.  I guess there must be a reference from an unused function to this
> define when SPARSEMEM is off.  Can you send me your .config please and
> I'll have a look.

The warning comes out of *every* kernel module that #includes kernel/mm.h
and reaches line 424:

 #if SECTIONS_SHIFT+NODES_SHIFT+ZONES_SHIFT <= FLAGS_RESERVED
 #define NODES_WIDTH             NODES_SHIFT
 #else
 #define NODES_WIDTH             0
 #endif

That's not wrapped in a #ifdef CONFIG_DISCONTIGMEM or SPARSEMEN or HIGHMEM or
any of the other obvious candidates.  You only see the warning if you add
-Wundef to your CFLAGS.

I'll send the .config privately in a separate message. Pretty standard stock
defaults for a laptop with only 256M of memory. The quick quick
synopsis for the rest of you:

[/usr/src/linux-2.6.12-rc5-mm1]2 grep MEM .config
CONFIG_SHMEM=y
# CONFIG_TINY_SHMEM is not set
CONFIG_NOHIGHMEM=y
# CONFIG_HIGHMEM4G is not set
# CONFIG_HIGHMEM64G is not set
CONFIG_SELECT_MEMORY_MODEL=y
CONFIG_FLATMEM_MANUAL=y
# CONFIG_DISCONTIGMEM_MANUAL is not set
# CONFIG_SPARSEMEM_MANUAL is not set
CONFIG_FLATMEM=y
CONFIG_FLAT_NODE_MEM_MAP=y
# CONFIG_BLK_DEV_UMEM is not set
CONFIG_SND_DEBUG_MEMORY=y



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.12-rc5-mm1 - missing #define SECTIONS_SHIFT in sparsemem

[Andy Whitcroft]

Could you try this patch out for me.  I've reordered the code
slightly to better match the original intent of the code whilst
avoiding the reference to the undefined value.

Cheers.

-apw

=== 8< ===

valdis.kletnieks@vt.edu reported that with -Wundef triggers warnings
for each use of mm.h, out of the flags fit code for SPARSEMEM.
Change this check so that it uses the actually allocated flags widths
rather than the maxima for each.  This matches the original intent,
adding the nodes field if there is space and avoids the reference
to SECTIONS_SHIFT which was triggering the warning.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>

---
 mm.h |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff -X /home/apw/brief/lib/vdiff.excl -rupN reference/include/linux/mm.h current/include/linux/mm.h
--- reference/include/linux/mm.h
+++ current/include/linux/mm.h
@@ -421,12 +421,6 @@ static inline void put_page(struct page 
  * with space for node: | SECTION | NODE | ZONE | ... | FLAGS |
  *   no space for node: | SECTION |     ZONE    | ... | FLAGS |
  */
-#if SECTIONS_SHIFT+NODES_SHIFT+ZONES_SHIFT <= FLAGS_RESERVED
-#define NODES_WIDTH		NODES_SHIFT
-#else
-#define NODES_WIDTH		0
-#endif
-
 #ifdef CONFIG_SPARSEMEM
 #define SECTIONS_WIDTH		SECTIONS_SHIFT
 #else
@@ -435,6 +429,12 @@ static inline void put_page(struct page 
 
 #define ZONES_WIDTH		ZONES_SHIFT
 
+#if SECTIONS_WIDTH+ZONES_WIDTH+NODES_SHIFT <= FLAGS_RESERVED
+#define NODES_WIDTH		NODES_SHIFT
+#else
+#define NODES_WIDTH		0
+#endif
+
 /* Page flags: | [SECTION] | [NODE] | ZONE | ... | FLAGS | */
 #define SECTIONS_PGOFF		((sizeof(page_flags_t)*8) - SECTIONS_WIDTH)
 #define NODES_PGOFF		(SECTIONS_PGOFF - NODES_WIDTH)