2023-05-09 09:11:52

by Lukas Bulwahn

[permalink] [raw]
Subject: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
and just gives that intent a much better name. That has been clearly a good
and long overdue renaming, and it is clearly an improvement to the kernel
build configuration that has shown to help managing the kernel build
configuration in the last decade.

However, rather than bravely and radically just deleting CONFIG_EMBEDDED,
this commit gives CONFIG_EMBEDDED a new intended semantics, but keeps it
open for future contributors to implement that intended semantics:

A new CONFIG_EMBEDDED option is added that automatically selects
CONFIG_EXPERT when enabled and can be used in the future to isolate
options that should only be considered for embedded systems (RISC
architectures, SLOB, etc).

Since then, this CONFIG_EMBEDDED implicitly had two purposes:

- It can make even more options visible beyond what CONFIG_EXPERT makes
visible. In other words, it may introduce another level of enabling the
visibility of configuration options: always visible, visible with
CONFIG_EXPERT and visible with CONFIG_EMBEDDED.

- Set certain default values of some configurations differently,
following the assumption that configuring a kernel build for an
embedded system generally starts with a different set of default values
compared to kernel builds for all other kind of systems.

Considering the first purpose, at the point in time where CONFIG_EMBEDDED
was renamed to CONFIG_EXPERT, CONFIG_EXPERT already made 130 more options
become visible throughout all different menus for the kernel configuration.
Over the last decade, this has gradually increased, so that currently, with
CONFIG_EXPERT, roughly 170 more options become visible throughout all
different menus for the kernel configuration. In comparison, currently with
CONFIG_EMBEDDED enabled, just seven more options are visible, one in x86,
one in arm, and five for the ChipIdea Highspeed Dual Role Controller.

As the numbers suggest, these two levels of enabling the visibility of even
more configuration options---beyond what CONFIG_EXPERT enables---never
evolved to a good solution in the last decade. In other words, this
additional level of visibility of configuration option with CONFIG_EMBEDDED
compared to CONFIG_EXPERT has since its introduction never become really
valuable. It requires quite some investigation to actually understand what
is additionally visible and it does not differ significantly in complexity
compared to just enabling CONFIG_EXPERT. This CONFIG_EMBEDDED---or any
other config to show more detailed options beyond CONFIG_EXPERT---is
unlikely to be valuable unless somebody puts significant effort in
identifying how such visibility options can be properly split and creating
clear criteria, when some config option is visible with CONFIG_EXPERT and
when some config option is visible only with some further option enabled
beyond CONFIG_EXPERT, such as CONFIG_EMBEDDED attempted to do. For now, it
is much more reasonable to simply make those additional seven options that
are visible with CONFIG_EMBEDDED visible with CONFIG_EXPERT, and then
remove CONFIG_EMBEDDED. If anyone spends significant effort in structuring
the visibility of config options, they may re-introduce suitable new
config options simply as they see fit.

Make the config X86_FEATURE_NAMES visible when CONFIG_EXPERT is enabled.

Signed-off-by: Lukas Bulwahn <[email protected]>
Reviewed-by: Masahiro Yamada <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
---
arch/x86/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ce460d6b4e25..595f6696281c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -442,7 +442,7 @@ config SMP
If you don't know what to do here, say N.

config X86_FEATURE_NAMES
- bool "Processor feature human-readable names" if EMBEDDED
+ bool "Processor feature human-readable names" if EXPERT
default y
help
This option compiles in a table of x86 feature bits and corresponding
--
2.17.1


2023-05-09 14:08:44

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On 5/9/23 01:40, Lukas Bulwahn wrote:
> Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
> introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
> and just gives that intent a much better name.

That was quite the changelog, and I'm still not quite sure:

What is the problem with the existing code?

What is the end user visible impact of this problem and of this proposed
change?


2023-05-09 18:26:53

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On Tue, May 9, 2023 at 4:07 PM Dave Hansen <[email protected]> wrote:
>
> On 5/9/23 01:40, Lukas Bulwahn wrote:
> > Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
> > introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
> > and just gives that intent a much better name.
>
> That was quite the changelog, and I'm still not quite sure:
>
> What is the problem with the existing code?
>
> What is the end user visible impact of this problem and of this proposed
> change?
>

