* [PATCH v2] x86/purgatory: Remove LTO flags
@ 2023-09-08 23:12 Song Liu
2023-09-11 15:59 ` Nick Desaulniers
2023-09-14 16:30 ` Song Liu
0 siblings, 2 replies; 6+ messages in thread
From: Song Liu @ 2023-09-08 23:12 UTC (permalink / raw)
To: linux-kernel
Cc: ndesaulniers, Song Liu, Ricardo Ribalda, Sami Tolvanen, kexec,
x86, llvm
With LTO enabled, ld.lld generates multiple .text sections for
purgatory.ro:
$ readelf -S purgatory.ro | grep " .text"
[ 1] .text PROGBITS 0000000000000000 00000040
[ 7] .text.purgatory PROGBITS 0000000000000000 000020e0
[ 9] .text.warn PROGBITS 0000000000000000 000021c0
[13] .text.sha256_upda PROGBITS 0000000000000000 000022f0
[15] .text.sha224_upda PROGBITS 0000000000000000 00002be0
[17] .text.sha256_fina PROGBITS 0000000000000000 00002bf0
[19] .text.sha224_fina PROGBITS 0000000000000000 00002cc0
This cause WARNING from kexec_purgatory_setup_sechdrs():
WARNING: CPU: 26 PID: 110894 at kernel/kexec_file.c:919
kexec_load_purgatory+0x37f/0x390
Fix this by disabling LTO for purgatory.
Fixes: 8652d44f466a ("kexec: support purgatories with .text.hot sections")
Cc: Ricardo Ribalda <ribalda@chromium.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: kexec@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: x86@kernel.org
Cc: llvm@lists.linux.dev
Signed-off-by: Song Liu <song@kernel.org>
---
AFAICT, x86 is the only arch that supports LTO and purgatory.
Changes in v2:
1. Use CC_FLAGS_LTO instead of hardcode -flto. (Nick Desaulniers)
---
arch/x86/purgatory/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index c2a29be35c01..08aa0f25f12a 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -19,6 +19,10 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS -D__NO_FORTIFY
# optimization flags.
KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,$(KBUILD_CFLAGS))
+# When LTO is enabled, llvm emits many text sections, which is not supported
+# by kexec. Remove -flto=* flags.
+KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO),$(KBUILD_CFLAGS))
+
# When linking purgatory.ro with -r unresolved symbols are not checked,
# also link a purgatory.chk binary without -r to check for unresolved symbols.
PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/purgatory: Remove LTO flags
2023-09-08 23:12 [PATCH v2] x86/purgatory: Remove LTO flags Song Liu
@ 2023-09-11 15:59 ` Nick Desaulniers
2023-09-11 16:31 ` Song Liu
2023-09-14 16:30 ` Song Liu
1 sibling, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2023-09-11 15:59 UTC (permalink / raw)
To: Song Liu; +Cc: linux-kernel, Ricardo Ribalda, Sami Tolvanen, kexec, x86, llvm
On Fri, Sep 8, 2023 at 4:13 PM Song Liu <song@kernel.org> wrote:
>
> With LTO enabled, ld.lld generates multiple .text sections for
> purgatory.ro:
>
> $ readelf -S purgatory.ro | grep " .text"
> [ 1] .text PROGBITS 0000000000000000 00000040
> [ 7] .text.purgatory PROGBITS 0000000000000000 000020e0
> [ 9] .text.warn PROGBITS 0000000000000000 000021c0
> [13] .text.sha256_upda PROGBITS 0000000000000000 000022f0
> [15] .text.sha224_upda PROGBITS 0000000000000000 00002be0
> [17] .text.sha256_fina PROGBITS 0000000000000000 00002bf0
> [19] .text.sha224_fina PROGBITS 0000000000000000 00002cc0
>
> This cause WARNING from kexec_purgatory_setup_sechdrs():
>
> WARNING: CPU: 26 PID: 110894 at kernel/kexec_file.c:919
> kexec_load_purgatory+0x37f/0x390
>
> Fix this by disabling LTO for purgatory.
Thanks for the v2!
>
> Fixes: 8652d44f466a ("kexec: support purgatories with .text.hot sections")
Dunno that this fixes tag is precise. I think perhaps
Fixes: b33fff07e3e3 ("x86, build: allow LTO to be selected")
would be more precise.
> Cc: Ricardo Ribalda <ribalda@chromium.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: kexec@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: x86@kernel.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Song Liu <song@kernel.org>
>
> ---
> AFAICT, x86 is the only arch that supports LTO and purgatory.
>
> Changes in v2:
> 1. Use CC_FLAGS_LTO instead of hardcode -flto. (Nick Desaulniers)
> ---
> arch/x86/purgatory/Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index c2a29be35c01..08aa0f25f12a 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -19,6 +19,10 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS -D__NO_FORTIFY
> # optimization flags.
> KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,$(KBUILD_CFLAGS))
>
> +# When LTO is enabled, llvm emits many text sections, which is not supported
> +# by kexec. Remove -flto=* flags.
-flto* in LLVM implies -ffunction-sections, which creates a .text.<fn
name> section per function definition to facilitate -Wl,--gc-sections.
Overall the question here is "do we really need to optimize kexec?"
If the answer is yes, then this patch AND 8652d44f466a are both
pessimizing kexec (though having it work at all is strictly better
than not at all). The best fix IMO would be to provide a linker
script for this purgatory image that rejoins the text sections back
into one .text. For example:
commit eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections")
I assume people do care about the time to kexec, hence the raison
d'etre for projects like kpatch.
I'm fine to sign off on this approach if we don't really care, or want
to care more later, but we can do better here.
> +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO),$(KBUILD_CFLAGS))
> +
> # When linking purgatory.ro with -r unresolved symbols are not checked,
> # also link a purgatory.chk binary without -r to check for unresolved symbols.
> PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
> --
> 2.34.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/purgatory: Remove LTO flags
2023-09-11 15:59 ` Nick Desaulniers
@ 2023-09-11 16:31 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2023-09-11 16:31 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-kernel, Ricardo Ribalda, Sami Tolvanen, kexec, x86, llvm
On Mon, Sep 11, 2023 at 9:00 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Sep 8, 2023 at 4:13 PM Song Liu <song@kernel.org> wrote:
> >
> > With LTO enabled, ld.lld generates multiple .text sections for
> > purgatory.ro:
> >
> > $ readelf -S purgatory.ro | grep " .text"
> > [ 1] .text PROGBITS 0000000000000000 00000040
> > [ 7] .text.purgatory PROGBITS 0000000000000000 000020e0
> > [ 9] .text.warn PROGBITS 0000000000000000 000021c0
> > [13] .text.sha256_upda PROGBITS 0000000000000000 000022f0
> > [15] .text.sha224_upda PROGBITS 0000000000000000 00002be0
> > [17] .text.sha256_fina PROGBITS 0000000000000000 00002bf0
> > [19] .text.sha224_fina PROGBITS 0000000000000000 00002cc0
> >
> > This cause WARNING from kexec_purgatory_setup_sechdrs():
> >
> > WARNING: CPU: 26 PID: 110894 at kernel/kexec_file.c:919
> > kexec_load_purgatory+0x37f/0x390
> >
> > Fix this by disabling LTO for purgatory.
>
> Thanks for the v2!
>
> >
> > Fixes: 8652d44f466a ("kexec: support purgatories with .text.hot sections")
>
> Dunno that this fixes tag is precise. I think perhaps
>
> Fixes: b33fff07e3e3 ("x86, build: allow LTO to be selected")
Thanks for the correction!
>
> would be more precise.
>
> > Cc: Ricardo Ribalda <ribalda@chromium.org>
> > Cc: Sami Tolvanen <samitolvanen@google.com>
> > Cc: kexec@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: x86@kernel.org
> > Cc: llvm@lists.linux.dev
> > Signed-off-by: Song Liu <song@kernel.org>
> >
> > ---
> > AFAICT, x86 is the only arch that supports LTO and purgatory.
> >
> > Changes in v2:
> > 1. Use CC_FLAGS_LTO instead of hardcode -flto. (Nick Desaulniers)
> > ---
> > arch/x86/purgatory/Makefile | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> > index c2a29be35c01..08aa0f25f12a 100644
> > --- a/arch/x86/purgatory/Makefile
> > +++ b/arch/x86/purgatory/Makefile
> > @@ -19,6 +19,10 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS -D__NO_FORTIFY
> > # optimization flags.
> > KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,$(KBUILD_CFLAGS))
> >
> > +# When LTO is enabled, llvm emits many text sections, which is not supported
> > +# by kexec. Remove -flto=* flags.
>
> -flto* in LLVM implies -ffunction-sections, which creates a .text.<fn
> name> section per function definition to facilitate -Wl,--gc-sections.
>
> Overall the question here is "do we really need to optimize kexec?"
>
> If the answer is yes, then this patch AND 8652d44f466a are both
> pessimizing kexec (though having it work at all is strictly better
> than not at all). The best fix IMO would be to provide a linker
> script for this purgatory image that rejoins the text sections back
> into one .text. For example:
>
> commit eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections")
>
> I assume people do care about the time to kexec, hence the raison
> d'etre for projects like kpatch.
AFAICT, optimizations like LTO and PGO can give a few % of
improvement, which is probably not important for kexec. The benefit
is in the order of seconds (or less?). The benefit of kpatch is that we
can keep the workload running while fixing the kernel bug. Based on
our experience at Meta, it may take hours to graceful shutdown the
application to run kexec. In this case, a few seconds of improvement
(via LTO/PGO purgatory) doesn't save us much.
Thanks,
Song
>
> I'm fine to sign off on this approach if we don't really care, or want
> to care more later, but we can do better here.
>
> > +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO),$(KBUILD_CFLAGS))
> > +
> > # When linking purgatory.ro with -r unresolved symbols are not checked,
> > # also link a purgatory.chk binary without -r to check for unresolved symbols.
> > PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/purgatory: Remove LTO flags
2023-09-08 23:12 [PATCH v2] x86/purgatory: Remove LTO flags Song Liu
2023-09-11 15:59 ` Nick Desaulniers
@ 2023-09-14 16:30 ` Song Liu
2023-09-14 16:34 ` Nick Desaulniers
1 sibling, 1 reply; 6+ messages in thread
From: Song Liu @ 2023-09-14 16:30 UTC (permalink / raw)
To: linux-kernel
Cc: ndesaulniers, Ricardo Ribalda, Sami Tolvanen, kexec, x86, llvm
Hi folks,
On Fri, Sep 8, 2023 at 4:13 PM Song Liu <song@kernel.org> wrote:
>
> With LTO enabled, ld.lld generates multiple .text sections for
> purgatory.ro:
>
> $ readelf -S purgatory.ro | grep " .text"
> [ 1] .text PROGBITS 0000000000000000 00000040
> [ 7] .text.purgatory PROGBITS 0000000000000000 000020e0
> [ 9] .text.warn PROGBITS 0000000000000000 000021c0
> [13] .text.sha256_upda PROGBITS 0000000000000000 000022f0
> [15] .text.sha224_upda PROGBITS 0000000000000000 00002be0
> [17] .text.sha256_fina PROGBITS 0000000000000000 00002bf0
> [19] .text.sha224_fina PROGBITS 0000000000000000 00002cc0
>
> This cause WARNING from kexec_purgatory_setup_sechdrs():
>
> WARNING: CPU: 26 PID: 110894 at kernel/kexec_file.c:919
> kexec_load_purgatory+0x37f/0x390
>
> Fix this by disabling LTO for purgatory.
>
> Fixes: 8652d44f466a ("kexec: support purgatories with .text.hot sections")
> Cc: Ricardo Ribalda <ribalda@chromium.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: kexec@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: x86@kernel.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Song Liu <song@kernel.org>
What would be the next step for this work? We hope to back port it
to our production kernel soon.
Thanks,
Song
>
> ---
> AFAICT, x86 is the only arch that supports LTO and purgatory.
>
> Changes in v2:
> 1. Use CC_FLAGS_LTO instead of hardcode -flto. (Nick Desaulniers)
> ---
> arch/x86/purgatory/Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index c2a29be35c01..08aa0f25f12a 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -19,6 +19,10 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS -D__NO_FORTIFY
> # optimization flags.
> KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,$(KBUILD_CFLAGS))
>
> +# When LTO is enabled, llvm emits many text sections, which is not supported
> +# by kexec. Remove -flto=* flags.
> +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO),$(KBUILD_CFLAGS))
> +
> # When linking purgatory.ro with -r unresolved symbols are not checked,
> # also link a purgatory.chk binary without -r to check for unresolved symbols.
> PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/purgatory: Remove LTO flags
2023-09-14 16:30 ` Song Liu
@ 2023-09-14 16:34 ` Nick Desaulniers
2023-09-14 17:02 ` Song Liu
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2023-09-14 16:34 UTC (permalink / raw)
To: Song Liu; +Cc: linux-kernel, Ricardo Ribalda, Sami Tolvanen, kexec, x86, llvm
On Thu, Sep 14, 2023 at 9:31 AM Song Liu <song@kernel.org> wrote:
>
> What would be the next step for this work? We hope to back port it
> to our production kernel soon.
Please send a v3 with the fixes tag updated. I wouldn't mind if you
added a comment to the commit message to the effect of:
We could also add the use of an explicit linker script to rejoin
.text.* sections back into .text. Simply disable the production of
more .text.* sections for now; -flto* implies -ffunction-sections.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] x86/purgatory: Remove LTO flags
2023-09-14 16:34 ` Nick Desaulniers
@ 2023-09-14 17:02 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2023-09-14 17:02 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-kernel, Ricardo Ribalda, Sami Tolvanen, kexec, x86, llvm
On Thu, Sep 14, 2023 at 9:34 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Sep 14, 2023 at 9:31 AM Song Liu <song@kernel.org> wrote:
> >
> > What would be the next step for this work? We hope to back port it
> > to our production kernel soon.
>
> Please send a v3 with the fixes tag updated. I wouldn't mind if you
> added a comment to the commit message to the effect of:
>
> We could also add the use of an explicit linker script to rejoin
> .text.* sections back into .text. Simply disable the production of
> more .text.* sections for now; -flto* implies -ffunction-sections.
Got it. I just sent v3 with these changes.
Thanks,
Song
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-14 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-08 23:12 [PATCH v2] x86/purgatory: Remove LTO flags Song Liu
2023-09-11 15:59 ` Nick Desaulniers
2023-09-11 16:31 ` Song Liu
2023-09-14 16:30 ` Song Liu
2023-09-14 16:34 ` Nick Desaulniers
2023-09-14 17:02 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox