Conditionals for development tests and output

Conditionals for development tests and output

[Christoph Lameter]

This introduces 

CONFIG_DEVELKERNEL

If CONFIG_DEVELKERNEL is set then this is a development kernel.
Otherwise the kernel to be built is a a production kernel.


If CONFIG_DEVELKERNEL is set then the constant

DEVELKERNEL

is set to 1. Otherwise it is zero.

The following functions are defined in kernel.h for diagnostics

DEVEL_WARN_ON(condition)

	Warn on the condition but only if this is a development kernel

DEVEL_WARN_ON_ONCE(condition)

	Warn on the condition once but only if this is a development kernel

devel_printk(fmt, ...)

	Output that should only be generated for a development kernel.


CONFIG_DEVELKERNEL is set and cleared by edit the Makefile. Line 7 contains

DEVELKERNEL = 1


Updates SLAB and SLUB to not perform the kmalloc(0) tests anymore
if the kernel is not a development kernel.


Editing DEVELKERNEL requires a 

	make clean

to correctly rebuild the kernel with the proper settings. This is similar 
to editing the version number. Sam: Is there any way to avoid the make 
clean?

Cc: Dave Jones <davej@redhat.com>
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.com>
Signed-off-by: Christoph Lameter <clameter@sgi.com>


---
 Makefile                 |    3 ++-
 include/linux/kernel.h   |   14 ++++++++++++++
 include/linux/slub_def.h |    2 +-
 mm/slab.c                |    2 +-
 mm/slub.c                |    4 ++--
 scripts/kconfig/symbol.c |    9 +++++++++
 6 files changed, 29 insertions(+), 5 deletions(-)

Index: slub/Makefile
===================================================================
--- slub.orig/Makefile	2007-06-01 20:11:57.000000000 -0700
+++ slub/Makefile	2007-06-01 20:56:27.000000000 -0700
@@ -3,6 +3,7 @@ PATCHLEVEL = 6
 SUBLEVEL = 22
 EXTRAVERSION = -rc3-mm1
 NAME = Jeff Thinks I Should Change This, But To What?
+DEVELKERNEL = 1
 
 # *DOCUMENTATION*
 # To see a list of typical targets execute "make help"
@@ -320,7 +321,7 @@ AFLAGS          := -D__ASSEMBLY__
 KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
 KERNELVERSION = $(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
 
-export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
+export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION DEVELKERNEL
 export ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
Index: slub/scripts/kconfig/symbol.c
===================================================================
--- slub.orig/scripts/kconfig/symbol.c	2007-06-01 20:13:24.000000000 -0700
+++ slub/scripts/kconfig/symbol.c	2007-06-01 21:07:52.000000000 -0700
@@ -72,6 +72,15 @@ void sym_init(void)
 	sym->type = S_STRING;
 	sym->flags |= SYMBOL_AUTO;
 	sym_add_default(sym, uts.release);
+
+	sym = sym_lookup("DEVELKERNEL", 0);
+	sym->type = S_BOOLEAN;
+	sym->flags |= SYMBOL_VALID|SYMBOL_AUTO;
+	p = getenv("DEVELKERNEL");
+	if (p && atoi(p))
+		sym_add_default(sym, "y");
+	else
+		sym_add_default(sym, "n");
 }
 
 enum symbol_type sym_get_type(struct symbol *sym)
Index: slub/include/linux/kernel.h
===================================================================
--- slub.orig/include/linux/kernel.h	2007-06-01 20:19:17.000000000 -0700
+++ slub/include/linux/kernel.h	2007-06-01 20:30:16.000000000 -0700
@@ -375,4 +375,18 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#ifdef CONFIG_DEVELKERNEL
+#define DEVELKERNEL 1
+#else
+#define DEVELKERNEL 0
+#endif
+
+#define DEVEL_WARN_ON(x)	WARN_ON(DEVELKERNEL && (x))
+#define DEVEL_WARN_ON_ONCE(x)	WARN_ON_ONCE(DEVELKERNEL && (x))
+
+#define devel_printk(fmt,arg...) 	({					\
+	if (DEVELKERNEL)						\
+			printk(fmt, ##arg);				\
+})
+
 #endif