Thanks, Dave, for your feedback.

The commit message is so lengthy, as I copied it from the original
cover letter into each commit, but I agree with your feedback.

So maybe this commit message fits better:

To simplify build configurations, the CONFIG_EMBEDDED is to be
removed. All configs that are only visible with CONFIG_EMBEDDED=y
shall be now visible with CONFIG_EXPERT=y.

In the x86 architecture, the config X86_FEATURE_NAMES is visible when
CONFIG_EMBEDDED is enabled. Now, make the config X86_FEATURE_NAMES
visible when CONFIG_EXPERT is enabled.


Dave, what do you think? If that is good enough for you, I will send
an updated patch with that commit message.

Lukas

2023-05-09 18:57:51

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On 5/9/23 11:41, Borislav Petkov wrote:
> On Tue, May 09, 2023 at 11:38:27AM -0700, Dave Hansen wrote:
>> This actually isn't _great_ for x86. We hid X86_FEATURE_NAMES behind
>> EMBEDDED because we didn't want to see it 99% of the time. But just
>> about everyone uses EXPERT=y, so the end result here is that everyone
>> will now see X86_FEATURE_NAMES.
>>
>> Oh well. It's just one Kconfig option. Not a big deal.
> Or we can make it unconditional. I haven't heard anything from the tiny
> .config folks in a while and a !X86_FEATURE_NAMES kernel is just
> unfriendly.

Like remove the option entirely? Or just remove the prompt so folks
have to hack the .config for it?

2023-05-09 19:08:59

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On 5/9/23 11:20, Lukas Bulwahn wrote:
> On Tue, May 9, 2023 at 4:07 PM Dave Hansen <[email protected]> wrote:
>>
>> On 5/9/23 01:40, Lukas Bulwahn wrote:
>>> Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
>>> introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
>>> and just gives that intent a much better name.
>>
>> That was quite the changelog, and I'm still not quite sure:
>>
>> What is the problem with the existing code?
>>
>> What is the end user visible impact of this problem and of this proposed
>> change?
>>
>
> Thanks, Dave, for your feedback.
>
> The commit message is so lengthy, as I copied it from the original
> cover letter into each commit, but I agree with your feedback.

Uhh... *What* cover letter? A cover letter is usually the 0/NN message
in a patch series. It is not obvious at *all* that this plain
non-numbered patch is part of a series.

> So maybe this commit message fits better:
>
> To simplify build configurations, the CONFIG_EMBEDDED is to be
> removed. All configs that are only visible with CONFIG_EMBEDDED=y
> shall be now visible with CONFIG_EXPERT=y.
>
> In the x86 architecture, the config X86_FEATURE_NAMES is visible when
> CONFIG_EMBEDDED is enabled. Now, make the config X86_FEATURE_NAMES
> visible when CONFIG_EXPERT is enabled.
>
> Dave, what do you think? If that is good enough for you, I will send
> an updated patch with that commit message.

CONFIG_EMBEDDED is being removed:

<INSERT LINK HERE>

That means that everything in Kconfig that uses CONFIG_EMBEDDED needs to
switch over to something else.

Move X86_FEATURE_NAMES over to CONFIG_EXPERT instead of CONFIG_EMBEDDED.

--

This actually isn't _great_ for x86. We hid X86_FEATURE_NAMES behind
EMBEDDED because we didn't want to see it 99% of the time. But just
about everyone uses EXPERT=y, so the end result here is that everyone
will now see X86_FEATURE_NAMES.

Oh well. It's just one Kconfig option. Not a big deal.

2023-05-09 19:10:56

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On Tue, May 09, 2023 at 11:38:27AM -0700, Dave Hansen wrote:
> This actually isn't _great_ for x86. We hid X86_FEATURE_NAMES behind
> EMBEDDED because we didn't want to see it 99% of the time. But just
> about everyone uses EXPERT=y, so the end result here is that everyone
> will now see X86_FEATURE_NAMES.
>
> Oh well. It's just one Kconfig option. Not a big deal.

Or we can make it unconditional. I haven't heard anything from the tiny
.config folks in a while and a !X86_FEATURE_NAMES kernel is just
unfriendly.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-05-09 19:12:07

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On Tue, May 09, 2023 at 11:43:12AM -0700, Dave Hansen wrote:
> Like remove the option entirely? Or just remove the prompt so folks
> have to hack the .config for it?

I wanna do entirely as we have waaay too many config options but we
probably could start with removing the prompt only and see who
complains...


--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-05-10 01:53:57

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On May 9, 2023 1:40:07 AM PDT, Lukas Bulwahn <[email protected]> wrote:
>Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
>introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
>and just gives that intent a much better name. That has been clearly a good
>and long overdue renaming, and it is clearly an improvement to the kernel
>build configuration that has shown to help managing the kernel build
>configuration in the last decade.
>
>However, rather than bravely and radically just deleting CONFIG_EMBEDDED,
>this commit gives CONFIG_EMBEDDED a new intended semantics, but keeps it
>open for future contributors to implement that intended semantics:
>
> A new CONFIG_EMBEDDED option is added that automatically selects
> CONFIG_EXPERT when enabled and can be used in the future to isolate
> options that should only be considered for embedded systems (RISC
> architectures, SLOB, etc).
>
>Since then, this CONFIG_EMBEDDED implicitly had two purposes:
>
> - It can make even more options visible beyond what CONFIG_EXPERT makes
> visible. In other words, it may introduce another level of enabling the
> visibility of configuration options: always visible, visible with
> CONFIG_EXPERT and visible with CONFIG_EMBEDDED.
>
> - Set certain default values of some configurations differently,
> following the assumption that configuring a kernel build for an
> embedded system generally starts with a different set of default values
> compared to kernel builds for all other kind of systems.
>
>Considering the first purpose, at the point in time where CONFIG_EMBEDDED
>was renamed to CONFIG_EXPERT, CONFIG_EXPERT already made 130 more options
>become visible throughout all different menus for the kernel configuration.
>Over the last decade, this has gradually increased, so that currently, with
>CONFIG_EXPERT, roughly 170 more options become visible throughout all
>different menus for the kernel configuration. In comparison, currently with
>CONFIG_EMBEDDED enabled, just seven more options are visible, one in x86,
>one in arm, and five for the ChipIdea Highspeed Dual Role Controller.
>
>As the numbers suggest, these two levels of enabling the visibility of even
>more configuration options---beyond what CONFIG_EXPERT enables---never
>evolved to a good solution in the last decade. In other words, this
>additional level of visibility of configuration option with CONFIG_EMBEDDED
>compared to CONFIG_EXPERT has since its introduction never become really
>valuable. It requires quite some investigation to actually understand what
>is additionally visible and it does not differ significantly in complexity
>compared to just enabling CONFIG_EXPERT. This CONFIG_EMBEDDED---or any
>other config to show more detailed options beyond CONFIG_EXPERT---is
>unlikely to be valuable unless somebody puts significant effort in
>identifying how such visibility options can be properly split and creating
>clear criteria, when some config option is visible with CONFIG_EXPERT and
>when some config option is visible only with some further option enabled
>beyond CONFIG_EXPERT, such as CONFIG_EMBEDDED attempted to do. For now, it
>is much more reasonable to simply make those additional seven options that
>are visible with CONFIG_EMBEDDED visible with CONFIG_EXPERT, and then
>remove CONFIG_EMBEDDED. If anyone spends significant effort in structuring
>the visibility of config options, they may re-introduce suitable new
>config options simply as they see fit.
>
>Make the config X86_FEATURE_NAMES visible when CONFIG_EXPERT is enabled.
>
>Signed-off-by: Lukas Bulwahn <[email protected]>
>Reviewed-by: Masahiro Yamada <[email protected]>
>Acked-by: Arnd Bergmann <[email protected]>
>---
> arch/x86/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index ce460d6b4e25..595f6696281c 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -442,7 +442,7 @@ config SMP
> If you don't know what to do here, say N.
>
> config X86_FEATURE_NAMES
>- bool "Processor feature human-readable names" if EMBEDDED
>+ bool "Processor feature human-readable names" if EXPERT
> default y
> help
> This option compiles in a table of x86 feature bits and corresponding

You know it used to be named that, and it was changed exactly because it was a terrible name, right?

