Re: [PATCH 13/15] x86/kconfig/64: Enable popular scheduler, cgroups and namespaces options in the defconfig

[Ingo Molnar <mingo@kernel.org>]

* Arnd Bergmann <arnd@arndb.de> wrote:

> > +CONFIG_SYSFS_SYSCALL=y
> > +CONFIG_EXPERT=y
> >  CONFIG_KALLSYMS_ALL=y
> >  CONFIG_PROFILING=y
> 
> I really don't like enabling CONFIG_EXPERT=y in a generic
> defconfig. What changes if you turn this off?

That's a good question.

Disabling it gives me material changes for 4 options:

	--- .config.before
	+++ .config.after
	-CONFIG_EXPERT=y
	-CONFIG_ARCH_HAS_ZONE_DMA_SET=y
	+CONFIG_RFKILL_INPUT=y
	-CONFIG_PCIE_BUS_DEFAULT=y
	+CONFIG_DEBUG_MEMORY_INIT=y

1) CONFIG_DEBUG_MEMORY_INIT

The CONFIG_DEBUG_MEMORY_INIT default is super weird:

  config DEBUG_MEMORY_INIT
        bool "Debug memory initialisation" if EXPERT
        default !EXPERT

I this might in fact be a bug, and Ubuntu might have fallen victim to 
it:

  .config.fedora: CONFIG_DEBUG_MEMORY_INIT=y
  .config.ubuntu: # CONFIG_DEBUG_MEMORY_INIT is not set

I believe this should be 'default y', or 'default n'.

2) CONFIG_ARCH_HAS_ZONE_DMA_SET

This one is an interim Kconfig helper flag, and it's a bit weird as 
well:

  arch/x86/Kconfig:       select ARCH_HAS_ZONE_DMA_SET if EXPERT

I *think* the intent here is to make configurability of ZONE_DMA and 
ZONE_DMA32 dependent on EXPERT, while still giving architectures an 
opt-in as well:

 config ZONE_DMA
        bool "Support DMA zone" if ARCH_HAS_ZONE_DMA_SET
        default y if ARM64 || X86

 config ZONE_DMA32
        bool "Support DMA32 zone" if ARCH_HAS_ZONE_DMA_SET
        depends on !X86_32
        default y if ARM64

I think the better approach would be to make the EXPERT policy at the 
ZONE_DMA and ZONE_DMA32 level:

        bool "Support DMA zone" if ARCH_HAS_ZONE_DMA_SET && EXPERT

but it should be functionally equivalent.

3) RFKILL_INPUT

I think this one's a bug too:

 config RFKILL_INPUT
        bool "RF switch input support" if EXPERT
        depends on RFKILL
        depends on INPUT = y || RFKILL = INPUT
        default y if !EXPERT

Basically if you turn on EXPERT, the default changes from Y to N.

I think this should be a plain 'default y'.

4) CONFIG_PCIE_BUS_DEFAULT

I think this is quite confusing code as well:

  choice
        prompt "PCI Express hierarchy optimization setting"
        default PCIE_BUS_DEFAULT
        depends on PCI && EXPERT
        help
  ...

  config PCIE_BUS_DEFAULT
        bool "Default"
        depends on PCI
        help
          Default choice; ensure that the MPS matches upstream bridge.

  ...
  endchoice

So the intent here is clearly to steer users towards picking 
PCIE_BUS_DEFAULT.

But the 'depends' line turns off the option entirely on !EXPERT.

Which happens to work due to how the config options are used by the PCI 
code:

  #ifdef CONFIG_PCIE_BUS_TUNE_OFF
  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;
  #elif defined CONFIG_PCIE_BUS_SAFE
  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_SAFE;
  #elif defined CONFIG_PCIE_BUS_PERFORMANCE
  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PERFORMANCE;
  #elif defined CONFIG_PCIE_BUS_PEER2PEER
  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PEER2PEER;
  #else
  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT;
  #endif

But this is highly unintuitive IMO. A cleaner implementation would be 
to always have CONFIG_PCIE_BUS_DEFAULT enabled on !EXPERT, which can be 
done by making the configurability of the choice-list depend on EXPERT:

  choice
        prompt "PCI Express hierarchy optimization setting" if EXPERT
        default PCIE_BUS_DEFAULT
        depends on PCI

> Based on the help text for CONFIG_EXPERT, nothing we
> consider the default should ever be guarded by it. If there
> is something that distros commonly that is prevented by
> EXPERT=n, it would be better to relay the dependency on that
> particular thing.

I think distro kernel maintainers mainly inherited their old configs 
and aren't afraid of CONFIG_EXPERT.

Thus *all* major distros I checked have CONFIG_EXPERT enabled: Ubuntu, 
Fedora, Debian, you name it. So literally over 99% of our users use a 
kernel that has CONFIG_EXPERT=y in it. Which is perfectly fine, distro 
kernel maintainers *are* the ultimate experts in this matter - but 
their choices inevitably make it to users configuring their own 
kernels: if users type 'make localmodconfig' they'll have 
CONFIG_EXPERT=y.

So I don't think we should ostracize CONFIG_EXPERT too much. :)

Otherwise I think you were right: 2 out of 4 of the configuration 
settings that change due to EXPERT are outright bugs IMO, the other 2 
are weird code that could be done in a more standard fashion, resulting 
in an invariant .config when EXPERT is toggled on/off.

Also, I kinda don't mind having CONFIG_EXPERT=y in the kernel 
defconfig: it's a helper config for *kernel developers* who want to 
have finegrained control over debug facilities and other details, it's 
not something for users - the resulting kernels won't result in a fully 
working system on modern x86 systems.

Thanks,

	Ingo