linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
@ 2011-06-05 21:02 Josh Triplett
  2011-06-05 21:31 ` Frederic Weisbecker
  2011-06-06 15:18 ` [PATCH] " Ingo Molnar
  0 siblings, 2 replies; 18+ messages in thread
From: Josh Triplett @ 2011-06-05 21:02 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

Several debugging options currently default to y, such as
CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
want to turn those options off to save space; however, turning them off
requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
CONFIG_EXPERT exists specifically to unhide options potentially needed
by experts and/or embedded users, make CONFIG_EXPERT automatically imply
CONFIG_DEBUG_KERNEL.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

Revised as requested by Ingo Molnar.

 init/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index ebafac4..5e482a3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -909,6 +909,7 @@ config ANON_INODES
 
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
+	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
           to be disabled or tweaked. This is for specialized
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-05 21:02 [PATCH] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options Josh Triplett
@ 2011-06-05 21:31 ` Frederic Weisbecker
  2011-06-06  1:23   ` [PATCHv2] " Josh Triplett
  2011-06-06 15:18 ` [PATCH] " Ingo Molnar
  1 sibling, 1 reply; 18+ messages in thread
From: Frederic Weisbecker @ 2011-06-05 21:31 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

On Sun, Jun 05, 2011 at 02:02:25PM -0700, Josh Triplett wrote:
> Several debugging options currently default to y, such as
> CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> want to turn those options off to save space; however, turning them off
> requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> CONFIG_EXPERT exists specifically to unhide options potentially needed
> by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> CONFIG_DEBUG_KERNEL.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> 
> Revised as requested by Ingo Molnar.
> 
>  init/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index ebafac4..5e482a3 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -909,6 +909,7 @@ config ANON_INODES
>  
>  menuconfig EXPERT
>  	bool "Configure standard kernel features (expert users)"
> +	select DEBUG_KERNEL

Please add a quick comment that explains the reason for that select. Because
without that changelog, it's very non obvious.

Thanks.

>  	help
>  	  This option allows certain base kernel options and settings
>            to be disabled or tweaked. This is for specialized
> -- 
> 1.7.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCHv2] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-05 21:31 ` Frederic Weisbecker
@ 2011-06-06  1:23   ` Josh Triplett
  2011-06-06 22:06     ` [tip:core/debug] debug: " tip-bot for Josh Triplett
  0 siblings, 1 reply; 18+ messages in thread
From: Josh Triplett @ 2011-06-06  1:23 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

Several debugging options currently default to y, such as
CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
want to turn those options off to save space; however, turning them off
requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
CONFIG_EXPERT exists specifically to unhide options potentially needed
by experts and/or embedded users, make CONFIG_EXPERT automatically imply
CONFIG_DEBUG_KERNEL.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

