We can't seem to have a kernel with CONFIG_EXPERT set but
CONFIG_DEBUG_KERNEL unset these days.
While some of the features under the CONFIG_EXPERT require
CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
It looks like CONFIG_KALLSYMS_ALL is the only feature that
requires CONFIG_DEBUG_KERNEL.
Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
still choose CONFIG_EXPERT without CONFIG_DEBUG.
Signed-off-by: Sinan Kaya <[email protected]>
---
init/Kconfig | 2 --
lib/Kconfig.debug | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index 4592bf7997c0..37e10a8391a3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1206,8 +1206,6 @@ config BPF
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 0d9e81779e37..9fbf3499ec8d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -434,6 +434,7 @@ config MAGIC_SYSRQ_SERIAL
config DEBUG_KERNEL
bool "Kernel debugging"
+ default EXPERT
help
Say Y here if you are developing drivers or trying to debug and
identify kernel problems.
--
2.21.0
On Wed, Apr 10, 2019 at 3:42 PM Sinan Kaya <[email protected]> wrote:
>
> We can't seem to have a kernel with CONFIG_EXPERT set but
> CONFIG_DEBUG_KERNEL unset these days.
>
> While some of the features under the CONFIG_EXPERT require
> CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
>
> It looks like CONFIG_KALLSYMS_ALL is the only feature that
> requires CONFIG_DEBUG_KERNEL.
>
> Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
Typo: CONFIG_DEBUG_KERNEL
> still choose CONFIG_EXPERT without CONFIG_DEBUG.
same.
>
> Signed-off-by: Sinan Kaya <[email protected]>
But with those fixed, looks good to me. Adding Josh (and others) to CC
since he originally added the linkage to EXPERT in commit
f505c553dbe2.
Reviewed-by: Kees Cook <[email protected]>
-Kees
> ---
> init/Kconfig | 2 --
> lib/Kconfig.debug | 1 +
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 4592bf7997c0..37e10a8391a3 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1206,8 +1206,6 @@ config BPF
>
> 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 0d9e81779e37..9fbf3499ec8d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -434,6 +434,7 @@ config MAGIC_SYSRQ_SERIAL
>
> config DEBUG_KERNEL
> bool "Kernel debugging"
> + default EXPERT
> help
> Say Y here if you are developing drivers or trying to debug and
> identify kernel problems.
> --
> 2.21.0
>
--
Kees Cook
On April 10, 2019 3:58:55 PM PDT, Kees Cook <[email protected]> wrote:
>On Wed, Apr 10, 2019 at 3:42 PM Sinan Kaya <[email protected]> wrote:
>>
>> We can't seem to have a kernel with CONFIG_EXPERT set but
>> CONFIG_DEBUG_KERNEL unset these days.
>>
>> While some of the features under the CONFIG_EXPERT require
>> CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
>>
>> It looks like CONFIG_KALLSYMS_ALL is the only feature that
>> requires CONFIG_DEBUG_KERNEL.
>>
>> Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
>
>Typo: CONFIG_DEBUG_KERNEL
>
>> still choose CONFIG_EXPERT without CONFIG_DEBUG.
>
>same.
>
>>
>> Signed-off-by: Sinan Kaya <[email protected]>
>
>But with those fixed, looks good to me. Adding Josh (and others) to CC
>since he originally added the linkage to EXPERT in commit
>f505c553dbe2.
CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it should only make more options appear in kconfig. I originally added this to ensure that features you might want to *disable* aren't hidden, as part of the tinification effort.
What specific problem does having CONFIG_DEBUG_KERNEL enabled cause for you? I'd still prefer to have a single switch for "don't hide things I might want to disable", rather than several.
This would also need checking to make sure it doesn't grow tinyconfig.
On Wed, Apr 10, 2019 at 4:22 PM Josh Triplett <[email protected]> wrote:
>
> On April 10, 2019 3:58:55 PM PDT, Kees Cook <[email protected]> wrote:
> >On Wed, Apr 10, 2019 at 3:42 PM Sinan Kaya <[email protected]> wrote:
> >>
> >> We can't seem to have a kernel with CONFIG_EXPERT set but
> >> CONFIG_DEBUG_KERNEL unset these days.
> >>
> >> While some of the features under the CONFIG_EXPERT require
> >> CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
> >>
> >> It looks like CONFIG_KALLSYMS_ALL is the only feature that
> >> requires CONFIG_DEBUG_KERNEL.
> >>
> >> Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
> >
> >Typo: CONFIG_DEBUG_KERNEL
> >
> >> still choose CONFIG_EXPERT without CONFIG_DEBUG.
> >
> >same.
> >
> >>
> >> Signed-off-by: Sinan Kaya <[email protected]>
> >
> >But with those fixed, looks good to me. Adding Josh (and others) to CC
> >since he originally added the linkage to EXPERT in commit
> >f505c553dbe2.
>
> CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it should only make more options appear in kconfig. I originally added this to ensure that features you might want to *disable* aren't hidden, as part of the tinification effort.
>
> What specific problem does having CONFIG_DEBUG_KERNEL enabled cause for you? I'd still prefer to have a single switch for "don't hide things I might want to disable", rather than several.
See earlier in the thread: code generation depends on
CONFIG_DEBUG_KERNEL now unfortunately.
--
Kees Cook
On April 10, 2019 4:24:18 PM PDT, Kees Cook <[email protected]> wrote:
>On Wed, Apr 10, 2019 at 4:22 PM Josh Triplett <[email protected]>
>wrote:
>>
>> On April 10, 2019 3:58:55 PM PDT, Kees Cook <[email protected]>
>wrote:
>> >On Wed, Apr 10, 2019 at 3:42 PM Sinan Kaya <[email protected]> wrote:
>> >>
>> >> We can't seem to have a kernel with CONFIG_EXPERT set but
>> >> CONFIG_DEBUG_KERNEL unset these days.
>> >>
>> >> While some of the features under the CONFIG_EXPERT require
>> >> CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
>> >>
>> >> It looks like CONFIG_KALLSYMS_ALL is the only feature that
>> >> requires CONFIG_DEBUG_KERNEL.
>> >>
>> >> Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
>> >
>> >Typo: CONFIG_DEBUG_KERNEL
>> >
>> >> still choose CONFIG_EXPERT without CONFIG_DEBUG.
>> >
>> >same.
>> >
>> >>
>> >> Signed-off-by: Sinan Kaya <[email protected]>
>> >
>> >But with those fixed, looks good to me. Adding Josh (and others) to
>CC
>> >since he originally added the linkage to EXPERT in commit
>> >f505c553dbe2.
>>
>> CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it
>should only make more options appear in kconfig. I originally added
>this to ensure that features you might want to *disable* aren't hidden,
>as part of the tinification effort.
>>
>> What specific problem does having CONFIG_DEBUG_KERNEL enabled cause
>for you? I'd still prefer to have a single switch for "don't hide
>things I might want to disable", rather than several.
>
>See earlier in the thread: code generation depends on
>CONFIG_DEBUG_KERNEL now unfortunately.
Then let's fix *that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this.
On 4/10/2019 11:02 PM, Josh Triplett wrote:
> Then let's fix*that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this.
I think we have to do both. We need to go after the users as well as
solve the immediate problem per this patch.
As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the
place and getting subsystem owners to remove let alone add a check
to checkpatch is just going to take time.
Please let us know if you are OK with this plan.
On 4/10/19 8:02 PM, Josh Triplett wrote:
> On April 10, 2019 4:24:18 PM PDT, Kees Cook <[email protected]> wrote:
>> On Wed, Apr 10, 2019 at 4:22 PM Josh Triplett <[email protected]>
>> wrote:
>>>
>>> On April 10, 2019 3:58:55 PM PDT, Kees Cook <[email protected]>
>> wrote:
>>>> On Wed, Apr 10, 2019 at 3:42 PM Sinan Kaya <[email protected]> wrote:
>>>>>
>>>>> We can't seem to have a kernel with CONFIG_EXPERT set but
>>>>> CONFIG_DEBUG_KERNEL unset these days.
>>>>>
>>>>> While some of the features under the CONFIG_EXPERT require
>>>>> CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
>>>>>
>>>>> It looks like CONFIG_KALLSYMS_ALL is the only feature that
>>>>> requires CONFIG_DEBUG_KERNEL.
>>>>>
>>>>> Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
>>>>
>>>> Typo: CONFIG_DEBUG_KERNEL
>>>>
>>>>> still choose CONFIG_EXPERT without CONFIG_DEBUG.
>>>>
>>>> same.
>>>>
>>>>>
>>>>> Signed-off-by: Sinan Kaya <[email protected]>
>>>>
>>>> But with those fixed, looks good to me. Adding Josh (and others) to
>> CC
>>>> since he originally added the linkage to EXPERT in commit
>>>> f505c553dbe2.
>>>
>>> CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it
>> should only make more options appear in kconfig. I originally added
>> this to ensure that features you might want to *disable* aren't hidden,
>> as part of the tinification effort.
>>>
>>> What specific problem does having CONFIG_DEBUG_KERNEL enabled cause
>> for you? I'd still prefer to have a single switch for "don't hide
>> things I might want to disable", rather than several.
>>
>> See earlier in the thread: code generation depends on
>> CONFIG_DEBUG_KERNEL now unfortunately.
>
> Then let's fix *that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this.
>
checkpatch is not an enforcer. It takes maintainers to do that.
--
~Randy
On Wed, Apr 10, 2019 at 11:13:52PM -0400, Sinan Kaya wrote:
> On 4/10/2019 11:02 PM, Josh Triplett wrote:
> > Then let's fix*that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this.
>
> I think we have to do both. We need to go after the users as well as
> solve the immediate problem per this patch.
>
> As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the
> place and getting subsystem owners to remove let alone add a check
> to checkpatch is just going to take time.
>
> Please let us know if you are OK with this plan.
I'm not OK with this plan. Turning on EXPERT should make the options
under DEBUG_KERNEL visible; it's a bug that DEBUG_KERNEL affects code
generation as well.
Proposed alternative plan: let's add a new symbol, something like
DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".
Does that sound like an appropriately rapid solution for this bug?
On Thu, Apr 11, 2019 at 3:16 PM Josh Triplett <[email protected]> wrote:
>
> On Wed, Apr 10, 2019 at 11:13:52PM -0400, Sinan Kaya wrote:
> > On 4/10/2019 11:02 PM, Josh Triplett wrote:
> > > Then let's fix*that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this.
> >
> > I think we have to do both. We need to go after the users as well as
> > solve the immediate problem per this patch.
> >
> > As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the
> > place and getting subsystem owners to remove let alone add a check
> > to checkpatch is just going to take time.
> >
> > Please let us know if you are OK with this plan.
>
> I'm not OK with this plan. Turning on EXPERT should make the options
> under DEBUG_KERNEL visible; it's a bug that DEBUG_KERNEL affects code
> generation as well.
>
> Proposed alternative plan: let's add a new symbol, something like
> DEBUG_MISC ("Miscellaneous debug code that should be under a more
> specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
> "default DEBUG_KERNEL" but allow itself to be turned off, and then
> mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
> "#ifdef CONFIG_DEBUG_MISC".
>
> Does that sound like an appropriately rapid solution for this bug?
Sure, that sounds fine to me. Sinan can you take care of that for v4?
--
Kees Cook
On 4/11/2019 6:21 PM, Kees Cook wrote:
>> Proposed alternative plan: let's add a new symbol, something like
>> DEBUG_MISC ("Miscellaneous debug code that should be under a more
>> specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
>> "default DEBUG_KERNEL" but allow itself to be turned off, and then
>> mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
>> "#ifdef CONFIG_DEBUG_MISC".
>>
>> Does that sound like an appropriately rapid solution for this bug?
> Sure, that sounds fine to me. Sinan can you take care of that for v4?
Sure, let me work on this.
On Thu, Apr 11, 2019 at 06:27:04PM -0400, Sinan Kaya wrote:
> On 4/11/2019 6:21 PM, Kees Cook wrote:
> > > Proposed alternative plan: let's add a new symbol, something like
> > > DEBUG_MISC ("Miscellaneous debug code that should be under a more
> > > specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
> > > "default DEBUG_KERNEL" but allow itself to be turned off, and then
> > > mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
> > > "#ifdef CONFIG_DEBUG_MISC".
> > >
> > > Does that sound like an appropriately rapid solution for this bug?
> > Sure, that sounds fine to me. Sinan can you take care of that for v4?
>
> Sure, let me work on this.
Thank you both! I really appreciate that.