Introduce CONFIG_STABLE to control checks only useful for development.
Signed-off-by: Christoph Lameter <[email protected]>
---
init/Kconfig | 7 +++++++
1 file changed, 7 insertions(+)
Index: slub/init/Kconfig
===================================================================
--- slub.orig/init/Kconfig 2007-05-30 16:35:05.000000000 -0700
+++ slub/init/Kconfig 2007-05-30 16:35:45.000000000 -0700
@@ -65,6 +65,13 @@ endmenu
menu "General setup"
+config STABLE
+ bool "Stable kernel"
+ help
+ If the kernel is configured to be a stable kernel then various
+ checks that are only of interest to kernel development will be
+ omitted.
+
config LOCALVERSION
string "Local version - append to kernel release"
help
--
Hi Christoph,
>Introduce CONFIG_STABLE to control checks only useful for development.
What about control checks only as SLUB_DEBUG is set?
Regards
dave
On Thu, 31 May 2007, young dave wrote:
> Hi Christoph,
>
> > Introduce CONFIG_STABLE to control checks only useful for development.
>
> What about control checks only as SLUB_DEBUG is set?
Debug code is always included in all builds unless it is an embedded
system. Debug code is kept out of the hot path.
Disabling SLUB_DEBUG should only be done for embedded systems. That is why
the option is in CONFIG_EMBEDDED.
> --- slub.orig/init/Kconfig 2007-05-30 16:35:05.000000000 -0700
> +++ slub/init/Kconfig 2007-05-30 16:35:45.000000000 -0700
> @@ -65,6 +65,13 @@ endmenu
>
> menu "General setup"
>
> +config STABLE
> + bool "Stable kernel"
> + help
> + If the kernel is configured to be a stable kernel then various
> + checks that are only of interest to kernel development will be
> + omitted.
> +
> config LOCALVERSION
> string "Local version - append to kernel release"
> help
a) Why in Kconfig, why not in Makefile?
b) Of course nobody wants STABLE=n. :-) How about:
config RELEASE
bool "Build for release"
help
If the kernel is declared as a release build here, then
various checks that are only of interest to kernel development
will be omitted.
c) A drawback of this general option is, it's hard to tell what will be
omitted in particular.
--
Stefan Richter
-=====-=-=== -=-= =====
http://arcgraph.de/sr/
From: Stefan Richter <[email protected]>
Date: Thu, 31 May 2007 10:54:36 +0200
> b) Of course nobody wants STABLE=n. :-) How about:
>
> config RELEASE
> bool "Build for release"
> help
> If the kernel is declared as a release build here, then
> various checks that are only of interest to kernel development
> will be omitted.
Agreed :-)
>
> c) A drawback of this general option is, it's hard to tell what will be
> omitted in particular.
In that sense it is similar to EMBEDDED, but I still think there
is high value to this, I can already think of several things I
want to put under this which are only noise I want to see during
development periods.
>> menu "General setup"
>>
>> +config STABLE
>> + bool "Stable kernel"
[...]
> a) Why in Kconfig, why not in Makefile?
>
> b) Of course nobody wants STABLE=n. :-) How about:
>
> config RELEASE
> bool "Build for release"
> help
> If the kernel is declared as a release build here, then
> various checks that are only of interest to kernel development
> will be omitted.
PS: Also, it could be reversed (e.g. config TESTBUILD) and put into the
Kernel Hacking submenu.
> c) A drawback of this general option is, it's hard to tell what will be
> omitted in particular.
--
Stefan Richter
-=====-=-=== -=-= =====
http://arcgraph.de/sr/
On Wed, 30 May 2007 17:20:48 -0700
[email protected] wrote:
> Introduce CONFIG_STABLE to control checks only useful for development.
>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> ---
> init/Kconfig | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> Index: slub/init/Kconfig
> ===================================================================
> --- slub.orig/init/Kconfig 2007-05-30 16:35:05.000000000 -0700
> +++ slub/init/Kconfig 2007-05-30 16:35:45.000000000 -0700
> @@ -65,6 +65,13 @@ endmenu
>
> menu "General setup"
>
> +config STABLE
> + bool "Stable kernel"
> + help
> + If the kernel is configured to be a stable kernel then various
> + checks that are only of interest to kernel development will be
> + omitted.
> +
> config LOCALVERSION
> string "Local version - append to kernel release"
> help
OK, but I think it'd be better if this knob was at line 6 of ./Makefile so
that Linus remembers to turn it on and off at appropriate times. Also, I
suspect that we want it available within Kconfig expressions as well as
within cpp expressions.
So something like this:
diff -puN Makefile~a Makefile
--- a/Makefile~a
+++ a/Makefile
@@ -3,6 +3,7 @@ PATCHLEVEL = 6
SUBLEVEL = 22
EXTRAVERSION = -rc3
NAME = Jeff Thinks I Should Change This, But To What?
+DEVEL_KERNEL = 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 DEVEL_KERNEL
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
diff -puN scripts/kconfig/symbol.c~a scripts/kconfig/symbol.c
--- a/scripts/kconfig/symbol.c~a
+++ a/scripts/kconfig/symbol.c
@@ -68,6 +68,15 @@ void sym_init(void)
if (p)
sym_add_default(sym, p);
+ sym = sym_lookup("DEVEL_KERNEL", 0);
+ sym->type = S_BOOLEAN;
+ sym->flags |= SYMBOL_AUTO;
+ p = getenv("DEVEL_KERNEL");
+ if (p && atoi(p))
+ sym_add_default(sym, "y");
+ else
+ sym_add_default(sym, "n");
+
sym = sym_lookup("UNAME_RELEASE", 0);
sym->type = S_STRING;
sym->flags |= SYMBOL_AUTO;
_
With the following behaviour:
DEVEL_KERNEL = 0 in Makefile:
DEVEL_KERNEL=n in Kconfig
CONFIG_DEVEL_KERNEL is not set in cpp
DEVEL_KERNEL = 1 in Makefile:
DEVEL_KERNEL=y in Kconfig
CONFIG_DEVEL_KERNEL is set in cpp
however the above patch doesn't do this correctly and I got bored of
fiddling with it. Help?
On Thu, 31 May 2007, Andrew Morton wrote:
> however the above patch doesn't do this correctly and I got bored of
> fiddling with it. Help?
Will get something like what you described into the next release if
no one else has anything to add.
>
> So something like this:
>
> diff -puN Makefile~a Makefile
> --- a/Makefile~a
> +++ a/Makefile
> @@ -3,6 +3,7 @@ PATCHLEVEL = 6
> SUBLEVEL = 22
> EXTRAVERSION = -rc3
> NAME = Jeff Thinks I Should Change This, But To What?
> +DEVEL_KERNEL = 1
Could we name this: KERNELDEVEL to fit with current naming convention?
Alternative: KERNEL_DEVEL
Maybe a little comment that this is mirrored as a CONFIG_ symbol?
>
> # *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 DEVEL_KERNEL
> 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
> diff -puN scripts/kconfig/symbol.c~a scripts/kconfig/symbol.c
> --- a/scripts/kconfig/symbol.c~a
> +++ a/scripts/kconfig/symbol.c
> @@ -68,6 +68,15 @@ void sym_init(void)
> if (p)
> sym_add_default(sym, p);
>
> + sym = sym_lookup("DEVEL_KERNEL", 0);
> + sym->type = S_BOOLEAN;
> + sym->flags |= SYMBOL_AUTO;
> + p = getenv("DEVEL_KERNEL");
> + if (p && atoi(p))
> + sym_add_default(sym, "y");
> + else
> + sym_add_default(sym, "n");
> +
sym_set_tristate_value(sym, yes);
else
sym_set_tristate_value(sym, no);
should do the trick (untested).
Sam
On Thu, May 31, 2007 at 11:30:46PM +0200, Sam Ravnborg wrote:
> > + sym = sym_lookup("DEVEL_KERNEL", 0);
> > + sym->type = S_BOOLEAN;
> > + sym->flags |= SYMBOL_AUTO;
> > + p = getenv("DEVEL_KERNEL");
> > + if (p && atoi(p))
> > + sym_add_default(sym, "y");
> > + else
> > + sym_add_default(sym, "n");
> > +
>
> sym_set_tristate_value(sym, yes);
> else
> sym_set_tristate_value(sym, no);
>
> should do the trick (untested).
Odd. What's the third state ? Undefined?
Dave
--
http://www.codemonkey.org.uk
On Wed, May 30, 2007 at 05:49:56PM -0700, Christoph Lameter wrote:
> On Thu, 31 May 2007, young dave wrote:
>
> > Hi Christoph,
> >
> > > Introduce CONFIG_STABLE to control checks only useful for development.
> >
> > What about control checks only as SLUB_DEBUG is set?
>
> Debug code is always included in all builds unless it is an embedded
> system. Debug code is kept out of the hot path.
>
> Disabling SLUB_DEBUG should only be done for embedded systems. That is why
> the option is in CONFIG_EMBEDDED.
Something I'd really love to have is a CONFIG option to decide if
slub_debug is set or not by default. The reasoning behind this is that during
development of each Fedora release, I used to leave SLAB_DEBUG=y for
months on end and catch all kinds of nasties.
Now that I've switched it over to using slub, I ended up adding the
ugly patch below, because otherwise, no-one would ever run with
slub_debug and we'd miss out on all those lovely bugs.
(I have 'make release' and 'make debug' targets which enable/disable
this [and other] patches in the Fedora kernel).
(Patch for illustration only, obviously not for applying).
Unless someone beats me to it, I'll hack up a CONFIG option around
this. Having that turned on if !CONFIG_STABLE would also be a win I think.
Dave
--- linux-2.6/mm/slub.c~ 2007-05-27 21:48:42.000000000 -0400
+++ linux-2.6/mm/slub.c 2007-05-27 21:51:22.000000000 -0400
@@ -323,7 +323,7 @@ static inline int slab_index(void *p, st
/*
* Debug settings:
*/
-static int slub_debug;
+static int slub_debug = DEBUG_DEFAULT_FLAGS;
static char *slub_debug_slabs;
--
http://www.codemonkey.org.uk
On Fri, 1 Jun 2007, Dave Jones wrote:
> > Disabling SLUB_DEBUG should only be done for embedded systems. That is why
> > the option is in CONFIG_EMBEDDED.
>
> Something I'd really love to have is a CONFIG option to decide if
> slub_debug is set or not by default. The reasoning behind this is that during
> development of each Fedora release, I used to leave SLAB_DEBUG=y for
> months on end and catch all kinds of nasties.
So slub_debug as a boot parameter is not enough.
> Now that I've switched it over to using slub, I ended up adding the
> ugly patch below, because otherwise, no-one would ever run with
> slub_debug and we'd miss out on all those lovely bugs.
Oh. No worry. By default slub puts its free pointer in the most dangerous
area. In my experience it will bug immediately if there is something
wrong. The mode of operations that I had in mind for development was to
run until we crash somewhere. Then reboot with slub_debug to get the
lovely report on who did it.
> (I have 'make release' and 'make debug' targets which enable/disable
> this [and other] patches in the Fedora kernel).
>
> (Patch for illustration only, obviously not for applying).
Hummm..... I need to think about this one.
> Unless someone beats me to it, I'll hack up a CONFIG option around
> this. Having that turned on if !CONFIG_STABLE would also be a win I think.
Doing so will impair performance testing. Memory use will be changed due
to the growth of all the objects etc etc. Generally I think running
with slub_debug by default is overkill.
Having said that you can do even more if you would run
slabinfo -v
to validate object from cron. That way you can check up on all slab
objects.
On Fri, Jun 01, 2007 at 02:02:00PM -0400, Dave Jones wrote:
> On Thu, May 31, 2007 at 11:30:46PM +0200, Sam Ravnborg wrote:
> > > + sym = sym_lookup("DEVEL_KERNEL", 0);
> > > + sym->type = S_BOOLEAN;
> > > + sym->flags |= SYMBOL_AUTO;
> > > + p = getenv("DEVEL_KERNEL");
> > > + if (p && atoi(p))
> > > + sym_add_default(sym, "y");
> > > + else
> > > + sym_add_default(sym, "n");
> > > +
> >
> > sym_set_tristate_value(sym, yes);
> > else
> > sym_set_tristate_value(sym, no);
> >
> > should do the trick (untested).
>
> Odd. What's the third state ? Undefined?
no, mod, yes
Representing: no, module, yes as the three config choices.
Sam
>
> With the following behaviour:
>
> DEVEL_KERNEL = 0 in Makefile:
>
> DEVEL_KERNEL=n in Kconfig
> CONFIG_DEVEL_KERNEL is not set in cpp
>
> DEVEL_KERNEL = 1 in Makefile:
>
> DEVEL_KERNEL=y in Kconfig
> CONFIG_DEVEL_KERNEL is set in cpp
>
> however the above patch doesn't do this correctly and I got bored of
> fiddling with it. Help?
My first try below.
It does the kconfig stuff as expected.
But the CONFIG_KERNEL_DEVEL is NOT updated unless you
touch .config (or in other ways change the config).
I did not see an easy way to fix that - Roman?
I only had to add SYMBOL_VALID as falg to get it working - but it
took me a while to figure out. Somehow all the comments describing the
data structures for kconfig has got lost.
Sam
diff --git a/Makefile b/Makefile
index 562a909..362668c 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?
+KERNEL_DEVEL =
# *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 KERNEL_DEVEL
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
diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index 8770a5d..5373d58 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -91,6 +91,14 @@ source "init/Kconfig"
menu "Processor type and features"
+config MY_KERNEL_DEVEL
+ bool "Needs Kernel devel"
+ depends on KERNEL_DEVEL
+
+config MY_KERNEL_DEVEL2
+ bool "Do not need kernel devel"
+ depends on !KERNEL_DEVEL
+
source "kernel/time/Kconfig"
config SMP
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c35dcc5..fb4d5b8 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -68,6 +68,15 @@ void sym_init(void)
if (p)
sym_add_default(sym, p);
+ sym = sym_lookup("KERNEL_DEVEL", 0);
+ sym->type = S_BOOLEAN;
+ sym->flags |= SYMBOL_VALID|SYMBOL_AUTO;
+ p = getenv("KERNEL_DEVEL");
+ if (p && atoi(p))
+ sym_add_default(sym, "y");
+ else
+ sym_add_default(sym, "n");
+
sym = sym_lookup("UNAME_RELEASE", 0);
sym->type = S_STRING;
sym->flags |= SYMBOL_AUTO;
On Fri, Jun 01, 2007 at 10:22:02PM +0200, Sam Ravnborg wrote:
> On Fri, Jun 01, 2007 at 02:02:00PM -0400, Dave Jones wrote:
> > On Thu, May 31, 2007 at 11:30:46PM +0200, Sam Ravnborg wrote:
> > > > + sym = sym_lookup("DEVEL_KERNEL", 0);
> > > > + sym->type = S_BOOLEAN;
> > > > + sym->flags |= SYMBOL_AUTO;
> > > > + p = getenv("DEVEL_KERNEL");
> > > > + if (p && atoi(p))
> > > > + sym_add_default(sym, "y");
> > > > + else
> > > > + sym_add_default(sym, "n");
> > > > +
> > >
> > > sym_set_tristate_value(sym, yes);
> > > else
> > > sym_set_tristate_value(sym, no);
> > >
> > > should do the trick (untested).
> >
> > Odd. What's the third state ? Undefined?
> no, mod, yes
> Representing: no, module, yes as the three config choices.
Now I'm even more puzzled. Why would 'DEVEL_KERNEL' need
to be modular ?
Dave
--
http://www.codemonkey.org.uk
On Fri, Jun 01, 2007 at 04:30:06PM -0400, Dave Jones wrote:
> On Fri, Jun 01, 2007 at 10:22:02PM +0200, Sam Ravnborg wrote:
> > On Fri, Jun 01, 2007 at 02:02:00PM -0400, Dave Jones wrote:
> > > On Thu, May 31, 2007 at 11:30:46PM +0200, Sam Ravnborg wrote:
> > > > > + sym = sym_lookup("DEVEL_KERNEL", 0);
> > > > > + sym->type = S_BOOLEAN;
> > > > > + sym->flags |= SYMBOL_AUTO;
> > > > > + p = getenv("DEVEL_KERNEL");
> > > > > + if (p && atoi(p))
> > > > > + sym_add_default(sym, "y");
> > > > > + else
> > > > > + sym_add_default(sym, "n");
> > > > > +
> > > >
> > > > sym_set_tristate_value(sym, yes);
> > > > else
> > > > sym_set_tristate_value(sym, no);
> > > >
> > > > should do the trick (untested).
> > >
> > > Odd. What's the third state ? Undefined?
> > no, mod, yes
> > Representing: no, module, yes as the three config choices.
>
> Now I'm even more puzzled. Why would 'DEVEL_KERNEL' need
> to be modular ?
The same type is used to represent a boolean and a tristate
within kconfig:
typedef enum tristate {
no, mod, yes
} tristate;
And in the cases where the config symbol is of type 'bool' then
the value 'mod' is not used.
Sam