Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] linux: translate config to structured config #41393

Closed
wants to merge 3 commits into from

Conversation

teto
Copy link
Member

@teto teto commented Jun 2, 2018

Follow up of #12158 @copumpkin . I am looking for comments before polishing as compiling kernels is not especially fast here.

The set is then converted into a string so backwards compatibility kinda applies (users can still specify their kernel extraConfig as a string) but it should be deprecated in favor of sets.

Motivation for this change
  • I want to be able to convert "module" answers into "yes" and vice-versa
  • with the current kernel config system, the first defined is taken into account so
extraConfig = ''
OPENVSWITCH y 
OPENVSWITCH m
'';

will end up with yes. Even though this PR doesn't solve this problem, it paves the way for some kind of mkMerge mechanism.

Things done
  • added structuredExtraConfig and mkValuePreprocess to buildLinux args, I kept extraConfig as a string for backawards compatibility, structuredExtraConfig must be a flat set e.g. { BPF = yes; BPF_SYSCALL = y; }
  • mkValuePreprocess allows to override each setting, aka convert 'yes' into 'modules' or vice-versa

I test the changes with this overlay expression and tried compiling a few kernels (can't do all though with my poor machine).

  linux_test = prev.linux_4_16.override {
    ignoreConfigErrors=true;
    autoModules = false;
    kernelPreferBuiltin = true;
  };

Future work
[ ] Convert patches to this new format
[ ] make packages describe their kernel requirements (bcc needs BPF, openvswitch needs OPENVSWITCH etc)
For instance the openvswitch package could contain hints to the configuration:

 with lib.kernel;
   passthru.kernelExtraConfig = with callPackage ./lib/kernel.nix { version = kernel.version; }; {
     OPENVSWITCH  = whenAtLeast "2.0" yes;
   };

and have the module actually enforce that configuration:

    boot.kernelPatches = [ {
         name = "openvswitch";
         patch = null;
         extraConfig = lib.kernel.generateKConf pkgs.openvswitch.passthru.kernelExtraConfig;
       }
     ];
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

/nix/store/si1gisyb8ly0vnrhgdx5qxdzngyvsi5v-linux-4.14.47

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

these paths will be fetched (48.28 MiB download, 78.80 MiB unpacked):
  /nix/store/swwnj5b8jfz1bi6fvvfvlcgqwwsbkzni-linux-4.14.47
copying path '/nix/store/swwnj5b8jfz1bi6fvvfvlcgqwwsbkzni-linux-4.14.47' from 'https://cache.nixos.org'...
warning: SQLite database '/nix/var/nix/db/db.sqlite' is busy
/nix/store/swwnj5b8jfz1bi6fvvfvlcgqwwsbkzni-linux-4.14.47

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

/nix/store/nn55hl7zxl8cx9xl0bv7ra8jz2bqjczr-linux-4.14.49

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

these paths will be fetched (48.27 MiB download, 78.79 MiB unpacked):
  /nix/store/6wpbdldldwn76lxq1m3vjrgp9a15aqxz-linux-4.14.49
copying path '/nix/store/6wpbdldldwn76lxq1m3vjrgp9a15aqxz-linux-4.14.49' from 'https://cache.nixos.org'...
/nix/store/6wpbdldldwn76lxq1m3vjrgp9a15aqxz-linux-4.14.49

@teto
Copy link
Member Author

teto commented Jun 17, 2018

I updated the default config so that it reflects exaclty (notwithstanding my mistakes) the current common-config.nix.
When I try to run nox-review wip --against=upstream/master I get this kind of error http://nixpaste.lbr.uno/-fsL_q4k?nix so I accept any tip on how to tackle this kind of "infinite recursion" problem

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
/nix/store/cv3cfq0axhlcqg0khw1lrxddxsg03nyb-linux-4.14.50

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
cannot find section .dynamic
/nix/store/bja0q8gf2bxv798kxmqhjw3sqzlhxadv-linux-4.14.50

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

/nix/store/cv3cfq0axhlcqg0khw1lrxddxsg03nyb-linux-4.14.50

@teto teto changed the title [wip] linux: translate config to structured config [RFC] linux: translate config to structured config Jun 18, 2018
@teto
Copy link
Member Author

teto commented Jun 18, 2018

I've updated common-config to reflect all the changes since copumpkin PRs and was able to compile 4.17. I updated the initial message too, removing out of scope changes. It should be ready for review (apart from the messy history)

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
cannot find section .dynamic
/nix/store/bja0q8gf2bxv798kxmqhjw3sqzlhxadv-linux-4.14.50

lib/kernel.nix Outdated
# returns a string, expr should be an attribute set
# generateNixKConf = exprs:
# generateNixKConfAdvanced exprs null;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo remove

@@ -59,8 +63,10 @@ let
netfilterRPFilter = true;
} // features) kernelPatches;