2023-05-10 07:30:42

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On Tue, May 9, 2023 at 9:03 PM Borislav Petkov <[email protected]> wrote:
>
> On Tue, May 09, 2023 at 11:43:12AM -0700, Dave Hansen wrote:
> > Like remove the option entirely? Or just remove the prompt so folks
> > have to hack the .config for it?
>
> I wanna do entirely as we have waaay too many config options but we
> probably could start with removing the prompt only and see who
> complains...
>

Dave, Boris, thanks for your review and feedback. I have now sent you
a new patch series to remove it from the prompt and remove it entirely
as two patches, so you can choose which option to take now. In the
end, all of this serves my overall goal of reducing the number of
configs and removing CONFIG_EMBEDDED very well.

Lukas

2023-05-10 08:01:50

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On Wed, May 10, 2023 at 3:07 AM H. Peter Anvin <[email protected]> wrote:
>
> On May 9, 2023 1:40:07 AM PDT, Lukas Bulwahn <[email protected]> wrote:
> >Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
> >introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
> >and just gives that intent a much better name. That has been clearly a good
> >and long overdue renaming, and it is clearly an improvement to the kernel
> >build configuration that has shown to help managing the kernel build
> >configuration in the last decade.
> >
> >However, rather than bravely and radically just deleting CONFIG_EMBEDDED,
> >this commit gives CONFIG_EMBEDDED a new intended semantics, but keeps it
> >open for future contributors to implement that intended semantics:
> >
> > A new CONFIG_EMBEDDED option is added that automatically selects
> > CONFIG_EXPERT when enabled and can be used in the future to isolate
> > options that should only be considered for embedded systems (RISC
> > architectures, SLOB, etc).
> >
> >Since then, this CONFIG_EMBEDDED implicitly had two purposes:
> >
> > - It can make even more options visible beyond what CONFIG_EXPERT makes
> > visible. In other words, it may introduce another level of enabling the
> > visibility of configuration options: always visible, visible with
> > CONFIG_EXPERT and visible with CONFIG_EMBEDDED.
> >
> > - Set certain default values of some configurations differently,
> > following the assumption that configuring a kernel build for an
> > embedded system generally starts with a different set of default values
> > compared to kernel builds for all other kind of systems.
> >
> >Considering the first purpose, at the point in time where CONFIG_EMBEDDED
> >was renamed to CONFIG_EXPERT, CONFIG_EXPERT already made 130 more options
> >become visible throughout all different menus for the kernel configuration.
> >Over the last decade, this has gradually increased, so that currently, with
> >CONFIG_EXPERT, roughly 170 more options become visible throughout all
> >different menus for the kernel configuration. In comparison, currently with
> >CONFIG_EMBEDDED enabled, just seven more options are visible, one in x86,
> >one in arm, and five for the ChipIdea Highspeed Dual Role Controller.
> >
> >As the numbers suggest, these two levels of enabling the visibility of even
> >more configuration options---beyond what CONFIG_EXPERT enables---never
> >evolved to a good solution in the last decade. In other words, this
> >additional level of visibility of configuration option with CONFIG_EMBEDDED
> >compared to CONFIG_EXPERT has since its introduction never become really
> >valuable. It requires quite some investigation to actually understand what
> >is additionally visible and it does not differ significantly in complexity
> >compared to just enabling CONFIG_EXPERT. This CONFIG_EMBEDDED---or any
> >other config to show more detailed options beyond CONFIG_EXPERT---is
> >unlikely to be valuable unless somebody puts significant effort in
> >identifying how such visibility options can be properly split and creating
> >clear criteria, when some config option is visible with CONFIG_EXPERT and
> >when some config option is visible only with some further option enabled
> >beyond CONFIG_EXPERT, such as CONFIG_EMBEDDED attempted to do. For now, it
> >is much more reasonable to simply make those additional seven options that
> >are visible with CONFIG_EMBEDDED visible with CONFIG_EXPERT, and then
> >remove CONFIG_EMBEDDED. If anyone spends significant effort in structuring
> >the visibility of config options, they may re-introduce suitable new
> >config options simply as they see fit.
> >
> >Make the config X86_FEATURE_NAMES visible when CONFIG_EXPERT is enabled.
> >
> >Signed-off-by: Lukas Bulwahn <[email protected]>
> >Reviewed-by: Masahiro Yamada <[email protected]>
> >Acked-by: Arnd Bergmann <[email protected]>
> >---
> > arch/x86/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> >index ce460d6b4e25..595f6696281c 100644
> >--- a/arch/x86/Kconfig
> >+++ b/arch/x86/Kconfig
> >@@ -442,7 +442,7 @@ config SMP
> > If you don't know what to do here, say N.
> >
> > config X86_FEATURE_NAMES
> >- bool "Processor feature human-readable names" if EMBEDDED
> >+ bool "Processor feature human-readable names" if EXPERT
> > default y
> > help
> > This option compiles in a table of x86 feature bits and corresponding
>
> You know it used to be named that, and it was changed exactly because it was a terrible name, right?

