Skip to content

Move in-tree kernel modules to separate derivation output#423933

Merged
jmbaur merged 3 commits intoNixOS:masterfrom
jmbaur:linux-outputs
Aug 16, 2025
Merged

Move in-tree kernel modules to separate derivation output#423933
jmbaur merged 3 commits intoNixOS:masterfrom
jmbaur:linux-outputs

Conversation

@jmbaur
Copy link
Contributor

@jmbaur jmbaur commented Jul 10, 2025

The kernel image takes up a significant amount of space in the derivation where kernel modules are installed (even more so on aarch64 where we install the uncompressed Image file). Since the kernel image is not functionally needed in this derivation (by the time kernel modules would be of any use, we've already booted into our kernel), we can save some space by splitting out the modules into their own output. This change doesn't really do a lot for pure NixOS closures today, since the kernel image is still in the closure of config.system.build.toplevel, however the linux kernel build recipe in nixpkgs is super useful for more than just NixOS use-cases anyways. Plus, it's not out of the realm of possibility for NixOS's toplevel to remove the reference to the kernel image, which would be especially helpful in saving space on image-based systems where the ESP and/or boot partition is built and populated ahead of time and not part of the switch-to-configuration/bootloader-installation step.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: kernel The Linux kernel 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 10, 2025
@nix-owners nix-owners bot requested a review from thoughtpolice July 10, 2025 01:38
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Jul 10, 2025
@K900
Copy link
Contributor

K900 commented Jul 10, 2025

CC @emilazy

@jmbaur
Copy link
Contributor Author

jmbaur commented Jul 10, 2025

I've tested this on a minimal kernel configuration, but this does not yet work with the full-blown NixOS kernel configs yet, still working through this issue:

make[2]: /nix/store/00zrahbb32nzawrmv9sjxn36h7qk9vrs-bash-5.2p37/bin/bash: Argument list too long
make[2]: *** [../scripts/Makefile.modfinal:31: arch/x86/events/amd/power.mod.o] Error 127
make[1]: *** [/build/linux-6.12.36/Makefile:1878: modules] Error 2

@emilazy
Copy link
Member

emilazy commented Jul 10, 2025

This seems like a sensible enough direction. Even better would be if we could if we could have one output per module, or even one derivation per module. But that might not be easy to achieve.

I plan to split the outputs of the kernel images as part of my ongoing refactoring, but that shouldn’t conflict with this (semantically – diff‐wise, it probably will).

@jmbaur
Copy link
Contributor Author

jmbaur commented Jul 10, 2025

I plan to split the outputs of the kernel images as part of my ongoing refactoring.

That would be great! Would be amazing to build as many kernel image types for a given architecture as possible (like the ideas in #324419), to allow for the user to pick the one they want later without having to incur a rebuild.

Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Seems sensible enough to me but haven't tested. I guess there's not anything we can really do to make it backwards compatible without defeating the purpose by adding a reference…

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jul 10, 2025
@emilazy
Copy link
Member

emilazy commented Jul 10, 2025

Yeah I think it makes sense to batch this with my other target change since that will be (deliberately) breaking.

That would be great! Would be amazing to build as many kernel image types for a given architecture as possible (like the ideas in #324419), to allow for the user to pick the one they want later without having to incur a rebuild.

Right, yeah, that’s the idea, especially as it means we can more safely prefer compressed UEFI stub images on AArch64. Though uImage I am dropping entirely.

@jmbaur
Copy link
Contributor Author

jmbaur commented Jul 10, 2025

I've tested this on a minimal kernel configuration, but this does not yet work with the full-blown NixOS kernel configs yet, still working through this issue:

make[2]: /nix/store/00zrahbb32nzawrmv9sjxn36h7qk9vrs-bash-5.2p37/bin/bash: Argument list too long
make[2]: *** [../scripts/Makefile.modfinal:31: arch/x86/events/amd/power.mod.o] Error 127
make[1]: *** [/build/linux-6.12.36/Makefile:1878: modules] Error 2

So it appears somewhere in the kernel build system, the $modules environment variable is read, which was causing this issue. I've changed the output name to mod, which works, but definitely open to other ideas as well.

@jmbaur
Copy link
Contributor Author

jmbaur commented Jul 10, 2025

@ofborg test systemd-initrd-modprobe

@jmbaur
Copy link
Contributor Author

jmbaur commented Jul 12, 2025

This is ready. I've tested locally with the systemd-initrd-modprobe test, though ofborg seems to be chugging on it.

@emilazy
Copy link
Member

emilazy commented Jul 12, 2025

We can avoid contorting the output name by just unsetting modules and using ${outputs[modules]}, I believe. Might require __structuredAttrs but would be as good a time as any to migrate to that if we're not already on it anyway.

@jmbaur jmbaur force-pushed the linux-outputs branch 2 times, most recently from 98829e9 to 59aba1f Compare July 12, 2025 21:36
@jmbaur
Copy link
Contributor Author

jmbaur commented Jul 12, 2025

We can avoid contorting the output name by just unsetting modules and using ${outputs[modules]}, I believe. Might require __structuredAttrs but would be as good a time as any to migrate to that if we're not already on it anyway.

Should be good now!

@emilazy
Copy link
Member

emilazy commented Jul 13, 2025

BTW, this should be release noted as a breaking change.

@vcunat
Copy link
Member

vcunat commented Aug 19, 2025

I appreciate that the fixes come very fast after reporting them, at least 😄

dramforever added a commit to dramforever/nixos-apple-silicon that referenced this pull request Aug 23, 2025
dramforever added a commit to dramforever/nixos-apple-silicon that referenced this pull request Aug 23, 2025
Add a hack to fix Rust support broken by NixOS/nixpkgs#423933

See NixOS/nixpkgs#436245 for Nixpkgs upstream fix
samueldr added a commit to samueldr/nixpkgs that referenced this pull request Aug 23, 2025
The changes in NixOS#423933 caused
previously working calls to `pkgs.makeModulesClosure` to produce empty
outputs, when paired with `allowMissing = true;`.

In turn, this broke boot in those systems, as instead of having kernel
modules (as one would expect), instead there was nothing to load.

This is not only a weird edge case, as due to how the options compose in
the NixOS modules system, `allowMissing` is de-facto required in some
use-cases. (See e.g. NixOS#154163 (comment) )

The choice of `allowEmpty` being set to `false` by default matches the
default assumptions from `allowMissing`: something should be produced.

This *may* break some builds, but it will break them at build time,
rather than breaking boot. In this situation setting `allowEmpty` will
be the solution to fix this issue.

This is a tradeoff: break the usage of `makeModulesClosure` at build
time for those that wanted an empty output, or break the usage of
`makeModulesClosure` at boot for the others.

The latter is much much much preferrable, since it prevents problematic
situations where a remote system is stuck not booting, without remote
hands. The former situation will only be groan-inducing.

This change was tested with Mobile NixOS. Between NixOS#423933 and this
change, it *will* produce broken stage-1 systems for modular kernels.
With this change, it breaks the builds. ***This is as expected.***

This could be worked around within NixOS outright, by having the
`makeModulesClosure` function check for `modules` output, but that might
be a worse workaround, and may cause weirder breakage. It especially
will make it much weirder to wrap one's head around `makeModulesClosure`.

Signed-off-by: Samuel Dionne-Riel <samuel@dionne-riel.com>
@r-vdp
Copy link
Contributor

r-vdp commented Aug 25, 2025

FYI, this broke nixosTests.systemd-boot.no-bootspec, since the synthesize command of the bootspec package expects to find $toplevel/kernel-modules/bzImage.
It should probably look at $toplevel/kernel instead, but that will require patching bootspec.

r-vdp added a commit to r-vdp/lanzaboote that referenced this pull request Aug 26, 2025
Instead of `$toplevel/kernel-modules/bzImage`, we use `$toplevel/kernel`
to find the kernel image of a generation.
@r-vdp r-vdp mentioned this pull request Aug 26, 2025
13 tasks
@r-vdp
Copy link
Contributor

r-vdp commented Aug 26, 2025

FYI, this broke nixosTests.systemd-boot.no-bootspec, since the synthesize command of the bootspec package expects to find $toplevel/kernel-modules/bzImage. It should probably look at $toplevel/kernel instead, but that will require patching bootspec.

Fix in #437139

r-vdp added a commit to r-vdp/bootspec that referenced this pull request Aug 26, 2025
cole-h pushed a commit to DeterminateSystems/bootspec that referenced this pull request Aug 26, 2025
Nixpkgs changed the location of the kernel image in NixOS/nixpkgs#423933.
@digitalrane digitalrane mentioned this pull request Aug 27, 2025
3 tasks
Mic92 added a commit to Mic92/nixpkgs that referenced this pull request Sep 1, 2025
Since NixOS#423933
nixpkgs kernel have a seperate "modules" output for kernel.
This check now acts similar to lib.getBin/lib.getLib etc.
@Mic92
Copy link
Member

Mic92 commented Sep 1, 2025

Here is a proposal to make aggregateModules behave the same as before:

#439178

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/patching-an-in-tree-linux-kernel-module/22137/3

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

Labels

6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

Comments