| Message ID | 20230822192315.8180-1-ju.o@free.fr |
|---|---|
| State | New |
| Headers | show |
| Series | [next,1/1] package/kexec: fix link error when BR2_OPTIMIZE_0=y | expand |
On Tue, 22 Aug 2023 21:23:14 +0200 Julien Olivain <ju.o@free.fr> wrote: > When BR2_OPTIMIZE_0, -O0 is passed in compiler CFLAGS. This means no > code optimization will be performed. > > kexec code uses a trick to detect unaligned accesses at link time > which needs at least dead-code-removal to work. See > put/get_unaligned() macros in kexec/kexec.h. This code was re-enabled > in upstream commit [1]. > > This commit sets at least -O1 (which include the sufficient > dead-code-removal) when BR2_OPTIMIZE_0=y, to fix those issues. > > Fixes: > - http://autobuild.buildroot.org/results/8f8/8f8532f1dfbd71e52c51c00118934af9fa45e7cb > - http://autobuild.buildroot.org/results/528/528fd7baf9b0ad5549d22ec8e0623c5fa1f2d117 > - http://autobuild.buildroot.org/results/499/499115439680adfb4b40042468e5bbb65d91ce6c > - ...and many others > > [1] https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/commit/?id=0723defb5308ac7fce296f8b596bff4df6803f01 > > Signed-off-by: Julien Olivain <ju.o@free.fr> > --- > Patch tested on branch next at commit bfa4a7c with commands: Thanks for the research, but why is this tagged for next? You're saying the issue was introduced in kexec-tools commit 0723defb5308ac7fce296f8b596bff4df6803f01, and this commit was initially part of the 2.0.1 release of kexec-tools, which we have in Buildroot since February 2010. Based on that, I would have expected this patch to be relevant for our master branch, but you explicitly say it should be merged in next, which to me doesn't make sense. Could you clarify? Thanks a lot! Thomas
Hi Thomas, On 22/08/2023 21:35, Thomas Petazzoni wrote: > On Tue, 22 Aug 2023 21:23:14 +0200 > Julien Olivain <ju.o@free.fr> wrote: > >> When BR2_OPTIMIZE_0, -O0 is passed in compiler CFLAGS. This means no >> code optimization will be performed. >> >> kexec code uses a trick to detect unaligned accesses at link time >> which needs at least dead-code-removal to work. See >> put/get_unaligned() macros in kexec/kexec.h. This code was re-enabled >> in upstream commit [1]. >> >> This commit sets at least -O1 (which include the sufficient >> dead-code-removal) when BR2_OPTIMIZE_0=y, to fix those issues. >> >> Fixes: >> - >> http://autobuild.buildroot.org/results/8f8/8f8532f1dfbd71e52c51c00118934af9fa45e7cb >> - >> http://autobuild.buildroot.org/results/528/528fd7baf9b0ad5549d22ec8e0623c5fa1f2d117 >> - >> http://autobuild.buildroot.org/results/499/499115439680adfb4b40042468e5bbb65d91ce6c >> - ...and many others >> >> [1] >> https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/commit/?id=0723defb5308ac7fce296f8b596bff4df6803f01 >> >> Signed-off-by: Julien Olivain <ju.o@free.fr> >> --- >> Patch tested on branch next at commit bfa4a7c with commands: > > Thanks for the research, but why is this tagged for next? You're saying > the issue was introduced in kexec-tools commit > 0723defb5308ac7fce296f8b596bff4df6803f01, and this commit was initially > part of the 2.0.1 release of kexec-tools, which we have in Buildroot > since February 2010. > > Based on that, I would have expected this patch to be relevant for our > master branch, but you explicitly say it should be merged in next, > which to me doesn't make sense. Could you clarify? This could indeed go into the master branch. Since 2023.08-rc2 was already tagged, I imagined only major fixes would go into master. This one has been longstanding issue (so apparently not so important). In fact, reading the Buildroot documentation release engineering at [1] does really allow me to decide what kind of commits (security, bugfixes, new features, minor improvements like typos) should go to master/next. Recently, I had the same interpretation in [2] that a minor improvement (in which nothing was actually broken) should go to next. Yann preferred master. If you summarize those situations, I would be happy to propose a doc improvement. > Thanks a lot! > > Thomas [1] https://nightly.buildroot.org/manual.html#_development [2] https://lists.buildroot.org/pipermail/buildroot/2023-August/672866.html Best regard, Julien.
On Tue, 22 Aug 2023 22:08:32 +0200 Julien Olivain <ju.o@free.fr> wrote: > This could indeed go into the master branch. Since 2023.08-rc2 was > already tagged, I imagined only major fixes would go into master. > This one has been longstanding issue (so apparently not so important). > > In fact, reading the Buildroot documentation release engineering at [1] > does really allow me to decide what kind of commits (security, bugfixes, > new features, minor improvements like typos) should go to master/next. > > Recently, I had the same interpretation in [2] that a minor improvement > (in which nothing was actually broken) should go to next. Yann preferred > master. > > If you summarize those situations, I would be happy to propose a doc > improvement. Essentially, the idea after -rc1 is to stop applying version bumps and major changes in master so that master progressively converges to something "stable" that we can release. So, we can still merge improvements and fixes to master. As such, the build fix for kexec definitely qualifies. The license file improvement for lsof that you pointed also qualifies: it's an improvement, very low risk. Of course, there's always a grey area, even for version bumps: sometimes we take a version bump in master because it fixes security issues. Sometimes we prefer to backport the patch fixing the security issue rather than bumping when the number of other changes that take place with the version bump is too large. Does that help? :-) Thomas
Hi Thomas, On 22/08/2023 22:35, Thomas Petazzoni wrote: > On Tue, 22 Aug 2023 22:08:32 +0200 > Julien Olivain <ju.o@free.fr> wrote: > >> This could indeed go into the master branch. Since 2023.08-rc2 was >> already tagged, I imagined only major fixes would go into master. >> This one has been longstanding issue (so apparently not so important). >> >> In fact, reading the Buildroot documentation release engineering at >> [1] >> does really allow me to decide what kind of commits (security, >> bugfixes, >> new features, minor improvements like typos) should go to master/next. >> >> Recently, I had the same interpretation in [2] that a minor >> improvement >> (in which nothing was actually broken) should go to next. Yann >> preferred >> master. >> >> If you summarize those situations, I would be happy to propose a doc >> improvement. > > Essentially, the idea after -rc1 is to stop applying version bumps and > major changes in master so that master progressively converges to > something "stable" that we can release. So, we can still merge > improvements and fixes to master. As such, the build fix for kexec > definitely qualifies. The license file improvement for lsof that you > pointed also qualifies: it's an improvement, very low risk. Of course, > there's always a grey area, even for version bumps: sometimes we take a > version bump in master because it fixes security issues. Sometimes we > prefer to backport the patch fixing the security issue rather than > bumping when the number of other changes that take place with the > version bump is too large. > > Does that help? :-) Yes it does! From now on, I'll also include minor improvements and bugfixes to master. Thanks for the clarification. > > Thomas Best regards, Julien.
diff --git a/package/kexec/kexec.mk b/package/kexec/kexec.mk index 562b09012b..6c7a6778ae 100644 --- a/package/kexec/kexec.mk +++ b/package/kexec/kexec.mk @@ -16,6 +16,14 @@ KEXEC_SELINUX_MODULES = kdump # Makefile expects $STRIP -o to work, so needed for !BR2_STRIP_strip KEXEC_MAKE_OPTS = STRIP="$(TARGET_CROSS)strip" +# kexec requires at least -O1 optimization level. Its code uses a +# trick to detect unaligned accesses at link time which needs at least +# dead-code-removal to work. See put/get_unaligned() macros in +# kexec/kexec.h +ifeq ($(BR2_OPTIMIZE_0),y) +KEXEC_CONF_ENV += CFLAGS="$(TARGET_CFLAGS) -O1" +endif + ifeq ($(BR2_PACKAGE_KEXEC_ZLIB),y) KEXEC_CONF_OPTS += --with-zlib KEXEC_DEPENDENCIES += zlib
When BR2_OPTIMIZE_0, -O0 is passed in compiler CFLAGS. This means no code optimization will be performed. kexec code uses a trick to detect unaligned accesses at link time which needs at least dead-code-removal to work. See put/get_unaligned() macros in kexec/kexec.h. This code was re-enabled in upstream commit [1]. This commit sets at least -O1 (which include the sufficient dead-code-removal) when BR2_OPTIMIZE_0=y, to fix those issues. Fixes: - http://autobuild.buildroot.org/results/8f8/8f8532f1dfbd71e52c51c00118934af9fa45e7cb - http://autobuild.buildroot.org/results/528/528fd7baf9b0ad5549d22ec8e0623c5fa1f2d117 - http://autobuild.buildroot.org/results/499/499115439680adfb4b40042468e5bbb65d91ce6c - ...and many others [1] https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/commit/?id=0723defb5308ac7fce296f8b596bff4df6803f01 Signed-off-by: Julien Olivain <ju.o@free.fr> --- Patch tested on branch next at commit bfa4a7c with commands: make check-package ... 0 warnings generated support/testing/run-tests \ -d dl -o output_folder \ tests.package.test_kexec ... OK --- package/kexec/kexec.mk | 8 ++++++++ 1 file changed, 8 insertions(+)