Abuse of CONFIG_FOO's as feature selectors

Abuse of CONFIG_FOO's as feature selectors

[Denys Vlasenko]

Hi,

Kernel has a growing number of CONFIG items which are not
user-selectable features of their particular kernel builds,
but simply booleans controlled by other CONFIGs.
Example:

config X86
        def_bool y
        select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
        select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
        select ARCH_HAS_FAST_MULTIPLIER
        select ARCH_HAS_GCOV_PROFILE_ALL
        select ARCH_MIGHT_HAVE_PC_PARPORT
        select ARCH_MIGHT_HAVE_PC_SERIO
        select HAVE_AOUT if X86_32
        select HAVE_UNSTABLE_SCHED_CLOCK
        select ARCH_SUPPORTS_NUMA_BALANCING if X86_64
        select ARCH_SUPPORTS_INT128 if X86_64
        select HAVE_IDE
        select HAVE_OPROFILE
        ...

I see how this practice originated: "select" statement
was initially added so that if feature X requires feature Y,
this can be enforced, but it was easy to use it to define
other booleans.

I have a feeling that in retrospect, it was a mistake.

It clutters .config with information which has nothing to do
with user's choice.

More importantly, now when you read some code, you don't know
whether a CONFIG_FOO you look at is user's configuration choice
or something else.

Now there are hundreds, maybe even thousands of these non-config
CONFIGs everywhere.


The same effect can be achieved, with marginally more typing,
with usual C defines in some header file:

#ifdef CONFIG_X86
# define ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
# define ARCH_HAS_FAST_MULTIPLIER
# define ARCH_HAS_GCOV_PROFILE_ALL
# define ARCH_MIGHT_HAVE_PC_PARPORT
# define ARCH_MIGHT_HAVE_PC_SERIO
...

Maybe we should stop doing the former and use the latter method?

Re: Abuse of CONFIG_FOO's as feature selectors

[Andreas Ruprecht]

Hi,

On 22.04.2015 20:20, Denys Vlasenko wrote:
> Hi,
> 
> Kernel has a growing number of CONFIG items which are not
> user-selectable features of their particular kernel builds,
> but simply booleans controlled by other CONFIGs.
> Example:
> 
> I see how this practice originated: "select" statement
> was initially added so that if feature X requires feature Y,
> this can be enforced, but it was easy to use it to define
> other booleans.
> 
> I have a feeling that in retrospect, it was a mistake.
> 
> It clutters .config with information which has nothing to do
> with user's choice.
> 
> More importantly, now when you read some code, you don't know
> whether a CONFIG_FOO you look at is user's configuration choice
> or something else.

Well, there seems to be at least some convention with regards to the
name of those options: They all start with (ARCH_)HAS/HAVE/MIGHT_HAVE
and so forth.

> 
> Now there are hundreds, maybe even thousands of these non-config
> CONFIGs everywhere.
> 
> 
> The same effect can be achieved, with marginally more typing,
> with usual C defines in some header file:
> 
> #ifdef CONFIG_X86
> # define ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
> # define ARCH_HAS_FAST_MULTIPLIER
> # define ARCH_HAS_GCOV_PROFILE_ALL
> # define ARCH_MIGHT_HAVE_PC_PARPORT
> # define ARCH_MIGHT_HAVE_PC_SERIO
> ...
> 
> Maybe we should stop doing the former and use the latter method?

Problem is, most of these options which are not selectable by the user
operate as something like a "bridge" inside Kconfig itself. For example,
an architecture can specify that it has some specific feature upon which
a driver might depend. So, the architecture Kconfig file can set the
option, the driver can *depend* on it, allowing the driver only to be
built on the right architectures.

Transferring everything into a header (quite like
include/config/auto.conf works) would hence break the whole point of the
"bridge" rationale behind it, as only the code (and not Kconfig) would
be able to see this information.

But I generally agree, the distinction between configuration options
selectable by the user, options only present to model dependencies
inside the guts of Kconfig and other things (like CONFIG_AS_AVX2, which
is only passed as a compiler parameter from a Makefile, yuck) is not
clear at all and can be quite confusing.

Regards,

Andreas

P.S.: I've CCed some folks who might want to add their thoughts.

Re: Abuse of CONFIG_FOO's as feature selectors

[Stefan Hengelein]

2015-04-22 20:56 GMT+02:00 Andreas Ruprecht <andreas.ruprecht@fau.de>:
> Hi,
>
> On 22.04.2015 20:20, Denys Vlasenko wrote:
>> Hi,
>>
>> Kernel has a growing number of CONFIG items which are not
>> user-selectable features of their particular kernel builds,
>> but simply booleans controlled by other CONFIGs.
>> Example:
>>
>> I see how this practice originated: "select" statement
>> was initially added so that if feature X requires feature Y,
>> this can be enforced, but it was easy to use it to define
>> other booleans.
>>
>> I have a feeling that in retrospect, it was a mistake.
>>
>> It clutters .config with information which has nothing to do
>> with user's choice.

Well, if that's the problem, one could filter Kconfig options with the
prefixes: CONFIG_{HAVE, HAS, MIGHT_HAVE} for the .config, however, i'm
not sure if most users really care about where the selection for
CONFIG_FOO CONFIG_BAR and CONFIG_BAZ come from, as long as the
dependencies are met.

>>
>> More importantly, now when you read some code, you don't know
>> whether a CONFIG_FOO you look at is user's configuration choice
>> or something else.

With more than 14000 Kconfig options, it might already be difficult to
recognize which options are set by the user or automatically through
selects, even if you disregard the CONFIG_{HAVE, HAS, MIGHT_HAVE}
options.

>
> Well, there seems to be at least some convention with regards to the
> name of those options: They all start with (ARCH_)HAS/HAVE/MIGHT_HAVE
> and so forth.

see Documentation/kbuild/kconfig-language.txt "Kconfig hints" section

>
>>
>> Now there are hundreds, maybe even thousands of these non-config
>> CONFIGs everywhere.
>>
>>
>> The same effect can be achieved, with marginally more typing,
>> with usual C defines in some header file:
>>
>> #ifdef CONFIG_X86
>> # define ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
>> # define ARCH_HAS_FAST_MULTIPLIER
>> # define ARCH_HAS_GCOV_PROFILE_ALL
>> # define ARCH_MIGHT_HAVE_PC_PARPORT
>> # define ARCH_MIGHT_HAVE_PC_SERIO
>> ...
>>
>> Maybe we should stop doing the former and use the latter method?
>
> Problem is, most of these options which are not selectable by the user
> operate as something like a "bridge" inside Kconfig itself. For example,
> an architecture can specify that it has some specific feature upon which
> a driver might depend. So, the architecture Kconfig file can set the
> option, the driver can *depend* on it, allowing the driver only to be
> built on the right architectures.

Or, as i understood it, enable the user to choose to enable an option
or not, i.e. ensure option A is visible on architecture X, but not on
architecture Y or Z that doesn't select HAVE_A.
(see Documentation/kbuild/kconfig-language.txt "Kconfig hints" section)

Of course, one could implement option A differently:

config A
   depends on (X || Z || ..)

but that would be more verbose and i guess it would be harder to see
(as the number of options is constantly rising) for architecture X
which options belong to X, because this information would be scattered
across the tree.

>
> Transferring everything into a header (quite like
> include/config/auto.conf works) would hence break the whole point of the
> "bridge" rationale behind it, as only the code (and not Kconfig) would
> be able to see this information.

full ack, i wouldn't expect these options to be used somewhere outside
of kconfig, maybe in kbuild, but not in actual c code.

>
> But I generally agree, the distinction between configuration options
> selectable by the user, options only present to model dependencies
> inside the guts of Kconfig and other things (like CONFIG_AS_AVX2, which
> is only passed as a compiler parameter from a Makefile, yuck) is not
> clear at all and can be quite confusing.

yes, we could add an "INTERNAL_" prefix, but "HAVE, HAS, MIGHT_HAVE"
is already kind of an "internal" prefix.

Regards,
Stefan

Re: Abuse of CONFIG_FOO's as feature selectors

[Paul Bolle]

On Wed, 2015-04-22 at 20:20 +0200, Denys Vlasenko wrote:
> Kernel has a growing number of CONFIG items which are not
> user-selectable features of their particular kernel builds,
> but simply booleans controlled by other CONFIGs.
> Example:
> 
> config X86
>         def_bool y
>         select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
>         select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
>         select ARCH_HAS_FAST_MULTIPLIER
>         select ARCH_HAS_GCOV_PROFILE_ALL
>         select ARCH_MIGHT_HAVE_PC_PARPORT
>         select ARCH_MIGHT_HAVE_PC_SERIO
>         select HAVE_AOUT if X86_32
>         select HAVE_UNSTABLE_SCHED_CLOCK
>         select ARCH_SUPPORTS_NUMA_BALANCING if X86_64
>         select ARCH_SUPPORTS_INT128 if X86_64
>         select HAVE_IDE
>         select HAVE_OPROFILE
>         ...
> 
> I see how this practice originated: "select" statement
> was initially added so that if feature X requires feature Y,
> this can be enforced, but it was easy to use it to define
> other booleans.
> 
> I have a feeling that in retrospect, it was a mistake.
> 
> It clutters .config with information which has nothing to do
> with user's choice.

No, those selects fill the .config with values as a direct consequence
of the choices made by the person doing the configuration. You might
just as well consider those values things that the user wanted to have
too.

> More importantly, now when you read some code, you don't know
> whether a CONFIG_FOO you look at is user's configuration choice
> or something else.

So what?

> Now there are hundreds, maybe even thousands of these non-config
> CONFIGs everywhere.
>
> The same effect can be achieved, with marginally more typing,
> with usual C defines in some header file:
> 
> #ifdef CONFIG_X86
> # define ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS
> # define ARCH_HAS_FAST_MULTIPLIER
> # define ARCH_HAS_GCOV_PROFILE_ALL
> # define ARCH_MIGHT_HAVE_PC_PARPORT
> # define ARCH_MIGHT_HAVE_PC_SERIO
> ...
> 
> Maybe we should stop doing the former and use the latter method?

And lose the sanity checks that the kconfig tools provide? And the
benefit of a having a single .config file showing the configuration the
build will (or did) use?

Anyhow, -ENOPATCH. Because I actually suspect that this scheme will
complicate the tree quite a bit. Do send in patches showing how this
scheme allows to drop a few Kconfig symbols. That makes it much easier
to evaluate the pros and cons of your idea.

Thanks,


Paul Bolle