All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Bunk <bunk@stusta.de>
To: Ingo Molnar <mingo@elte.hu>
Cc: Josh Triplett <josh@joshtriplett.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>
Subject: Re: Please revert "debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options"
Date: Mon, 10 Oct 2011 11:48:11 +0300	[thread overview]
Message-ID: <20111010084811.GG4586@localhost.pp.htv.fi> (raw)
In-Reply-To: <20111010072946.GA29035@elte.hu>

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.

This part of our discussion boils down to the following options:
1. DEBUG_KERNEL allows a user to enable additional debugging
2. DEBUG_KERNEL gives a user full control over all debug options

Traditionally it was 1., to tell a user reporting a bug that he should 
enable additional debugging in his kernel - without him accidentally 
disabling useful debugging.

Again, this part of the discussion is unrelated to commit f505c553.

> CONFIG_EXPERT is a superset to all that: it implies config control 
> over all debug options *and* over many other kernel components as 
> well.

I don't see the advantage that EXPERT=y is now unconditionally enabling 
the DEBUG_KERNEL knob.

When I am an "expert", why can't I still decide myself globally if I 
want to enable more debugging or not?

Another problem is that we have code using "#ifdef CONFIG_DEBUG_KERNEL"
so enabling EXPERT for making your kernel smaller can make your kernel 
bigger, but such usages should anyway get own config options.

> This is a pretty easy model to think about.

There are many options in the kernel that only enable seeing other 
config options.

With that model, shouldn't EXPERT also have
	select MISC_FILESYSTEMS
and many other similar select's?

The point I am trying to make here is that there are many convenience 
config options that only exist to allow a user to hide a bunch of 
options he is not interested in, and DEBUG_KERNEL is one of them. [1]

> > The crazy select added in commit f505c553 is wrong.
> 
> Why? Your original message does not explain it.

It is not needed for what it was claimed it was needed for,
and it is a bad idea in general.

If I didn't explain that properly that was my fault,
I hope the explanation in this email is better.

> Thanks,
> 
> 	Ingo

cu
Adrian

[1] ignoring the buggy code using "#ifdef CONFIG_DEBUG_KERNEL"

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


  reply	other threads:[~2011-10-10  8:48 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-30 13:42 Please revert "debug: Make CONFIG_EXPERT select CONFIG_DEBUG_KERNEL to unhide debug options" Adrian Bunk
2011-09-30 15:05 ` Josh Triplett
2011-09-30 15:25   ` Frederic Weisbecker
2011-09-30 15:32     ` Frederic Weisbecker
2011-09-30 15:54     ` Adrian Bunk
2011-09-30 15:50   ` Adrian Bunk
2011-10-10  7:29     ` Ingo Molnar
2011-10-10  8:48       ` Adrian Bunk [this message]
2011-10-10  9:44         ` Mike Galbraith
2011-10-10 10:21         ` Ingo Molnar
2011-10-10 12:13           ` Adrian Bunk
2011-10-12  8:38             ` Ingo Molnar
2011-10-20 21:41               ` Adrian Bunk
2011-10-21  8:19                 ` Michal Marek
2011-10-21  9:22                   ` Adrian Bunk
2011-10-21 12:37                     ` Michal Marek
2011-10-21 16:12                       ` Adrian Bunk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111010084811.GG4586@localhost.pp.htv.fi \
    --to=bunk@stusta.de \
    --cc=fweisbec@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=sam@ravnborg.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.