Re: Abuse of CONFIG_FOO's as feature selectors

[Stefan Hengelein]

2015-04-22 20:56 GMT+02:00 Andreas Ruprecht <andreas.ruprecht@xxxxxx>:
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/