config = import ./common-config.nix {
inherit stdenv version ;
# returns a config of the form
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

@@ -24,7 +24,7 @@ in {
modDirVersion ? version,
# The kernel source (tarball, git checkout, etc.)
src,
# Any patches
# a list of { name=..., patch=..., } patches
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add "extraConfig"

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
cannot find section .dynamic
/nix/store/bja0q8gf2bxv798kxmqhjw3sqzlhxadv-linux-4.14.50

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

  CC [M]  drivers/staging/speakup/fakekey.o
  CC [M]  drivers/staging/speakup/main.o
  CC [M]  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.o
  CC [M]  drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx_usb.o
  CC [M]  drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx_sdio.o
  CC [M]  drivers/staging/rtl8723bs/os_dep/ioctl_linux.o
  CC [M]  drivers/staging/speakup/keyhelp.o
  CC [M]  drivers/staging/speakup/kobjects.o
building of '/nix/store/24plzvpzg33v84pmfd7miq69imcyd8ff-linux-4.14.50.drv' timed out after 1800 seconds
error: build of '/nix/store/24plzvpzg33v84pmfd7miq69imcyd8ff-linux-4.14.50.drv' failed

Copy link
Member

@NeQuissimus NeQuissimus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to dependencies? If I set module A to "n" but module B to "y" despite B depending on A, the kernel config will error out, correct?
Is there something simple we can do to detect this? (I can't see a simple way)

DEBUG_DEVRES = no;
TIMER_STATS = whenOlder "4.11" yes;
DEBUG_NX_TEST = whenOlder "4.11" no;
CPU_NOTIFIER_ERROR_INJECT = whenOlder "4.4" (option no);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are no kernels <4.4, should we maybe just remove all the options for really old kernels?
And things such as whenAtLeast "3.4" yes may as well just be yes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that it's there I would say keep it, some people might run their own 3.X kernel and might appreciate it ? I am good either way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we will do that in a future PR. I highly doubt somebody uses master / future-18.09 with a 3.x kernel :D

X86_INTEL_LPSS = whenAtLeast "3.11" yes;
X86_INTEL_PSTATE = whenAtLeast "3.10" yes;
INTEL_IDLE = yes;
CPU_FREQ_DEFAULT_GOV_PERFORMANCE = yes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to add the other governors as options here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do such changes separately from the structured-options refactoring; that way it is much easier to verify there weren't unintended changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, I had to remove some copumpkin CONFIG_ additions that would generate errors on recent kernels.

@teto
Copy link
Member Author

teto commented Jun 22, 2018

What happens to dependencies? If I set module A to "n" but module B to "y" despite B depending on A, > the kernel config will error out, correct?
Is there something simple we can do to detect this? (I can't see a simple way)
This is the current behavior and the PR doesn't change that.

I discovered yesterday https://github.com/ulfalizer/Kconfiglib which might help in that regard at a later date, I am not sure. The kernel has also a script to merge different configs https://github.com/torvalds/linux/blob/master/scripts/kconfig/merge_config.sh, I wonder if this could be used to fix some of our dependencies problem. But I would rather leave this exploration to future PRs.

@dezgeg
Copy link
Contributor

dezgeg commented Jun 22, 2018

I noticed some minor differences in the configs, feel free to squash the fixes from https://github.com/NixOS/nixpkgs/compare/master...dezgeg:structured-kernel-conf-fixes?expand=1

@teto
Copy link
Member Author

teto commented Jun 22, 2018

wow thanks that's super nice, you caught quite a few ones. I squashed and rebased. I added a commit to remove references to any kernel < 4.4 so that it can be reverted later.
I was able to compile the config for 4.4 and 4.16 (haven't tested any other)

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
cannot find section .dynamic
/nix/store/g3cq7wr2g5hqm8rvj6gky6x7qa1lgjn4-linux-4.14.51

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

  CC [M]  drivers/staging/rtl8192e/rtl8192e/rtl_pm.o
  CC [M]  drivers/video/fbdev/sstfb.o
  CC [M]  drivers/staging/rtl8712/usb_halinit.o
  CC [M]  drivers/staging/rtl8192u/r8190_rtl8256.o
  CC [M]  drivers/staging/rtl8192e/rtl8192e/rtl_ps.o
  CC [M]  drivers/video/fbdev/cirrusfb.o
  CC [M]  drivers/staging/rtl8712/usb_ops.o
  CC [M]  drivers/staging/rtl8192u/r819xU_phy.o
building of '/nix/store/kvi48kp4zzpbaqyvv313kpjygxwc2sra-linux-4.14.51.drv' timed out after 1800 seconds
error: build of '/nix/store/kvi48kp4zzpbaqyvv313kpjygxwc2sra-linux-4.14.51.drv' failed

copumpkin and others added 2 commits June 23, 2018 08:41
Instead of using a string to describe kernel config, use a nix
attribute set, then converted to a string.
- allows to override the config, aka convert 'yes' into 'modules' or
vice-versa
- while for now merging different configs is still crude (last spec wins),
at least there should be only one CONFIG_XYZ value compared to the current string
config where the first defined would be used and others ignored.
The oldest kernel in nixpkgs being 4.4, we get rid of checks for older
kernels.
with import ../../../../lib/kernel.nix { inherit (stdenv) lib; inherit version; };

let
# temporary hack
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what's this for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when reworking the original PR, the features system had changed a bit and I came up with the hack to remove later. I had forgotten about it; I pushed a 3rd commit which uses features.xdom0 instead. It should fix it.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: linux

Partial log (click to expand)

wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
wrong ELF type
cannot find section .dynamic
/nix/store/qp98m3d4xa1f7x8xbnvwa7wczfbllav7-linux-4.14.51

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: linux

Partial log (click to expand)

  CC [M]  drivers/usb/storage/isd200.o
  CC [M]  drivers/usb/storage/jumpshot.o
  CC [M]  drivers/usb/storage/karma.o
  CC [M]  drivers/usb/storage/onetouch.o
  CC [M]  drivers/usb/storage/realtek_cr.o
  CC [M]  drivers/usb/storage/sddr09.o
  CC [M]  drivers/usb/storage/sddr55.o
  CC [M]  drivers/usb/storage/shuttle_usbat.o
building of '/nix/store/qq0jx495ka6g8s75h3zn3nhby0gc3nbv-linux-4.14.51.drv' timed out after 1800 seconds
error: build of '/nix/store/qq0jx495ka6g8s75h3zn3nhby0gc3nbv-linux-4.14.51.drv' failed

@dezgeg
Copy link
Contributor

dezgeg commented Jun 30, 2018

I verified with scripts that after a minor fix I squashed in there's no change to the effective configs on x86_64, i686, aarch64 and armv7l. So merged.

@dezgeg dezgeg closed this Jun 30, 2018
@teto teto deleted the kernel_config branch June 30, 2018 15:06
@teto
Copy link
Member Author

teto commented Jan 25, 2019

@dezgeg mind sharing these scripts please :) ? I want to do the same in #42838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants