Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n

PATCH In review Josh Triplett 2 participants 5 messages Jun 5, 2011 Library code

Share & tools

Josh Triplett

author

2011-06-05 08:32 UTC #0 lore raw

PATCH 1/1 Unhide DEBUG_BUGVERBOSE when EXPERT=y, even if DEBUG_KERNEL=n

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 <[email protected]>

---

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(-)

lib/Kconfig.debug +1 -1

index dd373c8..1edc75d 100644
@@ -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

Signature · 1 line

1.7.5.3

Ingo Molnar

2011-06-05 09:35 UTC #1 lore raw

lib/Kconfig.debug config DEBUG_HIGHMEM · line 697

Quoted 3 lines · Josh Triplett

config DEBUG_BUGVERBOSE

lib/Kconfig.debug +1 -1

-	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

Josh Triplett

author

2011-06-05 17:06 UTC #2 lore raw

Quoted 11 lines · Ingo Molnar

lib/Kconfig.debug config DEBUG_HIGHMEM · line 697
Quoted 3 lines · Josh Triplett
config DEBUG_BUGVERBOSE
lib/Kconfig.debug +1 -1
-	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

Quoted 5 lines

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

Ingo Molnar

2011-06-05 17:32 UTC #3 lore raw

Quoted 18 lines · Josh Triplett

Quoted 11 lines · Ingo Molnar
lib/Kconfig.debug config DEBUG_HIGHMEM · line 697
Quoted 3 lines · Josh Triplett
config DEBUG_BUGVERBOSE
lib/Kconfig.debug +1 -1
-	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.

Quoted 12 lines

~/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.

Quoted 4 lines

Quoted 4 lines
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.

Quoted 8 lines

Quoted 1 line
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

Josh Triplett

author

2011-06-05 20:24 UTC #4 lore raw

Quoted 31 lines · Ingo Molnar

Quoted 17 lines · Josh Triplett
Quoted 10 lines · Ingo Molnar
lib/Kconfig.debug config DEBUG_HIGHMEM · line 697
Quoted 3 lines · Josh Triplett
config DEBUG_BUGVERBOSE
lib/Kconfig.debug +1 -1
-	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.
Quoted 12 lines
~/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

Quoted 2 lines

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.

Quoted 13 lines

Quoted 4 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 1 line
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.

Quoted 3 lines

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