Index: slub/include/linux/slub_def.h
===================================================================
--- slub.orig/include/linux/slub_def.h	2007-06-01 20:25:06.000000000 -0700
+++ slub/include/linux/slub_def.h	2007-06-01 20:28:02.000000000 -0700
@@ -80,7 +80,7 @@ static inline int kmalloc_index(size_t s
 	 * allocate memory but return ZERO_SIZE_PTR.
 	 * WARN so that people can review and fix their code.
 	 */
-	WARN_ON_ONCE(size == 0);
+	DEVEL_WARN_ON_ONCE(size == 0);
 
 	if (!size)
 		return 0;
Index: slub/mm/slub.c
===================================================================
--- slub.orig/mm/slub.c	2007-06-01 20:24:26.000000000 -0700
+++ slub/mm/slub.c	2007-06-01 20:28:20.000000000 -0700
@@ -2525,8 +2525,8 @@ void __init kmem_cache_init(void)
 	kmem_size = offsetof(struct kmem_cache, cpu_slab) +
 				nr_cpu_ids * sizeof(struct page *);
 
-	printk(KERN_INFO "SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, MinObjects=%d,"
-		" Processors=%d, Nodes=%d\n",
+	devel_printk(KERN_INFO "SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, "
+		"MinObjects=%d, Processors=%d, Nodes=%d\n",
 		KMALLOC_SHIFT_HIGH, cache_line_size(),
 		slub_min_order, slub_max_order, slub_min_objects,
 		nr_cpu_ids, nr_node_ids);
Index: slub/mm/slab.c
===================================================================
--- slub.orig/mm/slab.c	2007-06-01 20:29:39.000000000 -0700
+++ slub/mm/slab.c	2007-06-01 20:29:45.000000000 -0700
@@ -774,7 +774,7 @@ static inline struct kmem_cache *__find_
 	 */
 	BUG_ON(malloc_sizes[INDEX_AC].cs_cachep == NULL);
 #endif
-	WARN_ON_ONCE(size == 0);
+	DEVEL_WARN_ON_ONCE(size == 0);
 	while (size > csizep->cs_size)
 		csizep++;
 

Re: Conditionals for development tests and output

[Adrian Bunk]

On Fri, Jun 01, 2007 at 09:17:53PM -0700, Christoph Lameter wrote:
> This introduces 
> 
> CONFIG_DEVELKERNEL
> 
> If CONFIG_DEVELKERNEL is set then this is a development kernel.
> Otherwise the kernel to be built is a a production kernel.
>...

I have two problems with your patch:

First, there will be a slub/slab specific mechanism for debug code 
controlled by a parameter in the Makefile plus the common 
CONFIG_DEBUG_KERNEL approach used by most other debug options.
We need one approach for all options.

And your approach could easily result in code paths never tested in 
-mm or -rc kernels exploding in the actual release.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: Conditionals for development tests and output

[Andrew Morton]

On Sat, 2 Jun 2007 16:43:48 +0200 Adrian Bunk <bunk@stusta.de> wrote:

> And your approach could easily result in code paths never tested in 
> -mm or -rc kernels exploding in the actual release.

yep, we need to ensure that DEVELKERNEL gets turned off a few weeks
before final release.

Re: Conditionals for development tests and output

[Sam Ravnborg]

On Fri, Jun 01, 2007 at 09:17:53PM -0700, Christoph Lameter wrote:
> This introduces 
> 
> CONFIG_DEVELKERNEL
> 
> If CONFIG_DEVELKERNEL is set then this is a development kernel.
> Otherwise the kernel to be built is a a production kernel.

The below patch suffers from the problem that changing the
value of DEVELKERNEL in the Makefile are not automatically
reflected in autoconf.h.
We need to get this fixed before merging this I think.

I have looked a bit on the kconfig stuff but so far
I did not find a good way to do so.

I hope Roman Zippel can come up with something - yes?

	Sam

[Kept rest of mail on purpose]
> 
> 
> If CONFIG_DEVELKERNEL is set then the constant
> 
> DEVELKERNEL
> 
> is set to 1. Otherwise it is zero.
> 
> The following functions are defined in kernel.h for diagnostics
> 
> DEVEL_WARN_ON(condition)
> 
> 	Warn on the condition but only if this is a development kernel
> 
> DEVEL_WARN_ON_ONCE(condition)
> 
> 	Warn on the condition once but only if this is a development kernel
> 
> devel_printk(fmt, ...)
> 
> 	Output that should only be generated for a development kernel.
> 
> 
> CONFIG_DEVELKERNEL is set and cleared by edit the Makefile. Line 7 contains
> 
> DEVELKERNEL = 1
> 
> 
> Updates SLAB and SLUB to not perform the kmalloc(0) tests anymore
> if the kernel is not a development kernel.
> 
> 
> Editing DEVELKERNEL requires a 
> 
> 	make clean
> 
> to correctly rebuild the kernel with the proper settings. This is similar 
> to editing the version number. Sam: Is there any way to avoid the make 
> clean?
> 
> Cc: Dave Jones <davej@redhat.com>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.com>
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> 
> ---
>  Makefile                 |    3 ++-
>  include/linux/kernel.h   |   14 ++++++++++++++
>  include/linux/slub_def.h |    2 +-
>  mm/slab.c                |    2 +-
>  mm/slub.c                |    4 ++--
>  scripts/kconfig/symbol.c |    9 +++++++++
>  6 files changed, 29 insertions(+), 5 deletions(-)
> 
> Index: slub/Makefile
> ===================================================================
> --- slub.orig/Makefile	2007-06-01 20:11:57.000000000 -0700
> +++ slub/Makefile	2007-06-01 20:56:27.000000000 -0700
> @@ -3,6 +3,7 @@ PATCHLEVEL = 6
>  SUBLEVEL = 22
>  EXTRAVERSION = -rc3-mm1
>  NAME = Jeff Thinks I Should Change This, But To What?
> +DEVELKERNEL = 1
>  
>  # *DOCUMENTATION*
>  # To see a list of typical targets execute "make help"
> @@ -320,7 +321,7 @@ AFLAGS          := -D__ASSEMBLY__
>  KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
>  KERNELVERSION = $(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
>  
> -export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
> +export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION DEVELKERNEL
>  export ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
>  export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
>  export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
> Index: slub/scripts/kconfig/symbol.c
> ===================================================================
> --- slub.orig/scripts/kconfig/symbol.c	2007-06-01 20:13:24.000000000 -0700
> +++ slub/scripts/kconfig/symbol.c	2007-06-01 21:07:52.000000000 -0700
> @@ -72,6 +72,15 @@ void sym_init(void)
>  	sym->type = S_STRING;
>  	sym->flags |= SYMBOL_AUTO;
>  	sym_add_default(sym, uts.release);
> +
> +	sym = sym_lookup("DEVELKERNEL", 0);
> +	sym->type = S_BOOLEAN;
> +	sym->flags |= SYMBOL_VALID|SYMBOL_AUTO;
> +	p = getenv("DEVELKERNEL");
> +	if (p && atoi(p))
> +		sym_add_default(sym, "y");
> +	else
> +		sym_add_default(sym, "n");
>  }
>  
>  enum symbol_type sym_get_type(struct symbol *sym)
> Index: slub/include/linux/kernel.h
> ===================================================================
> --- slub.orig/include/linux/kernel.h	2007-06-01 20:19:17.000000000 -0700
> +++ slub/include/linux/kernel.h	2007-06-01 20:30:16.000000000 -0700
> @@ -375,4 +375,18 @@ struct sysinfo {
>  #define NUMA_BUILD 0
>  #endif
>  
> +#ifdef CONFIG_DEVELKERNEL
> +#define DEVELKERNEL 1
> +#else
> +#define DEVELKERNEL 0
> +#endif
> +
> +#define DEVEL_WARN_ON(x)	WARN_ON(DEVELKERNEL && (x))
> +#define DEVEL_WARN_ON_ONCE(x)	WARN_ON_ONCE(DEVELKERNEL && (x))
> +
> +#define devel_printk(fmt,arg...) 	({					\
> +	if (DEVELKERNEL)						\
> +			printk(fmt, ##arg);				\
> +})
> +
>  #endif
> Index: slub/include/linux/slub_def.h
> ===================================================================
> --- slub.orig/include/linux/slub_def.h	2007-06-01 20:25:06.000000000 -0700
> +++ slub/include/linux/slub_def.h	2007-06-01 20:28:02.000000000 -0700
> @@ -80,7 +80,7 @@ static inline int kmalloc_index(size_t s
>  	 * allocate memory but return ZERO_SIZE_PTR.
>  	 * WARN so that people can review and fix their code.
>  	 */
> -	WARN_ON_ONCE(size == 0);
> +	DEVEL_WARN_ON_ONCE(size == 0);
>  
>  	if (!size)
>  		return 0;
> Index: slub/mm/slub.c
> ===================================================================
> --- slub.orig/mm/slub.c	2007-06-01 20:24:26.000000000 -0700
> +++ slub/mm/slub.c	2007-06-01 20:28:20.000000000 -0700
> @@ -2525,8 +2525,8 @@ void __init kmem_cache_init(void)
>  	kmem_size = offsetof(struct kmem_cache, cpu_slab) +
>  				nr_cpu_ids * sizeof(struct page *);
>  
> -	printk(KERN_INFO "SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, MinObjects=%d,"
> -		" Processors=%d, Nodes=%d\n",
> +	devel_printk(KERN_INFO "SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, "
> +		"MinObjects=%d, Processors=%d, Nodes=%d\n",
>  		KMALLOC_SHIFT_HIGH, cache_line_size(),
>  		slub_min_order, slub_max_order, slub_min_objects,
>  		nr_cpu_ids, nr_node_ids);
> Index: slub/mm/slab.c
> ===================================================================
> --- slub.orig/mm/slab.c	2007-06-01 20:29:39.000000000 -0700
> +++ slub/mm/slab.c	2007-06-01 20:29:45.000000000 -0700
> @@ -774,7 +774,7 @@ static inline struct kmem_cache *__find_
>  	 */
>  	BUG_ON(malloc_sizes[INDEX_AC].cs_cachep == NULL);
>  #endif
> -	WARN_ON_ONCE(size == 0);
> +	DEVEL_WARN_ON_ONCE(size == 0);
>  	while (size > csizep->cs_size)
>  		csizep++;
>  

Re: Conditionals for development tests and output

[Christoph Lameter]

On Sat, 2 Jun 2007, Sam Ravnborg wrote:

> The below patch suffers from the problem that changing the
> value of DEVELKERNEL in the Makefile are not automatically
> reflected in autoconf.h.

Well you are restating what is in the description. See below.

> We need to get this fixed before merging this I think.

I thought you would maybe able to help?

> > Editing DEVELKERNEL requires a 
> > 
> > 	make clean
> > 
> > to correctly rebuild the kernel with the proper settings. This is similar 
> > to editing the version number. Sam: Is there any way to avoid the make 
> > clean?
   ^^^^^^^^^^^^

Re: Conditionals for development tests and output

[Sam Ravnborg]

On Sat, Jun 02, 2007 at 04:29:27PM -0700, Christoph Lameter wrote:
> On Sat, 2 Jun 2007, Sam Ravnborg wrote:
> 
> > The below patch suffers from the problem that changing the
> > value of DEVELKERNEL in the Makefile are not automatically
> > reflected in autoconf.h.
> 
> Well you are restating what is in the description. See below.

This turned out to be a simple dependency issue.
Added Makefile as prerequisite solved it as far as my testing showed.
Updated patch below.

The naming I do not like.
What about: KERNELHACKING (to make association with the Kernel hacking menu)?

Thats a claer signal that if you hack on the kernel you better set this.

	Sam

diff --git a/Makefile b/Makefile
index 562a909..d1e1e6e 100644
--- a/Makefile
+++ b/Makefile
@@ -3,6 +3,7 @@ PATCHLEVEL = 6
 SUBLEVEL = 22
 EXTRAVERSION = -rc3
 NAME = Jeff Thinks I Should Change This, But To What?
+DEVELKERNEL = 1
 
 # *DOCUMENTATION*
 # To see a list of typical targets execute "make help"
@@ -320,7 +321,7 @@ AFLAGS          := -D__ASSEMBLY__
 KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
 KERNELVERSION = $(VERSION).$(PATCHLEVEL).$(SUBLEVEL)$(EXTRAVERSION)
 
-export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
+export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION DEVELKERNEL
 export ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
@@ -453,7 +454,8 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
 # with it and forgot to run make oldconfig.
 # if auto.conf.cmd is missing then we are probably in a cleaned tree so
 # we execute the config step to be sure to catch updated Kconfig files
-include/config/auto.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
+include/config/auto.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd Makefile
+	$(Q)echo "  Kconfig - silentoldconfig"
 	$(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
 else
 # external modules needs include/linux/autoconf.h and include/config/auto.conf
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 45353d7..cb98841 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -375,4 +375,18 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#ifdef CONFIG_DEVELKERNEL
+#define DEVELKERNEL 1
+#else
+#define DEVELKERNEL 0
+#endif
+
+#define DEVEL_WARN_ON(x)	WARN_ON(DEVELKERNEL && (x))
+#define DEVEL_WARN_ON_ONCE(x)	WARN_ON_ONCE(DEVELKERNEL && (x))
+
+#define devel_printk(fmt,arg...) 	({					\
+	if (DEVELKERNEL)						\
+			printk(fmt, ##arg);				\
+})
+
 #endif
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 0764c82..8c58d44 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -74,7 +74,7 @@ static inline int kmalloc_index(size_t size)
 	 * We should return 0 if size == 0 but we use the smallest object
 	 * here for SLAB legacy reasons.
 	 */
-	WARN_ON_ONCE(size == 0);
+	DEVEL_WARN_ON_ONCE(size == 0);
 
 	if (size > KMALLOC_MAX_SIZE)
 		return -1;
diff --git a/mm/slab.c b/mm/slab.c
index 2e71a32..0b7aca9 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -774,7 +774,7 @@ static inline struct kmem_cache *__find_general_cachep(size_t size,
 	 */
 	BUG_ON(malloc_sizes[INDEX_AC].cs_cachep == NULL);
 #endif
-	WARN_ON_ONCE(size == 0);
+	DEVEL_WARN_ON_ONCE(size == 0);
 	while (size > csizep->cs_size)
 		csizep++;
 
diff --git a/mm/slub.c b/mm/slub.c
index 3e5aefc..4de2b24 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2464,8 +2464,8 @@ void __init kmem_cache_init(void)
 	kmem_size = offsetof(struct kmem_cache, cpu_slab) +
 				nr_cpu_ids * sizeof(struct page *);
 
-	printk(KERN_INFO "SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, MinObjects=%d,"
-		" Processors=%d, Nodes=%d\n",
+	devel_printk(KERN_INFO "SLUB: Genslabs=%d, HWalign=%d, Order=%d-%d, "
+		"MinObjects=%d, Processors=%d, Nodes=%d\n",
 		KMALLOC_SHIFT_HIGH, cache_line_size(),
 		slub_min_order, slub_max_order, slub_min_objects,
 		nr_cpu_ids, nr_node_ids);
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c35dcc5..bf788ad 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -72,6 +72,15 @@ void sym_init(void)
 	sym->type = S_STRING;
 	sym->flags |= SYMBOL_AUTO;
 	sym_add_default(sym, uts.release);
+
+	sym = sym_lookup("DEVELKERNEL", 0);
+	sym->type = S_BOOLEAN;
+	sym->flags |= SYMBOL_VALID|SYMBOL_AUTO;
+	p = getenv("DEVELKERNEL");
+	if (p && atoi(p))
+		sym_add_default(sym, "y");
+	else
+		sym_add_default(sym, "n");
 }
 
 enum symbol_type sym_get_type(struct symbol *sym)

Re: Conditionals for development tests and output

[Matt Mackall]

On Sat, Jun 02, 2007 at 10:48:21AM -0700, Andrew Morton wrote:
> On Sat, 2 Jun 2007 16:43:48 +0200 Adrian Bunk <bunk@stusta.de> wrote:
> 
> > And your approach could easily result in code paths never tested in 
> > -mm or -rc kernels exploding in the actual release.
> 
> yep, we need to ensure that DEVELKERNEL gets turned off a few weeks
> before final release.

I'm not very keen on this whole idea. It's a fairly well-known
phenomenon that behavior can change for the worse when you turn off
debug flags for a variety of reasons. So instituting a monoculture
where -everyone- has a global debug flag on then turns it off is a little
worrisome.

When we're talking about a single warning, it's probably not a big
deal, but once we expand the usage to cover a dozen or a hundred
things, the odds that we'll hide a race or side-effect somewhere
increase.

-- 
Mathematics is the supreme nostalgia of our time.