Yes, I do (commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to
CONFIG_EXPERT")); and keeping CONFIG_EMBEDDED after that renaming was
not the best idea in retrospect.

Lukas

2023-05-10 08:12:19

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] x86: make config X86_FEATURE_NAMES visible with EXPERT

On Tue, May 9, 2023 at 8:38 PM Dave Hansen <[email protected]> wrote:
>
> On 5/9/23 11:20, Lukas Bulwahn wrote:
> > On Tue, May 9, 2023 at 4:07 PM Dave Hansen <[email protected]> wrote:
> >>
> >> On 5/9/23 01:40, Lukas Bulwahn wrote:
> >>> Commit 6a108a14fa35 ("kconfig: rename CONFIG_EMBEDDED to CONFIG_EXPERT")
> >>> introduces CONFIG_EXPERT to carry the previous intent of CONFIG_EMBEDDED
> >>> and just gives that intent a much better name.
> >>
> >> That was quite the changelog, and I'm still not quite sure:
> >>
> >> What is the problem with the existing code?
> >>
> >> What is the end user visible impact of this problem and of this proposed
> >> change?
> >>
> >
> > Thanks, Dave, for your feedback.
> >
> > The commit message is so lengthy, as I copied it from the original
> > cover letter into each commit, but I agree with your feedback.
>
> Uhh... *What* cover letter? A cover letter is usually the 0/NN message
> in a patch series. It is not obvious at *all* that this plain
> non-numbered patch is part of a series.
>

Yeah, sending out the patch isolated that originated from a larger
patch series was a bad idea. I got that.

> > So maybe this commit message fits better:
> >
> > To simplify build configurations, the CONFIG_EMBEDDED is to be
> > removed. All configs that are only visible with CONFIG_EMBEDDED=y
> > shall be now visible with CONFIG_EXPERT=y.
> >
> > In the x86 architecture, the config X86_FEATURE_NAMES is visible when
> > CONFIG_EMBEDDED is enabled. Now, make the config X86_FEATURE_NAMES
> > visible when CONFIG_EXPERT is enabled.
> >
> > Dave, what do you think? If that is good enough for you, I will send
> > an updated patch with that commit message.
>
> CONFIG_EMBEDDED is being removed:
>
> <INSERT LINK HERE>
>
> That means that everything in Kconfig that uses CONFIG_EMBEDDED needs to
> switch over to something else.
>
> Move X86_FEATURE_NAMES over to CONFIG_EXPERT instead of CONFIG_EMBEDDED.
>

Just for completeness of the mailing list archive, I will add the link:

CONFIG_EMBEDDED is being removed:

https://lore.kernel.org/all/[email protected]/

Following Boris' suggestion, I have now created a different patch to
make the option non-visible in the prompt (see Link below). So, this
patch may not be required at all if the other patch is accepted. In
case there is a critical number of users out there that really want to
tinker with this option and we decide to make it visible with
CONFIG_EXPERT, I will propose this patch with the commit message you
provided. For now, this patch is superseded by the patch 1 of 2 below:

Link: https://lore.kernel.org/all/[email protected]/

Thanks for all the help.

Lukas

> --
>
> This actually isn't _great_ for x86. We hid X86_FEATURE_NAMES behind
> EMBEDDED because we didn't want to see it 99% of the time. But just
> about everyone uses EXPERT=y, so the end result here is that everyone
> will now see X86_FEATURE_NAMES.
>
> Oh well. It's just one Kconfig option. Not a big deal.