v2: Add a comment explaining the select of DEBUG_KERNEL, as suggested by
Frederic Weisbecker.

 init/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index ebafac4..48a173e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -909,6 +909,8 @@ config ANON_INODES
 
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+ 	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
           to be disabled or tweaked. This is for specialized
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-05 21:02 [PATCH] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options Josh Triplett
  2011-06-05 21:31 ` Frederic Weisbecker
@ 2011-06-06 15:18 ` Ingo Molnar
  2011-06-06 16:51   ` [PATCHv3] " Josh Triplett
  1 sibling, 1 reply; 18+ messages in thread
From: Ingo Molnar @ 2011-06-06 15:18 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Thomas Gleixner, Sam Ravnborg, Andrew Morton, Linus Torvalds,
	linux-kernel


* Josh Triplett <josh@joshtriplett.org> wrote:

> Several debugging options currently default to y, such as
> CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> want to turn those options off to save space; however, turning them off
> requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> CONFIG_EXPERT exists specifically to unhide options potentially needed
> by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> CONFIG_DEBUG_KERNEL.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> 
> Revised as requested by Ingo Molnar.
> 
>  init/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index ebafac4..5e482a3 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -909,6 +909,7 @@ config ANON_INODES
>  
>  menuconfig EXPERT
>  	bool "Configure standard kernel features (expert users)"
> +	select DEBUG_KERNEL
>  	help
>  	  This option allows certain base kernel options and settings
>            to be disabled or tweaked. This is for specialized

So now that it's selected, mind removing the superfluous EXPERT bits 
from the Kconfig files that have it?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCHv3] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 15:18 ` [PATCH] " Ingo Molnar
@ 2011-06-06 16:51   ` Josh Triplett
  2011-06-06 18:16     ` Frederic Weisbecker
  2011-06-06 21:39     ` [tip:core/debug] debug: " tip-bot for Josh Triplett
  0 siblings, 2 replies; 18+ messages in thread
From: Josh Triplett @ 2011-06-06 16:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Thomas Gleixner, Sam Ravnborg, Andrew Morton, Linus Torvalds,
	linux-kernel

Several debugging options currently default to y, such as
CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
want to turn those options off to save space; however, turning them off
requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
CONFIG_EXPERT exists specifically to unhide options potentially needed
by experts and/or embedded users, make CONFIG_EXPERT automatically imply
CONFIG_DEBUG_KERNEL.

Change various debugging options to only reference DEBUG_KERNEL, not
EXPERT.

Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed wrong,
since EXPERT should not directly affect anything except the availability
of other options.  (And EXPERT definitely shouldn't mean "implicitly
turn off default safety features".)  Change it to default to y, which
means that turning on EXPERT does not automatically disable it, but will
provide the option to disable it.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

v3: Changed other debugging options to stop referencing EXPERT.

 arch/tile/Kconfig.debug |    2 +-
 init/Kconfig            |    2 ++
 lib/Kconfig.debug       |    6 +++---
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/tile/Kconfig.debug b/arch/tile/Kconfig.debug
index ddbfc33..afc9cf3 100644
--- a/arch/tile/Kconfig.debug
+++ b/arch/tile/Kconfig.debug
@@ -3,7 +3,7 @@ menu "Kernel hacking"
 source "lib/Kconfig.debug"
 
 config EARLY_PRINTK
-	bool "Early printk" if EXPERT && DEBUG_KERNEL
+	bool "Early printk" if DEBUG_KERNEL
 	default y
 	help
 	  Write kernel log output directly via the hypervisor console.
diff --git a/init/Kconfig b/init/Kconfig
index ebafac4..14370d3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -909,6 +909,8 @@ config ANON_INODES
 
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
           to be disabled or tweaked. This is for specialized
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dd373c8..b121413 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -694,7 +694,7 @@ config DEBUG_HIGHMEM
 	  Disable for production systems.
 
 config DEBUG_BUGVERBOSE
-	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
+	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL
 	depends on BUG
 	depends on ARM || AVR32 || M32R || M68K || SPARC32 || SPARC64 || \
 		   FRV || SUPERH || GENERIC_BUG || BLACKFIN || MN10300 || TILE
@@ -766,8 +766,8 @@ config DEBUG_WRITECOUNT
 	  If unsure, say N.
 
 config DEBUG_MEMORY_INIT
-	bool "Debug memory initialisation" if EXPERT
-	default !EXPERT
+	bool "Debug memory initialisation" if DEBUG_KERNEL
+	default y
 	help
 	  Enable this for additional checks during memory initialisation.
 	  The sanity checks verify aspects of the VM such as the memory model
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCHv3] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 16:51   ` [PATCHv3] " Josh Triplett
@ 2011-06-06 18:16     ` Frederic Weisbecker
  2011-06-06 18:39       ` Josh Triplett
  2011-06-06 21:39     ` [tip:core/debug] debug: " tip-bot for Josh Triplett
  1 sibling, 1 reply; 18+ messages in thread
From: Frederic Weisbecker @ 2011-06-06 18:16 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

On Mon, Jun 06, 2011 at 09:51:30AM -0700, Josh Triplett wrote:
> Several debugging options currently default to y, such as
> CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> want to turn those options off to save space; however, turning them off
> requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> CONFIG_EXPERT exists specifically to unhide options potentially needed
> by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> CONFIG_DEBUG_KERNEL.
> 
> Change various debugging options to only reference DEBUG_KERNEL, not
> EXPERT.

Ok, so ideally this should have been done in two patches. They are both
different logical changes that don't imply the same.

v2 was fine, and should be an independent change.

But some comment on what was added in this v3 below:

> 
> Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed wrong,
> since EXPERT should not directly affect anything except the availability
> of other options.  (And EXPERT definitely shouldn't mean "implicitly
> turn off default safety features".)  Change it to default to y, which
> means that turning on EXPERT does not automatically disable it, but will
> provide the option to disable it.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> 
> v3: Changed other debugging options to stop referencing EXPERT.
> 
>  arch/tile/Kconfig.debug |    2 +-
>  init/Kconfig            |    2 ++
>  lib/Kconfig.debug       |    6 +++---
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/tile/Kconfig.debug b/arch/tile/Kconfig.debug
> index ddbfc33..afc9cf3 100644
> --- a/arch/tile/Kconfig.debug
> +++ b/arch/tile/Kconfig.debug
> @@ -3,7 +3,7 @@ menu "Kernel hacking"
>  source "lib/Kconfig.debug"
>  
>  config EARLY_PRINTK
> -	bool "Early printk" if EXPERT && DEBUG_KERNEL
> +	bool "Early printk" if DEBUG_KERNEL
>  	default y

I don't understand why early_printk is default y. But that's debatable,
we may want to have any user able to run a raw vga console for debugging
requests.

Either:

- if we consider it's very important and mandatory, let's keep it "if EXPERT"
and default y.

- if we consider it's only used by kernel developers, it should be off by
default and depend on DEBUG_KERNEL only.

>  	help
>  	  Write kernel log output directly via the hypervisor console.
> diff --git a/init/Kconfig b/init/Kconfig
> index ebafac4..14370d3 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -909,6 +909,8 @@ config ANON_INODES
>  
>  menuconfig EXPERT
>  	bool "Configure standard kernel features (expert users)"
> +	# Unhide debug options, to make the on-by-default options visible
> +	select DEBUG_KERNEL
>  	help
>  	  This option allows certain base kernel options and settings
>            to be disabled or tweaked. This is for specialized
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index dd373c8..b121413 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -694,7 +694,7 @@ config DEBUG_HIGHMEM
>  	  Disable for production systems.
>  
>  config DEBUG_BUGVERBOSE
> -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL

I believe this should not be changed, let keep it how it is, or change to
"if EXPERT" because EXPERT selects DEBUG_KERNEL anyway.

There are no good reason to disable this option, except for some embedded
system perhaps, so EXPERT still make sense here. 

Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv3] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 18:16     ` Frederic Weisbecker
@ 2011-06-06 18:39       ` Josh Triplett
  2011-06-06 21:34         ` Frederic Weisbecker
  2011-06-06 22:08         ` [PATCHv4] " Josh Triplett
  0 siblings, 2 replies; 18+ messages in thread
From: Josh Triplett @ 2011-06-06 18:39 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

On Mon, Jun 06, 2011 at 08:16:09PM +0200, Frederic Weisbecker wrote:
> On Mon, Jun 06, 2011 at 09:51:30AM -0700, Josh Triplett wrote:
> > Several debugging options currently default to y, such as
> > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> > want to turn those options off to save space; however, turning them off
> > requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> > CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> > CONFIG_EXPERT exists specifically to unhide options potentially needed
> > by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> > CONFIG_DEBUG_KERNEL.
> > 
> > Change various debugging options to only reference DEBUG_KERNEL, not
> > EXPERT.
> 
> Ok, so ideally this should have been done in two patches. They are both
> different logical changes that don't imply the same.

Fair enough; I'd considered whether to split this patch or not, and
ended up with "not" on the theory that this seemed like a case of "fix
an API and fix up the callers", but I can easily split it in v4.

> v2 was fine, and should be an independent change.
> 
> But some comment on what was added in this v3 below:
> 
> > Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed wrong,
> > since EXPERT should not directly affect anything except the availability
> > of other options.  (And EXPERT definitely shouldn't mean "implicitly
> > turn off default safety features".)  Change it to default to y, which
> > means that turning on EXPERT does not automatically disable it, but will
> > provide the option to disable it.
> > 
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > ---
> > 
> > v3: Changed other debugging options to stop referencing EXPERT.
> > 
> >  arch/tile/Kconfig.debug |    2 +-
> >  init/Kconfig            |    2 ++
> >  lib/Kconfig.debug       |    6 +++---
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/tile/Kconfig.debug b/arch/tile/Kconfig.debug
> > index ddbfc33..afc9cf3 100644
> > --- a/arch/tile/Kconfig.debug
> > +++ b/arch/tile/Kconfig.debug
> > @@ -3,7 +3,7 @@ menu "Kernel hacking"
> >  source "lib/Kconfig.debug"
> >  
> >  config EARLY_PRINTK
> > -	bool "Early printk" if EXPERT && DEBUG_KERNEL
> > +	bool "Early printk" if DEBUG_KERNEL
> >  	default y
> 
> I don't understand why early_printk is default y. But that's debatable,
> we may want to have any user able to run a raw vga console for debugging
> requests.
> 
> Either:
> 
> - if we consider it's very important and mandatory, let's keep it "if EXPERT"
> and default y.
> 
> - if we consider it's only used by kernel developers, it should be off by
> default and depend on DEBUG_KERNEL only.

Note that this change occurs in arch/tile/Kconfig.debug; you'd have to
ask the Tile architecture developers about the rationale there.  For the
purposes of this change, I'd prefer not to make any semantic change
there.  Based on your explanation, that probably means I should change
it to "if EXPERT" rather than "if DEBUG_KERNEL", keeping the current
behavior of "only experts can turn this off".  I'll do that in v4.

> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -694,7 +694,7 @@ config DEBUG_HIGHMEM
> >  	  Disable for production systems.
> >  
> >  config DEBUG_BUGVERBOSE
> > -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> > +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL
> 
> I believe this should not be changed, let keep it how it is, or change to
> "if EXPERT" because EXPERT selects DEBUG_KERNEL anyway.
> 
> There are no good reason to disable this option, except for some embedded
> system perhaps, so EXPERT still make sense here. 

I guess this matches the rationale above for tile's EARLY_PRINTK; I'll
use "if EXPERT" to preserve the behavior of "only experts can turn this
off".  To me it seems equivalent to any other debug option, and I don't
really see the harm in allowing users to turn it off if they turned on
DEBUG_KERNEL, but nevertheless this patch shouldn't make that kind of
semantic change.

- Josh Triplett

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv3] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 18:39       ` Josh Triplett
@ 2011-06-06 21:34         ` Frederic Weisbecker
  2011-06-06 22:02           ` Ingo Molnar
  2011-06-06 22:08         ` [PATCHv4] " Josh Triplett
  1 sibling, 1 reply; 18+ messages in thread
From: Frederic Weisbecker @ 2011-06-06 21:34 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

On Mon, Jun 06, 2011 at 11:39:39AM -0700, Josh Triplett wrote:
> On Mon, Jun 06, 2011 at 08:16:09PM +0200, Frederic Weisbecker wrote:
> > On Mon, Jun 06, 2011 at 09:51:30AM -0700, Josh Triplett wrote:
> > > Several debugging options currently default to y, such as
> > > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> > > want to turn those options off to save space; however, turning them off
> > > requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> > > CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> > > CONFIG_EXPERT exists specifically to unhide options potentially needed
> > > by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> > > CONFIG_DEBUG_KERNEL.
> > > 
> > > Change various debugging options to only reference DEBUG_KERNEL, not
> > > EXPERT.
> > 
> > Ok, so ideally this should have been done in two patches. They are both
> > different logical changes that don't imply the same.
> 
> Fair enough; I'd considered whether to split this patch or not, and
> ended up with "not" on the theory that this seemed like a case of "fix
> an API and fix up the callers", but I can easily split it in v4.

You haven't just fixed the API and propagated to the callers accordingly.
What you did is to fix the API and also completely change the behaviour of the callers.

You have changed EXPERT to imply DEBUG_KERNEL.
You'd have fixed the callers accordingly if you only changed "depend on EXPERT && DEBUG_KERNEL"
into "depend on EXPERT" in the callers, because if EXPERT => DEBUG_KERNEL then
(EXPERT && DEBUG_KERNEL) == EXPERT. Thus it would have fit into the same logic and
gathering the whole into a single patch would have made sense (although that's debatable, I
would have cut that personally, but we don't care at that level).

Now changing callers that "depend on EXPERT && DEBUG_KERNEL" to "depend on DEBUG_KERNEL" subscribes
to a completely different logical change. It has actually even nothing to deal with
the previous change. You can make that behaviour change even without having EXPERT => DEBUG_KERNEL.

So not only you gathered two different logical changes into a same patch, but the
two things even hardly have any connection together.

 
> > v2 was fine, and should be an independent change.
> > 
> > But some comment on what was added in this v3 below:
> > 
> > > Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed wrong,
> > > since EXPERT should not directly affect anything except the availability
> > > of other options.  (And EXPERT definitely shouldn't mean "implicitly
> > > turn off default safety features".)  Change it to default to y, which
> > > means that turning on EXPERT does not automatically disable it, but will
> > > provide the option to disable it.
> > > 
> > > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > > ---
> > > 
> > > v3: Changed other debugging options to stop referencing EXPERT.
> > > 
> > >  arch/tile/Kconfig.debug |    2 +-
> > >  init/Kconfig            |    2 ++
> > >  lib/Kconfig.debug       |    6 +++---
> > >  3 files changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/tile/Kconfig.debug b/arch/tile/Kconfig.debug
> > > index ddbfc33..afc9cf3 100644
> > > --- a/arch/tile/Kconfig.debug
> > > +++ b/arch/tile/Kconfig.debug
> > > @@ -3,7 +3,7 @@ menu "Kernel hacking"
> > >  source "lib/Kconfig.debug"
> > >  
> > >  config EARLY_PRINTK
> > > -	bool "Early printk" if EXPERT && DEBUG_KERNEL
> > > +	bool "Early printk" if DEBUG_KERNEL
> > >  	default y
> > 
> > I don't understand why early_printk is default y. But that's debatable,
> > we may want to have any user able to run a raw vga console for debugging
> > requests.
> > 
> > Either:
> > 
> > - if we consider it's very important and mandatory, let's keep it "if EXPERT"
> > and default y.
> > 
> > - if we consider it's only used by kernel developers, it should be off by
> > default and depend on DEBUG_KERNEL only.
> 
> Note that this change occurs in arch/tile/Kconfig.debug; you'd have to
> ask the Tile architecture developers about the rationale there.  For the
> purposes of this change, I'd prefer not to make any semantic change
> there.  Based on your explanation, that probably means I should change
> it to "if EXPERT" rather than "if DEBUG_KERNEL", keeping the current
> behavior of "only experts can turn this off".  I'll do that in v4.

Ah it's in tile! I read that part too quickly, sorry. Yeah let them
deal with that and only have "depend on EXPERT"

> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -694,7 +694,7 @@ config DEBUG_HIGHMEM
> > >  	  Disable for production systems.
> > >  
> > >  config DEBUG_BUGVERBOSE
> > > -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> > > +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL
> > 
> > I believe this should not be changed, let keep it how it is, or change to
> > "if EXPERT" because EXPERT selects DEBUG_KERNEL anyway.
> > 
> > There are no good reason to disable this option, except for some embedded
> > system perhaps, so EXPERT still make sense here. 
> 
> I guess this matches the rationale above for tile's EARLY_PRINTK; I'll
> use "if EXPERT" to preserve the behavior of "only experts can turn this
> off".

Cool.

> To me it seems equivalent to any other debug option, and I don't
> really see the harm in allowing users to turn it off if they turned on
> DEBUG_KERNEL, but nevertheless this patch shouldn't make that kind of
> semantic change.

It's not equivalent to other kernel debug options because unlike most other debug
options we don't want only kernel developers to enable it, we want everyone to
enable it.

If you let this option to be easily turned off, people may disable it by mistake
and then developers will suddenly receive useless bug reports, which can be
a frustrating experience.

There is no good reason to disable it except in some particular embedded cases I guess.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 16:51   ` [PATCHv3] " Josh Triplett
  2011-06-06 18:16     ` Frederic Weisbecker
@ 2011-06-06 21:39     ` tip-bot for Josh Triplett
  2011-06-06 21:45       ` Frederic Weisbecker
  1 sibling, 1 reply; 18+ messages in thread
From: tip-bot for Josh Triplett @ 2011-06-06 21:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, torvalds, sam, fweisbec, tglx, josh, mingo

Commit-ID:  2458073da04571182748aa4289630dca52ba63cc
Gitweb:     http://git.kernel.org/tip/2458073da04571182748aa4289630dca52ba63cc
Author:     Josh Triplett <josh@joshtriplett.org>
AuthorDate: Mon, 6 Jun 2011 09:51:30 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 6 Jun 2011 19:30:01 +0200

debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options

Several debugging options currently default to y, such as
CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.

Embedded users might want to turn those options off to save space;
however, turning them off requires turning on CONFIG_DEBUG_KERNEL to
unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
unhide debugging options, and CONFIG_EXPERT exists specifically
to unhide options potentially needed by experts and/or embedded
users, make CONFIG_EXPERT automatically imply CONFIG_DEBUG_KERNEL.

Change various debugging options to only reference DEBUG_KERNEL,
not EXPERT.

Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed
wrong, since EXPERT should not directly affect anything except
the availability of other options.  (And EXPERT definitely
shouldn't mean "implicitly turn off default safety features".)

Change it to default to y, which means that turning on EXPERT
does not automatically disable it, but will provide the option
to disable it.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20110606165130.GA1844@leaf
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/tile/Kconfig.debug |    2 +-
 init/Kconfig            |    2 ++
 lib/Kconfig.debug       |    6 +++---
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/tile/Kconfig.debug b/arch/tile/Kconfig.debug
index ddbfc33..afc9cf3 100644
--- a/arch/tile/Kconfig.debug
+++ b/arch/tile/Kconfig.debug
@@ -3,7 +3,7 @@ menu "Kernel hacking"
 source "lib/Kconfig.debug"
 
 config EARLY_PRINTK
-	bool "Early printk" if EXPERT && DEBUG_KERNEL
+	bool "Early printk" if DEBUG_KERNEL
 	default y
 	help
 	  Write kernel log output directly via the hypervisor console.
diff --git a/init/Kconfig b/init/Kconfig
index ebafac4..14370d3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -909,6 +909,8 @@ config ANON_INODES
 
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
           to be disabled or tweaked. This is for specialized
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dd373c8..b121413 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -694,7 +694,7 @@ config DEBUG_HIGHMEM
 	  Disable for production systems.
 
 config DEBUG_BUGVERBOSE
-	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
+	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL
 	depends on BUG
 	depends on ARM || AVR32 || M32R || M68K || SPARC32 || SPARC64 || \
 		   FRV || SUPERH || GENERIC_BUG || BLACKFIN || MN10300 || TILE
@@ -766,8 +766,8 @@ config DEBUG_WRITECOUNT
 	  If unsure, say N.
 
 config DEBUG_MEMORY_INIT
-	bool "Debug memory initialisation" if EXPERT
-	default !EXPERT
+	bool "Debug memory initialisation" if DEBUG_KERNEL
+	default y
 	help
 	  Enable this for additional checks during memory initialisation.
 	  The sanity checks verify aspects of the VM such as the memory model

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 21:39     ` [tip:core/debug] debug: " tip-bot for Josh Triplett
@ 2011-06-06 21:45       ` Frederic Weisbecker
  2011-06-06 22:09         ` Josh Triplett
  0 siblings, 1 reply; 18+ messages in thread
