Messages in this thread | | | Date | Wed, 12 Oct 2011 10:38:01 +0200 | From | Ingo Molnar <> | Subject | Re: Please revert "debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options" |
| |
* Adrian Bunk <bunk@stusta.de> wrote:
> On Mon, Oct 10, 2011 at 12:21:21PM +0200, Ingo Molnar wrote: > > > > * Adrian Bunk <bunk@stusta.de> wrote: > > > > > On Mon, Oct 10, 2011 at 09:29:48AM +0200, Ingo Molnar wrote: > > > > * Adrian Bunk <bunk@stusta.de> wrote: > > > >... > > > > I think you are wrong not just about that detail but about the whole > > > > premise of your complaint as well: > > > > > > > > > config DEBUG_BUGVERBOSE > > > > > - bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT > > > > > + bool "Verbose BUG() reporting (adds 70K)" if EXPERT > > > > > > > > > > This part of the patch would have been the correct and complete > > > > > solution for DEBUG_BUGVERBOSE. > > > > > > > > Not really - it's a debugging-only option and when i turn on > > > > CONFIG_DEBUG_KERNEL I expect to have full config control over all > > > > debug options - with sane defaults provided. > > > > > > Then you would have to remove the dependency on EXPERT from the prompt, > > > and allow unsetting DEBUG_BUGVERBOSE with EXPERT=n, DEBUG_KERNEL=y. > > > > > > Note that this is completely unrelated to the commit we are discussing, > > > since commit f505c553 has no effect in the EXPERT=n case you are > > > discussing here. > > > > > > > I definitely don't want a debugging option to depend on > > > > CONFIG_EXPERT. > > > > > > DEBUG_BUGVERBOSE does not depend on EXPERT. > > > > > > But EXPERT is currently required for disabling it. > > > > Correct - that's a further variation. In the case of debug options > > that we *really* don't want normal users to disable we do something > > like this: > > > > config DEBUG_BUGVERBOSE > > bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT > > > > Commit f505c553 ("debug: Make CONFIG_EXPERT select > > CONFIG_DEBUG_KERNEL to unhide debug options") allows this line to be > > further simplified into: > > > > bool "Verbose BUG() reporting (adds 70K)" if EXPERT > > > > ... but this was not the main purpose of the commit - nor is this > > simplification strictly necessary. > > You do not need commit f505c553 for that, the dependency of this > prompt on DEBUG_KERNEL should be removed in any case.
An opt-out model is more maintainable here than an opt-in method. People will get debug dependencies right most of the time - but getting debug *and* CONFIG_EXPERT interactions right is on the backburner generally.
So we are better off if CONFIG_EXPERT simply implies (selects) CONFIG_KERNEL_DEBUG - makes CONFIG_EXPERT an invariant as far as debugging features are concerned and reduces/eliminates the trickle of avoidable CONFIG_EXPERT tweaking patches in lib/Kconfig.debug.
> Why do you want to make life harder for people with EXPERT=y by not > allowing them to turn this off if they want to?
It's simpler to have one flat CONFIG_EXPERT=y option to gain broad expert-configurability of core debug functionality of the kernel.
It should arguably not explode the options to *all* drivers of the kernel:
> When configuring his kernel, a user set MISC_FILESYSTEMS=n. > > Now he sets EXPERT=y and runs "make oldconfig". > > Why would it make sense that he is now asked for each of these > filesystems whether he wants to enable it?
I agree with you that filesystems are more like drivers here and should probably not be selected by CONFIG_EXPERT. It's up to the VFS folks whether they consider experts to be frequent requestors. (probably not)
But the important case here is the situation outlined in the changelog, that a default-y core kernel option such as BUGVERBOSE unconditionally *adds* code. Core kernel debugging code is an area that by its nature has and is bound to have such options - MISC_FILESYSTEMS probably not.
Thanks,
Ingo
| |