| Message ID | 20210413122218.GA5777@darth.lan |
|---|---|
| State | Accepted |
| Delegated to: | Paul Spooren |
| Headers | show |
| Series | build: fix regression for kernels < 5.10 | expand |
On 2021-04-13 14:22, Sebastian Kemper wrote: > This fixes a regression introduced with commit > 5ed1e5140a80558ab47fd70410ae3242bed5becf ("build: build kernel image > before building modules/packages"). > > Before this commit the make target would always include "modules", > resulting in a MODPOST and a complete Module.symvers file. Since this > commit a MODPOST of the kernel modules is not guaranteed for kernels < > 5.10. This results in some broken SDKs in which external packages that > depend on exported symbols from kernel modules fail to compile. Why is it not enough to do this in the CompileModules step? > Adding "modules" back to the calls to the CompileImage defines fixes the > regression. For kernels > 5.10 this is not needed, but it doesn't cause > any harm either. Why is >5.10 not affected? Can we backport the fix? I'd like to avoid adding extra unnecessary build step that slow down running make target/install. > Tested with kernels 5.4.x and 5.10.x. > > Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net> > --- > include/kernel-defaults.mk | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/kernel-defaults.mk b/include/kernel-defaults.mk > index 4b0b136a03..1b3b4497a2 100644 > --- a/include/kernel-defaults.mk > +++ b/include/kernel-defaults.mk > @@ -147,12 +147,17 @@ define Kernel/CopyImage > } > endef > > +# Always add "modules" so a proper Module.symvers file is written that > +# also contains symbols from the kernel modules. Without these symbols > +# external packages that depend on exported symbols from kernel modules > +# will fail to build. > define Kernel/CompileImage/Default > rm -f $(TARGET_DIR)/init > - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) > + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) modules > $(call Kernel/CopyImage) > endef > > +# Here as well, always add "modules", see comment above. > ifneq ($(CONFIG_TARGET_ROOTFS_INITRAMFS),) > define Kernel/CompileImage/Initramfs > $(call Kernel/Configure/Initramfs) > @@ -173,7 +178,7 @@ endif > # ? $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_LZ4),) > $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_ZSTD),$(STAGING_DIR_HOST)/bin/zstd -T0 -f -o $(KERNEL_BUILD_DIR)/initrd.cpio.zstd $(KERNEL_BUILD_DIR)/initrd.cpio) > endif > - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) > + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) modules Why do this for initramfs as well? - Felix
On Tue, Apr 13, 2021 at 03:09:24PM +0200, Felix Fietkau wrote: > > On 2021-04-13 14:22, Sebastian Kemper wrote: > > This fixes a regression introduced with commit > > 5ed1e5140a80558ab47fd70410ae3242bed5becf ("build: build kernel image > > before building modules/packages"). > > > > Before this commit the make target would always include "modules", > > resulting in a MODPOST and a complete Module.symvers file. Since this > > commit a MODPOST of the kernel modules is not guaranteed for kernels < > > 5.10. This results in some broken SDKs in which external packages that > > depend on exported symbols from kernel modules fail to compile. > Why is it not enough to do this in the CompileModules step? Because Module.symvers gets regenerated during CompileImage as well. > > > Adding "modules" back to the calls to the CompileImage defines fixes the > > regression. For kernels > 5.10 this is not needed, but it doesn't cause > > any harm either. > Why is >5.10 not affected? Can we backport the fix? I'd like to avoid > adding extra unnecessary build step that slow down running make > target/install. > You added 5ed1e5140a80558ab47fd70410ae3242bed5becf because of changes in this area. I imagine one would need to look there. I don't see adding "modules" back slowing anything down, because the modules are already built when CompileImage is run. "modules" just makes sure on kernels < 5.10 that MODPOST is run and the Module.symvers file is complete. > > Tested with kernels 5.4.x and 5.10.x. > > > > Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net> > > --- > > include/kernel-defaults.mk | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/include/kernel-defaults.mk b/include/kernel-defaults.mk > > index 4b0b136a03..1b3b4497a2 100644 > > --- a/include/kernel-defaults.mk > > +++ b/include/kernel-defaults.mk > > @@ -147,12 +147,17 @@ define Kernel/CopyImage > > } > > endef > > > > +# Always add "modules" so a proper Module.symvers file is written that > > +# also contains symbols from the kernel modules. Without these symbols > > +# external packages that depend on exported symbols from kernel modules > > +# will fail to build. > > define Kernel/CompileImage/Default > > rm -f $(TARGET_DIR)/init > > - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) > > + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) modules > > $(call Kernel/CopyImage) > > endef > > > > +# Here as well, always add "modules", see comment above. > > ifneq ($(CONFIG_TARGET_ROOTFS_INITRAMFS),) > > define Kernel/CompileImage/Initramfs > > $(call Kernel/Configure/Initramfs) > > @@ -173,7 +178,7 @@ endif > > # ? $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_LZ4),) > > $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_ZSTD),$(STAGING_DIR_HOST)/bin/zstd -T0 -f -o $(KERNEL_BUILD_DIR)/initrd.cpio.zstd $(KERNEL_BUILD_DIR)/initrd.cpio) > > endif > > - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) > > + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) modules > Why do this for initramfs as well? Again, because Module.symvers is regenerated. And whatever Module.symvers file is the last one to be generated will make its way into the SDK. > > - Felix
diff --git a/include/kernel-defaults.mk b/include/kernel-defaults.mk index 4b0b136a03..1b3b4497a2 100644 --- a/include/kernel-defaults.mk +++ b/include/kernel-defaults.mk @@ -147,12 +147,17 @@ define Kernel/CopyImage } endef +# Always add "modules" so a proper Module.symvers file is written that +# also contains symbols from the kernel modules. Without these symbols +# external packages that depend on exported symbols from kernel modules +# will fail to build. define Kernel/CompileImage/Default rm -f $(TARGET_DIR)/init - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) modules $(call Kernel/CopyImage) endef +# Here as well, always add "modules", see comment above. ifneq ($(CONFIG_TARGET_ROOTFS_INITRAMFS),) define Kernel/CompileImage/Initramfs $(call Kernel/Configure/Initramfs) @@ -173,7 +178,7 @@ endif # ? $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_LZ4),) $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_ZSTD),$(STAGING_DIR_HOST)/bin/zstd -T0 -f -o $(KERNEL_BUILD_DIR)/initrd.cpio.zstd $(KERNEL_BUILD_DIR)/initrd.cpio) endif - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if $(KERNELNAME),$(KERNELNAME),all) modules $(call Kernel/CopyImage,-initramfs) endef else
This fixes a regression introduced with commit 5ed1e5140a80558ab47fd70410ae3242bed5becf ("build: build kernel image before building modules/packages"). Before this commit the make target would always include "modules", resulting in a MODPOST and a complete Module.symvers file. Since this commit a MODPOST of the kernel modules is not guaranteed for kernels < 5.10. This results in some broken SDKs in which external packages that depend on exported symbols from kernel modules fail to compile. Adding "modules" back to the calls to the CompileImage defines fixes the regression. For kernels > 5.10 this is not needed, but it doesn't cause any harm either. Tested with kernels 5.4.x and 5.10.x. Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net> --- include/kernel-defaults.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 2.20.1