From: Frederic Weisbecker @ 2011-06-06 21:45 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, sam, torvalds, tglx, josh, mingo
  Cc: linux-tip-commits

On Mon, Jun 06, 2011 at 09:39:40PM +0000, tip-bot for Josh Triplett wrote:
> Commit-ID:  2458073da04571182748aa4289630dca52ba63cc
> Gitweb:     http://git.kernel.org/tip/2458073da04571182748aa4289630dca52ba63cc
> Author:     Josh Triplett <josh@joshtriplett.org>
> AuthorDate: Mon, 6 Jun 2011 09:51:30 -0700
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Mon, 6 Jun 2011 19:30:01 +0200
> 
> debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
> 
> Several debugging options currently default to y, such as
> CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.
> 
> Embedded users might want to turn those options off to save space;
> however, turning them off requires turning on CONFIG_DEBUG_KERNEL to
> unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
> unhide debugging options, and CONFIG_EXPERT exists specifically
> to unhide options potentially needed by experts and/or embedded
> users, make CONFIG_EXPERT automatically imply CONFIG_DEBUG_KERNEL.
> 
> Change various debugging options to only reference DEBUG_KERNEL,
> not EXPERT.
> 
> Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed
> wrong, since EXPERT should not directly affect anything except
> the availability of other options.  (And EXPERT definitely
> shouldn't mean "implicitly turn off default safety features".)
> 
> Change it to default to y, which means that turning on EXPERT
> does not automatically disable it, but will provide the option
> to disable it.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

No, Nacked actually. I'm fine with the v2 that only changed
the EXPERT config to imply DEBUG_KERNEL. But all the other
changes here need to be revised, as I outlined in the thread.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv3] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 21:34         ` Frederic Weisbecker
@ 2011-06-06 22:02           ` Ingo Molnar
  0 siblings, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2011-06-06 22:02 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Josh Triplett, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Mon, Jun 06, 2011 at 11:39:39AM -0700, Josh Triplett wrote:
> > On Mon, Jun 06, 2011 at 08:16:09PM +0200, Frederic Weisbecker wrote:
> > > On Mon, Jun 06, 2011 at 09:51:30AM -0700, Josh Triplett wrote:
> > > > Several debugging options currently default to y, such as
> > > > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> > > > want to turn those options off to save space; however, turning them off
> > > > requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> > > > CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> > > > CONFIG_EXPERT exists specifically to unhide options potentially needed
> > > > by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> > > > CONFIG_DEBUG_KERNEL.
> > > > 
> > > > Change various debugging options to only reference DEBUG_KERNEL, not
> > > > EXPERT.
> > > 
> > > Ok, so ideally this should have been done in two patches. They are both
> > > different logical changes that don't imply the same.
> > 
> > Fair enough; I'd considered whether to split this patch or not, and
> > ended up with "not" on the theory that this seemed like a case of "fix
> > an API and fix up the callers", but I can easily split it in v4.
> 
> You haven't just fixed the API and propagated to the callers accordingly.
> What you did is to fix the API and also completely change the behaviour of the callers.
> 
> You have changed EXPERT to imply DEBUG_KERNEL.

yes, i suggested that, because it leads to a more maintainable 
(EXPERT noise free) DEBUG_KERNEL Kconfig section.

> You'd have fixed the callers accordingly if you only changed 
> "depend on EXPERT && DEBUG_KERNEL" into "depend on EXPERT" in the 
> callers, because if EXPERT => DEBUG_KERNEL then (EXPERT && 
> DEBUG_KERNEL) == EXPERT. Thus it would have fit into the same logic 
> and gathering the whole into a single patch would have made sense 
> (although that's debatable, I would have cut that personally, but 
> we don't care at that level).

Hm, that's my fault - i missed that.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06  1:23   ` [PATCHv2] " Josh Triplett
@ 2011-06-06 22:06     ` tip-bot for Josh Triplett
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Josh Triplett @ 2011-06-06 22:06 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, torvalds, sam, fweisbec, tglx, josh, mingo

Commit-ID:  f505c553dbe24b18a8590eb0eb5890a839acd0c3
Gitweb:     http://git.kernel.org/tip/f505c553dbe24b18a8590eb0eb5890a839acd0c3
Author:     Josh Triplett <josh@joshtriplett.org>
AuthorDate: Sun, 5 Jun 2011 18:23:58 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 7 Jun 2011 00:05:15 +0200

debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options

Several debugging options currently default to y, such as
CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users
might want to turn those options off to save space; however,
turning them off requires turning on CONFIG_DEBUG_KERNEL to
unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
unhide debugging options, and CONFIG_EXPERT exists specifically
to unhide options potentially needed by experts and/or embedded
users, make CONFIG_EXPERT automatically imply
CONFIG_DEBUG_KERNEL.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20110606012358.GA1909@leaf
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 init/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index ebafac4..14370d3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -909,6 +909,8 @@ config ANON_INODES
 
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
           to be disabled or tweaked. This is for specialized

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCHv4] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 18:39       ` Josh Triplett
  2011-06-06 21:34         ` Frederic Weisbecker
@ 2011-06-06 22:08         ` Josh Triplett
  2011-06-06 23:48           ` Frederic Weisbecker
  1 sibling, 1 reply; 18+ messages in thread
From: Josh Triplett @ 2011-06-06 22:08 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

Several debugging options currently default to y, such as
CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
want to turn those options off to save space; however, turning them off
requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
CONFIG_EXPERT exists specifically to unhide options potentially needed
by experts and/or embedded users, make CONFIG_EXPERT automatically imply
CONFIG_DEBUG_KERNEL.

Since EXPERT now implies DEBUG_KERNEL, change debugging options that
reference DEBUG_KERNEL && EXPERT to just reference EXPERT.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v4: Avoid semantic changes, by changing DEBUG_KERNEL && EXPERT to EXPERT
rather than to DEBUG_KERNEL.  This avoids exposing debug options that
only experts should turn off, such as DEBUG_BUGVERBOSE.

A subsequent patch should probably go through and change the various
"default !EXPERT" lines to "default y", since enabling CONFIG_EXPERT
should not directly change anything.

 arch/tile/Kconfig.debug |    2 +-
 init/Kconfig            |    2 ++
 lib/Kconfig.debug       |    2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/tile/Kconfig.debug b/arch/tile/Kconfig.debug
index ddbfc33..1d31246 100644
--- a/arch/tile/Kconfig.debug
+++ b/arch/tile/Kconfig.debug
@@ -3,7 +3,7 @@ menu "Kernel hacking"
 source "lib/Kconfig.debug"
 
 config EARLY_PRINTK
-	bool "Early printk" if EXPERT && DEBUG_KERNEL
+	bool "Early printk" if EXPERT
 	default y
 	help
 	  Write kernel log output directly via the hypervisor console.
diff --git a/init/Kconfig b/init/Kconfig
index ebafac4..14370d3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -909,6 +909,8 @@ config ANON_INODES
 
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
           to be disabled or tweaked. This is for specialized
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dd373c8..9a00361 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -694,7 +694,7 @@ config DEBUG_HIGHMEM
 	  Disable for production systems.
 
 config DEBUG_BUGVERBOSE
-	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
+	bool "Verbose BUG() reporting (adds 70K)" if EXPERT
 	depends on BUG
 	depends on ARM || AVR32 || M32R || M68K || SPARC32 || SPARC64 || \
 		   FRV || SUPERH || GENERIC_BUG || BLACKFIN || MN10300 || TILE
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 21:45       ` Frederic Weisbecker
@ 2011-06-06 22:09         ` Josh Triplett
  2011-06-07 10:18           ` Ingo Molnar
  0 siblings, 1 reply; 18+ messages in thread
From: Josh Triplett @ 2011-06-06 22:09 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: mingo, hpa, linux-kernel, sam, torvalds, tglx, mingo, linux-tip-commits

On Mon, Jun 06, 2011 at 11:45:04PM +0200, Frederic Weisbecker wrote:
> On Mon, Jun 06, 2011 at 09:39:40PM +0000, tip-bot for Josh Triplett wrote:
> > Commit-ID:  2458073da04571182748aa4289630dca52ba63cc
> > Gitweb:     http://git.kernel.org/tip/2458073da04571182748aa4289630dca52ba63cc
> > Author:     Josh Triplett <josh@joshtriplett.org>
> > AuthorDate: Mon, 6 Jun 2011 09:51:30 -0700
> > Committer:  Ingo Molnar <mingo@elte.hu>
> > CommitDate: Mon, 6 Jun 2011 19:30:01 +0200
> > 
> > debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
> > 
> > Several debugging options currently default to y, such as
> > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.
> > 
> > Embedded users might want to turn those options off to save space;
> > however, turning them off requires turning on CONFIG_DEBUG_KERNEL to
> > unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
> > unhide debugging options, and CONFIG_EXPERT exists specifically
> > to unhide options potentially needed by experts and/or embedded
> > users, make CONFIG_EXPERT automatically imply CONFIG_DEBUG_KERNEL.
> > 
> > Change various debugging options to only reference DEBUG_KERNEL,
> > not EXPERT.
> > 
> > Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed
> > wrong, since EXPERT should not directly affect anything except
> > the availability of other options.  (And EXPERT definitely
> > shouldn't mean "implicitly turn off default safety features".)
> > 
> > Change it to default to y, which means that turning on EXPERT
> > does not automatically disable it, but will provide the option
> > to disable it.
> > 
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
> No, Nacked actually. I'm fine with the v2 that only changed
> the EXPERT config to imply DEBUG_KERNEL. But all the other
> changes here need to be revised, as I outlined in the thread.

Fixed in v4; please replace the patch in tip with that.

- Josh Triplett

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv4] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 22:08         ` [PATCHv4] " Josh Triplett
@ 2011-06-06 23:48           ` Frederic Weisbecker
  0 siblings, 0 replies; 18+ messages in thread
From: Frederic Weisbecker @ 2011-06-06 23:48 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Thomas Gleixner, Sam Ravnborg, Andrew Morton,
	Linus Torvalds, linux-kernel

On Mon, Jun 06, 2011 at 03:08:10PM -0700, Josh Triplett wrote:
> Several debugging options currently default to y, such as
> CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.  Embedded users might
> want to turn those options off to save space; however, turning them off
> requires turning on CONFIG_DEBUG_KERNEL to unhide them.  Since
> CONFIG_DEBUG_KERNEL exists specifically to unhide debugging options, and
> CONFIG_EXPERT exists specifically to unhide options potentially needed
> by experts and/or embedded users, make CONFIG_EXPERT automatically imply
> CONFIG_DEBUG_KERNEL.
> 
> Since EXPERT now implies DEBUG_KERNEL, change debugging options that
> reference DEBUG_KERNEL && EXPERT to just reference EXPERT.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

Ingo, is it possible to zap the old one from the tree and apply this one
instead?

Or may be Josh can send an incremental patch instead (I just feel bad in advance to
ask him for a v5 :)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-06 22:09         ` Josh Triplett
@ 2011-06-07 10:18           ` Ingo Molnar
  2011-06-07 15:34             ` Josh Triplett
  0 siblings, 1 reply; 18+ messages in thread
From: Ingo Molnar @ 2011-06-07 10:18 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Frederic Weisbecker, mingo, hpa, linux-kernel, sam, torvalds,
	tglx, linux-tip-commits


* Josh Triplett <josh@joshtriplett.org> wrote:

> On Mon, Jun 06, 2011 at 11:45:04PM +0200, Frederic Weisbecker wrote:
> > On Mon, Jun 06, 2011 at 09:39:40PM +0000, tip-bot for Josh Triplett wrote:
> > > Commit-ID:  2458073da04571182748aa4289630dca52ba63cc
> > > Gitweb:     http://git.kernel.org/tip/2458073da04571182748aa4289630dca52ba63cc
> > > Author:     Josh Triplett <josh@joshtriplett.org>
> > > AuthorDate: Mon, 6 Jun 2011 09:51:30 -0700
> > > Committer:  Ingo Molnar <mingo@elte.hu>
> > > CommitDate: Mon, 6 Jun 2011 19:30:01 +0200
> > > 
> > > debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
> > > 
> > > Several debugging options currently default to y, such as
> > > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.
> > > 
> > > Embedded users might want to turn those options off to save space;
> > > however, turning them off requires turning on CONFIG_DEBUG_KERNEL to
> > > unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
> > > unhide debugging options, and CONFIG_EXPERT exists specifically
> > > to unhide options potentially needed by experts and/or embedded
> > > users, make CONFIG_EXPERT automatically imply CONFIG_DEBUG_KERNEL.
> > > 
> > > Change various debugging options to only reference DEBUG_KERNEL,
> > > not EXPERT.
> > > 
> > > Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed
> > > wrong, since EXPERT should not directly affect anything except
> > > the availability of other options.  (And EXPERT definitely
> > > shouldn't mean "implicitly turn off default safety features".)
> > > 
> > > Change it to default to y, which means that turning on EXPERT
> > > does not automatically disable it, but will provide the option
> > > to disable it.
> > > 
> > > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > > Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> > 
> > No, Nacked actually. I'm fine with the v2 that only changed
> > the EXPERT config to imply DEBUG_KERNEL. But all the other
> > changes here need to be revised, as I outlined in the thread.
> 
> Fixed in v4; please replace the patch in tip with that.

i applied the much simpler v2 twoliner version instead.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-07 10:18           ` Ingo Molnar
@ 2011-06-07 15:34             ` Josh Triplett
  2011-06-07 18:04               ` Ingo Molnar
  0 siblings, 1 reply; 18+ messages in thread
From: Josh Triplett @ 2011-06-07 15:34 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Frederic Weisbecker, mingo, hpa, linux-kernel, sam, torvalds,
	tglx, linux-tip-commits

On Tue, Jun 07, 2011 at 12:18:51PM +0200, Ingo Molnar wrote:
> 
> * Josh Triplett <josh@joshtriplett.org> wrote:
> 
> > On Mon, Jun 06, 2011 at 11:45:04PM +0200, Frederic Weisbecker wrote:
> > > On Mon, Jun 06, 2011 at 09:39:40PM +0000, tip-bot for Josh Triplett wrote:
> > > > Commit-ID:  2458073da04571182748aa4289630dca52ba63cc
> > > > Gitweb:     http://git.kernel.org/tip/2458073da04571182748aa4289630dca52ba63cc
> > > > Author:     Josh Triplett <josh@joshtriplett.org>
> > > > AuthorDate: Mon, 6 Jun 2011 09:51:30 -0700
> > > > Committer:  Ingo Molnar <mingo@elte.hu>
> > > > CommitDate: Mon, 6 Jun 2011 19:30:01 +0200
> > > > 
> > > > debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
> > > > 
> > > > Several debugging options currently default to y, such as
> > > > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.
> > > > 
> > > > Embedded users might want to turn those options off to save space;
> > > > however, turning them off requires turning on CONFIG_DEBUG_KERNEL to
> > > > unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
> > > > unhide debugging options, and CONFIG_EXPERT exists specifically
> > > > to unhide options potentially needed by experts and/or embedded
> > > > users, make CONFIG_EXPERT automatically imply CONFIG_DEBUG_KERNEL.
> > > > 
> > > > Change various debugging options to only reference DEBUG_KERNEL,
> > > > not EXPERT.
> > > > 
> > > > Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed
> > > > wrong, since EXPERT should not directly affect anything except
> > > > the availability of other options.  (And EXPERT definitely
> > > > shouldn't mean "implicitly turn off default safety features".)
> > > > 
> > > > Change it to default to y, which means that turning on EXPERT
> > > > does not automatically disable it, but will provide the option
> > > > to disable it.
> > > > 
> > > > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > > > Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> > > 
> > > No, Nacked actually. I'm fine with the v2 that only changed
> > > the EXPERT config to imply DEBUG_KERNEL. But all the other
> > > changes here need to be revised, as I outlined in the thread.
> > 
> > Fixed in v4; please replace the patch in tip with that.
> 
> i applied the much simpler v2 twoliner version instead.

OK...

So, should I prepare a separate patch which changes the instances of
DEBUG_KERNEL && EXPERT, or just leave them?

- Josh Triplett

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [tip:core/debug] debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
  2011-06-07 15:34             ` Josh Triplett
@ 2011-06-07 18:04               ` Ingo Molnar
  0 siblings, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2011-06-07 18:04 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Frederic Weisbecker, mingo, hpa, linux-kernel, sam, torvalds,
	tglx, linux-tip-commits


* Josh Triplett <josh@joshtriplett.org> wrote:

> On Tue, Jun 07, 2011 at 12:18:51PM +0200, Ingo Molnar wrote:
> > 
> > * Josh Triplett <josh@joshtriplett.org> wrote:
> > 
> > > On Mon, Jun 06, 2011 at 11:45:04PM +0200, Frederic Weisbecker wrote:
> > > > On Mon, Jun 06, 2011 at 09:39:40PM +0000, tip-bot for Josh Triplett wrote:
> > > > > Commit-ID:  2458073da04571182748aa4289630dca52ba63cc
> > > > > Gitweb:     http://git.kernel.org/tip/2458073da04571182748aa4289630dca52ba63cc
> > > > > Author:     Josh Triplett <josh@joshtriplett.org>
> > > > > AuthorDate: Mon, 6 Jun 2011 09:51:30 -0700
> > > > > Committer:  Ingo Molnar <mingo@elte.hu>
> > > > > CommitDate: Mon, 6 Jun 2011 19:30:01 +0200
> > > > > 
> > > > > debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options
> > > > > 
> > > > > Several debugging options currently default to y, such as
> > > > > CONFIG_DEBUG_BUGVERBOSE and CONFIG_DEBUG_RODATA.
> > > > > 
> > > > > Embedded users might want to turn those options off to save space;
> > > > > however, turning them off requires turning on CONFIG_DEBUG_KERNEL to
> > > > > unhide them.  Since CONFIG_DEBUG_KERNEL exists specifically to
> > > > > unhide debugging options, and CONFIG_EXPERT exists specifically
> > > > > to unhide options potentially needed by experts and/or embedded
> > > > > users, make CONFIG_EXPERT automatically imply CONFIG_DEBUG_KERNEL.
> > > > > 
> > > > > Change various debugging options to only reference DEBUG_KERNEL,
> > > > > not EXPERT.
> > > > > 
> > > > > Note that DEBUG_MEMORY_INIT defaulted to !EXPERT, which seemed
> > > > > wrong, since EXPERT should not directly affect anything except
> > > > > the availability of other options.  (And EXPERT definitely
> > > > > shouldn't mean "implicitly turn off default safety features".)
> > > > > 
> > > > > Change it to default to y, which means that turning on EXPERT
> > > > > does not automatically disable it, but will provide the option
> > > > > to disable it.
> > > > > 
> > > > > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > > > > Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> > > > 
> > > > No, Nacked actually. I'm fine with the v2 that only changed
> > > > the EXPERT config to imply DEBUG_KERNEL. But all the other
> > > > changes here need to be revised, as I outlined in the thread.
> > > 
> > > Fixed in v4; please replace the patch in tip with that.
> > 
> > i applied the much simpler v2 twoliner version instead.
> 
> OK...
> 
> So, should I prepare a separate patch which changes the instances 
> of DEBUG_KERNEL && EXPERT, or just leave them?

Yeah, would be nice to send a separate patch for that - 'DEBUG_KERNEL 
&& EXPERT' is equivalent to EXPERT now, right?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2011-06-07 18:04 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-05 21:02 [PATCH] Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options Josh Triplett
2011-06-05 21:31 ` Frederic Weisbecker
2011-06-06  1:23   ` [PATCHv2] " Josh Triplett
2011-06-06 22:06     ` [tip:core/debug] debug: " tip-bot for Josh Triplett
2011-06-06 15:18 ` [PATCH] " Ingo Molnar
2011-06-06 16:51   ` [PATCHv3] " Josh Triplett
2011-06-06 18:16     ` Frederic Weisbecker
2011-06-06 18:39       ` Josh Triplett
2011-06-06 21:34         ` Frederic Weisbecker
2011-06-06 22:02           ` Ingo Molnar
2011-06-06 22:08         ` [PATCHv4] " Josh Triplett
2011-06-06 23:48           ` Frederic Weisbecker
2011-06-06 21:39     ` [tip:core/debug] debug: " tip-bot for Josh Triplett
2011-06-06 21:45       ` Frederic Weisbecker
2011-06-06 22:09         ` Josh Triplett
2011-06-07 10:18           ` Ingo Molnar
2011-06-07 15:34             ` Josh Triplett
2011-06-07 18:04               ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).