Skip to content

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

Closed
teto wants to merge 3 commits intoNixOS:masterfrom
teto:kernel_config
Closed

[RFC] linux: translate config to structured config#41393
teto wants to merge 3 commits intoNixOS:masterfrom
teto:kernel_config

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.

@teto teto requested review from edolstra and nbp as code owners June 2, 2018 17:12
@teto teto force-pushed the kernel_config branch from a2d99b8 to ed86292 Compare June 2, 2018 17:14
@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jun 2, 2018
@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

@GrahamcOfBorg GrahamcOfBorg added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 17, 2018
@GrahamcOfBorg GrahamcOfBorg removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 17, 2018
@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 GrahamcOfBorg added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jun 17, 2018
@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 GrahamcOfBorg removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jun 18, 2018
@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
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

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

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)

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

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

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants