public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n
@ 2011-06-05  8:32 Josh Triplett
  2011-06-05  9:34 ` Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Josh Triplett @ 2011-06-05  8:32 UTC (permalink / raw)
  To: Ingo Molnar, Sam Ravnborg, Jeremy Fitzhardinge, Andrew Morton,
	Linus Torvalds, linux-kernel

CONFIG_DEBUG_BUGVERBOSE defaults to y.  Embedded systems might want to
disable this to save space; however, disabling it requires enabling
CONFIG_DEBUG_KERNEL to see it.  Unhide it for CONFIG_EXPERT=y as well,
so it shows up as an option for embedded users without having to turn on
CONFIG_DEBUG_KERNEL.

Also make the option show up if CONFIG_DEBUG_KERNEL=y and
CONFIG_EXPERT=n, which seems sensible for a debug feature.

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

To me, it seemed highly counterintuitive to have to enable
CONFIG_DEBUG_KERNEL in order to *disable* debugging features; hence this
patch to expose one of the two that defaults to y.

 lib/Kconfig.debug |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index dd373c8..1edc75d 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 || 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] 5+ messages in thread

* Re: [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n
  2011-06-05  8:32 [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n Josh Triplett
@ 2011-06-05  9:34 ` Ingo Molnar
  2011-06-05 17:06   ` Josh Triplett
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2011-06-05  9:34 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Sam Ravnborg, Jeremy Fitzhardinge, Andrew Morton, Linus Torvalds,
	linux-kernel


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

>  config DEBUG_BUGVERBOSE
> -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL || EXPERT

Well, DEBUG_KERNEL really means two things:

  - make more debugging options available
  - allow the *disabling* of existing (default-enabled) debug options

So i think the right solution would be to select DEBUG_KERNEL if 
EXPERT is enabled - this would simplify things and would allow the 
removal of a lot of EXPERT conditions from the debug options.

Ok?

Thanks,

	Ingo

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

* Re: [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n
  2011-06-05  9:34 ` Ingo Molnar
@ 2011-06-05 17:06   ` Josh Triplett
  2011-06-05 17:31     ` Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Josh Triplett @ 2011-06-05 17:06 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Sam Ravnborg, Jeremy Fitzhardinge, Andrew Morton, Linus Torvalds,
	linux-kernel

On Sun, Jun 05, 2011 at 11:34:45AM +0200, Ingo Molnar wrote:
> 
> * Josh Triplett <josh@joshtriplett.org> wrote:
> 
> >  config DEBUG_BUGVERBOSE
> > -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> > +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL || EXPERT
> 
> Well, DEBUG_KERNEL really means two things:
> 
>   - make more debugging options available
>   - allow the *disabling* of existing (default-enabled) debug options

As well as one more: a quick "git grep DEBUG_KERNEL" turns up a few uses
in actual kernel source code, to control debugging features.  Those
should likely use separately selectable debug options, but they
currently don't.

~/src/linux-2.6$ find * -not -name 'Kconfig*' -not -name '*defconfig' | xargs grep -n DEBUG_KERNEL
arch/parisc/mm/init.c:653:#ifdef CONFIG_DEBUG_KERNEL /* double-sanity-check paranoia */
arch/powerpc/kernel/sysfs.c:229:#ifdef CONFIG_DEBUG_KERNEL
arch/powerpc/kernel/sysfs.c:258:#endif /* CONFIG_DEBUG_KERNEL */
arch/powerpc/kernel/sysfs.c:299:#ifdef CONFIG_DEBUG_KERNEL
arch/powerpc/kernel/sysfs.c:328:#endif /* CONFIG_DEBUG_KERNEL */
arch/blackfin/include/asm/entry.h:53:/* As a debugging aid - we save IPEND when DEBUG_KERNEL is on,
arch/blackfin/include/asm/entry.h:56:# ifndef CONFIG_DEBUG_KERNEL
arch/blackfin/include/asm/entry.h:65:# else /* CONFIG_DEBUG_KERNEL */
arch/blackfin/include/asm/entry.h:77:# endif /* CONFIG_DEBUG_KERNEL */
arch/blackfin/include/asm/context.S:208:#ifdef CONFIG_DEBUG_KERNEL
drivers/usb/musb/musb_core.c:557:                * REVISIT:  do delays from lots of DEBUG_KERNEL checks

> So i think the right solution would be to select DEBUG_KERNEL if 
> EXPERT is enabled - this would simplify things and would allow the 
> removal of a lot of EXPERT conditions from the debug options.
> 
> Ok?

I could live with that, as long as CONFIG_DEBUG_KERNEL never directly
enables any debugging code like it does above, and just acts like
CONFIG_EXPERT in hiding a pile of unnecessary options.  DEBUG_KERNEL
should probably also have some text saying it doesn't actually enable
any kernel debugging on its own, once that becomes true.

- Josh Triplett

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

* Re: [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n
  2011-06-05 17:06   ` Josh Triplett
@ 2011-06-05 17:31     ` Ingo Molnar
  2011-06-05 20:24       ` Josh Triplett
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2011-06-05 17:31 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Sam Ravnborg, Jeremy Fitzhardinge, Andrew Morton, Linus Torvalds,
	linux-kernel


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

> On Sun, Jun 05, 2011 at 11:34:45AM +0200, Ingo Molnar wrote:
> > 
> > * Josh Triplett <josh@joshtriplett.org> wrote:
> > 
> > >  config DEBUG_BUGVERBOSE
> > > -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> > > +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL || EXPERT
> > 
> > Well, DEBUG_KERNEL really means two things:
> > 
> >   - make more debugging options available
> >   - allow the *disabling* of existing (default-enabled) debug options
> 
> As well as one more: a quick "git grep DEBUG_KERNEL" turns up a few uses
> in actual kernel source code, to control debugging features.  Those
> should likely use separately selectable debug options, but they
> currently don't.
> 
> ~/src/linux-2.6$ find * -not -name 'Kconfig*' -not -name '*defconfig' | xargs grep -n DEBUG_KERNEL
> arch/parisc/mm/init.c:653:#ifdef CONFIG_DEBUG_KERNEL /* double-sanity-check paranoia */
> arch/powerpc/kernel/sysfs.c:229:#ifdef CONFIG_DEBUG_KERNEL
> arch/powerpc/kernel/sysfs.c:258:#endif /* CONFIG_DEBUG_KERNEL */
> arch/powerpc/kernel/sysfs.c:299:#ifdef CONFIG_DEBUG_KERNEL
> arch/powerpc/kernel/sysfs.c:328:#endif /* CONFIG_DEBUG_KERNEL */
> arch/blackfin/include/asm/entry.h:53:/* As a debugging aid - we save IPEND when DEBUG_KERNEL is on,
> arch/blackfin/include/asm/entry.h:56:# ifndef CONFIG_DEBUG_KERNEL
> arch/blackfin/include/asm/entry.h:65:# else /* CONFIG_DEBUG_KERNEL */
> arch/blackfin/include/asm/entry.h:77:# endif /* CONFIG_DEBUG_KERNEL */
> arch/blackfin/include/asm/context.S:208:#ifdef CONFIG_DEBUG_KERNEL
> drivers/usb/musb/musb_core.c:557:                * REVISIT:  do delays from lots of DEBUG_KERNEL checks

These are basically just 4 cases out of thousands of drivers, zero in 
essence. Also, none seems significant in terms of code size.

> > So i think the right solution would be to select DEBUG_KERNEL if 
> > EXPERT is enabled - this would simplify things and would allow the 
> > removal of a lot of EXPERT conditions from the debug options.
> > 
> > Ok?
> 
> I could live with that, as long as CONFIG_DEBUG_KERNEL never 
> directly enables any debugging code like it does above, and just 
> acts like CONFIG_EXPERT in hiding a pile of unnecessary options.  
> DEBUG_KERNEL should probably also have some text saying it doesn't 
> actually enable any kernel debugging on its own, once that becomes 
> true.

Yeah. I'd suggest you try that route instead of cluttering all of the 
debugging Kconfigs with || EXPERT conditions, which doesnt really 
look acceptable.

Thanks,

	Ingo

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

* Re: [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n
  2011-06-05 17:31     ` Ingo Molnar
@ 2011-06-05 20:24       ` Josh Triplett
  0 siblings, 0 replies; 5+ messages in thread
From: Josh Triplett @ 2011-06-05 20:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Sam Ravnborg, Jeremy Fitzhardinge, Andrew Morton, Linus Torvalds,
	linux-kernel

On Sun, Jun 05, 2011 at 07:31:43PM +0200, Ingo Molnar wrote:
> * Josh Triplett <josh@joshtriplett.org> wrote:
> > On Sun, Jun 05, 2011 at 11:34:45AM +0200, Ingo Molnar wrote:
> > > * Josh Triplett <josh@joshtriplett.org> wrote:
> > > 
> > > >  config DEBUG_BUGVERBOSE
> > > > -	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
> > > > +	bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL || EXPERT
> > > 
> > > Well, DEBUG_KERNEL really means two things:
> > > 
> > >   - make more debugging options available
> > >   - allow the *disabling* of existing (default-enabled) debug options
> > 
> > As well as one more: a quick "git grep DEBUG_KERNEL" turns up a few uses
> > in actual kernel source code, to control debugging features.  Those
> > should likely use separately selectable debug options, but they
> > currently don't.
> > 
> > ~/src/linux-2.6$ find * -not -name 'Kconfig*' -not -name '*defconfig' | xargs grep -n DEBUG_KERNEL
> > arch/parisc/mm/init.c:653:#ifdef CONFIG_DEBUG_KERNEL /* double-sanity-check paranoia */
> > arch/powerpc/kernel/sysfs.c:229:#ifdef CONFIG_DEBUG_KERNEL
> > arch/powerpc/kernel/sysfs.c:258:#endif /* CONFIG_DEBUG_KERNEL */
> > arch/powerpc/kernel/sysfs.c:299:#ifdef CONFIG_DEBUG_KERNEL
> > arch/powerpc/kernel/sysfs.c:328:#endif /* CONFIG_DEBUG_KERNEL */
> > arch/blackfin/include/asm/entry.h:53:/* As a debugging aid - we save IPEND when DEBUG_KERNEL is on,
> > arch/blackfin/include/asm/entry.h:56:# ifndef CONFIG_DEBUG_KERNEL
> > arch/blackfin/include/asm/entry.h:65:# else /* CONFIG_DEBUG_KERNEL */
> > arch/blackfin/include/asm/entry.h:77:# endif /* CONFIG_DEBUG_KERNEL */
> > arch/blackfin/include/asm/context.S:208:#ifdef CONFIG_DEBUG_KERNEL
> > drivers/usb/musb/musb_core.c:557:                * REVISIT:  do delays from lots of DEBUG_KERNEL checks
> 
> These are basically just 4 cases out of thousands of drivers, zero in 
> essence. Also, none seems significant in terms of code size.

Sure, but they still need fixing.

> > > So i think the right solution would be to select DEBUG_KERNEL if 
> > > EXPERT is enabled - this would simplify things and would allow the 
> > > removal of a lot of EXPERT conditions from the debug options.
> > > 
> > > Ok?
> > 
> > I could live with that, as long as CONFIG_DEBUG_KERNEL never 
> > directly enables any debugging code like it does above, and just 
> > acts like CONFIG_EXPERT in hiding a pile of unnecessary options.  
> > DEBUG_KERNEL should probably also have some text saying it doesn't 
> > actually enable any kernel debugging on its own, once that becomes 
> > true.
> 
> Yeah. I'd suggest you try that route instead of cluttering all of the 
> debugging Kconfigs with || EXPERT conditions, which doesnt really 
> look acceptable.

In fairness, I didn't plan to add it to more than two. :)

Nonetheless, patch shortly.

- Josh Triplett

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

end of thread, other threads:[~2011-06-05 20:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-05  8:32 [PATCH] Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n Josh Triplett
2011-06-05  9:34 ` Ingo Molnar
2011-06-05 17:06   ` Josh Triplett
2011-06-05 17:31     ` Ingo Molnar
2011-06-05 20:24       ` Josh Triplett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox