* [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too @ 2020-06-17 21:06 Nick Desaulniers 2020-06-17 21:06 ` Nick Desaulniers 2020-06-17 21:27 ` Fāng-ruì Sòng 0 siblings, 2 replies; 15+ messages in thread From: Nick Desaulniers @ 2020-06-17 21:06 UTC (permalink / raw) To: Arnd Bergmann Cc: clang-built-linux, Kees Cook, Nick Desaulniers, stable, Jian Cai, Luis Lozano, Fāng-ruì Sòng, Manoj Gupta, linux-arch, linux-kernel ld.bfd's internal linker script considers .text.hot AND .text.hot.* to be part of .text, as well as .text.unlikely and .text.unlikely.*. ld.lld will produce .text.hot.*/.text.unlikely.* sections. Make sure to group these together. Otherwise these orphan sections may be placed outside of the the _stext/_etext boundaries. Cc: stable@vger.kernel.org Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 Link: https://reviews.llvm.org/D79600 Reported-by: Jian Cai <jiancai@google.com> Debugged-by: Luis Lozano <llozano@google.com> Suggested-by: Fāng-ruì Sòng <maskray@google.com> Tested-by: Luis Lozano <llozano@google.com> Tested-by: Manoj Gupta <manojgupta@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- include/asm-generic/vmlinux.lds.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index d7c7c7f36c4a..fe5aaef169e3 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -560,7 +560,9 @@ */ #define TEXT_TEXT \ ALIGN_FUNCTION(); \ - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ + *(.text.hot .text.hot.*) \ + *(TEXT_MAIN .text.fixup) \ + *(.text.unlikely .text.unlikely.*) \ NOINSTR_TEXT \ *(.text..refcount) \ *(.ref.text) \ -- 2.27.0.290.gba653c62da-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-17 21:06 [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too Nick Desaulniers @ 2020-06-17 21:06 ` Nick Desaulniers 2020-06-17 21:27 ` Fāng-ruì Sòng 1 sibling, 0 replies; 15+ messages in thread From: Nick Desaulniers @ 2020-06-17 21:06 UTC (permalink / raw) To: Arnd Bergmann Cc: clang-built-linux, Kees Cook, Nick Desaulniers, stable, Jian Cai, Luis Lozano, Fāng-ruì Sòng, Manoj Gupta, linux-arch, linux-kernel ld.bfd's internal linker script considers .text.hot AND .text.hot.* to be part of .text, as well as .text.unlikely and .text.unlikely.*. ld.lld will produce .text.hot.*/.text.unlikely.* sections. Make sure to group these together. Otherwise these orphan sections may be placed outside of the the _stext/_etext boundaries. Cc: stable@vger.kernel.org Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 Link: https://reviews.llvm.org/D79600 Reported-by: Jian Cai <jiancai@google.com> Debugged-by: Luis Lozano <llozano@google.com> Suggested-by: Fāng-ruì Sòng <maskray@google.com> Tested-by: Luis Lozano <llozano@google.com> Tested-by: Manoj Gupta <manojgupta@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- include/asm-generic/vmlinux.lds.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index d7c7c7f36c4a..fe5aaef169e3 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -560,7 +560,9 @@ */ #define TEXT_TEXT \ ALIGN_FUNCTION(); \ - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ + *(.text.hot .text.hot.*) \ + *(TEXT_MAIN .text.fixup) \ + *(.text.unlikely .text.unlikely.*) \ NOINSTR_TEXT \ *(.text..refcount) \ *(.ref.text) \ -- 2.27.0.290.gba653c62da-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-17 21:06 [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too Nick Desaulniers 2020-06-17 21:06 ` Nick Desaulniers @ 2020-06-17 21:27 ` Fāng-ruì Sòng 2020-06-17 21:27 ` Fāng-ruì Sòng 2020-06-22 23:04 ` Nick Desaulniers 1 sibling, 2 replies; 15+ messages in thread From: Fāng-ruì Sòng @ 2020-06-17 21:27 UTC (permalink / raw) To: Nick Desaulniers Cc: Arnd Bergmann, clang-built-linux, Kees Cook, stable, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, linux-kernel On 2020-06-17, Nick Desaulniers wrote: >ld.bfd's internal linker script considers .text.hot AND .text.hot.* to >be part of .text, as well as .text.unlikely and .text.unlikely.*. >ld.lld will produce .text.hot.*/.text.unlikely.* sections. Correction to this sentence. lld is not relevant here. -ffunction-sections combined with profile-guided optimization can produce .text.hot.* .text.unlikely.* sections. Newer clang may produce .text.hot. .text.unlikely. (without suffix, but with a trailing dot) when -fno-unique-section-names is specified, as an optimization to make .strtab smaller. We've already seen that GCC can place main in .text.startup without -ffunction-sections. There may be other non -ffunction-sections cases for .text.hot.* or .text.unlikely.*. So it is definitely a good idea to be more specific even if we don't care about -ffunction-sections for now. >Make sure to group these together. Otherwise these orphan sections may >be placed outside of the the _stext/_etext boundaries. > >Cc: stable@vger.kernel.org >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 >Link: https://reviews.llvm.org/D79600 >Reported-by: Jian Cai <jiancai@google.com> >Debugged-by: Luis Lozano <llozano@google.com> >Suggested-by: Fāng-ruì Sòng <maskray@google.com> >Tested-by: Luis Lozano <llozano@google.com> >Tested-by: Manoj Gupta <manojgupta@google.com> >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >--- > include/asm-generic/vmlinux.lds.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h >index d7c7c7f36c4a..fe5aaef169e3 100644 >--- a/include/asm-generic/vmlinux.lds.h >+++ b/include/asm-generic/vmlinux.lds.h >@@ -560,7 +560,9 @@ > */ > #define TEXT_TEXT \ > ALIGN_FUNCTION(); \ >- *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ >+ *(.text.hot .text.hot.*) \ >+ *(TEXT_MAIN .text.fixup) \ >+ *(.text.unlikely .text.unlikely.*) \ > NOINSTR_TEXT \ > *(.text..refcount) \ > *(.ref.text) \ >-- >2.27.0.290.gba653c62da-goog > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-17 21:27 ` Fāng-ruì Sòng @ 2020-06-17 21:27 ` Fāng-ruì Sòng 2020-06-22 23:04 ` Nick Desaulniers 1 sibling, 0 replies; 15+ messages in thread From: Fāng-ruì Sòng @ 2020-06-17 21:27 UTC (permalink / raw) To: Nick Desaulniers Cc: Arnd Bergmann, clang-built-linux, Kees Cook, stable, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, linux-kernel On 2020-06-17, Nick Desaulniers wrote: >ld.bfd's internal linker script considers .text.hot AND .text.hot.* to >be part of .text, as well as .text.unlikely and .text.unlikely.*. >ld.lld will produce .text.hot.*/.text.unlikely.* sections. Correction to this sentence. lld is not relevant here. -ffunction-sections combined with profile-guided optimization can produce .text.hot.* .text.unlikely.* sections. Newer clang may produce .text.hot. .text.unlikely. (without suffix, but with a trailing dot) when -fno-unique-section-names is specified, as an optimization to make .strtab smaller. We've already seen that GCC can place main in .text.startup without -ffunction-sections. There may be other non -ffunction-sections cases for .text.hot.* or .text.unlikely.*. So it is definitely a good idea to be more specific even if we don't care about -ffunction-sections for now. >Make sure to group these together. Otherwise these orphan sections may >be placed outside of the the _stext/_etext boundaries. > >Cc: stable@vger.kernel.org >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 >Link: https://reviews.llvm.org/D79600 >Reported-by: Jian Cai <jiancai@google.com> >Debugged-by: Luis Lozano <llozano@google.com> >Suggested-by: Fāng-ruì Sòng <maskray@google.com> >Tested-by: Luis Lozano <llozano@google.com> >Tested-by: Manoj Gupta <manojgupta@google.com> >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >--- > include/asm-generic/vmlinux.lds.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h >index d7c7c7f36c4a..fe5aaef169e3 100644 >--- a/include/asm-generic/vmlinux.lds.h >+++ b/include/asm-generic/vmlinux.lds.h >@@ -560,7 +560,9 @@ > */ > #define TEXT_TEXT \ > ALIGN_FUNCTION(); \ >- *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ >+ *(.text.hot .text.hot.*) \ >+ *(TEXT_MAIN .text.fixup) \ >+ *(.text.unlikely .text.unlikely.*) \ > NOINSTR_TEXT \ > *(.text..refcount) \ > *(.ref.text) \ >-- >2.27.0.290.gba653c62da-goog > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-17 21:27 ` Fāng-ruì Sòng 2020-06-17 21:27 ` Fāng-ruì Sòng @ 2020-06-22 23:04 ` Nick Desaulniers 2020-06-22 23:04 ` Nick Desaulniers 2020-06-22 23:15 ` Fāng-ruì Sòng 1 sibling, 2 replies; 15+ messages in thread From: Nick Desaulniers @ 2020-06-22 23:04 UTC (permalink / raw) To: Fāng-ruì Sòng Cc: Arnd Bergmann, clang-built-linux, Kees Cook, # 3.4.x, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML On Wed, Jun 17, 2020 at 2:27 PM Fāng-ruì Sòng <maskray@google.com> wrote: > > > On 2020-06-17, Nick Desaulniers wrote: > >ld.bfd's internal linker script considers .text.hot AND .text.hot.* to > >be part of .text, as well as .text.unlikely and .text.unlikely.*. > > >ld.lld will produce .text.hot.*/.text.unlikely.* sections. > > Correction to this sentence. lld is not relevant here. > > -ffunction-sections combined with profile-guided optimization can > produce .text.hot.* .text.unlikely.* sections. Newer clang may produce > .text.hot. .text.unlikely. (without suffix, but with a trailing dot) > when -fno-unique-section-names is specified, as an optimization to make > .strtab smaller. Then why was the bug report reporting https://reviews.llvm.org/D79600 as the result of a bisection, if LLD is not relevant? Was the bisection wrong? The upstream report wasn't initially public, for no good reason. So I didn't include it, but if we end up taking v1, this should have Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 The kernel doesn't use -fno-unique-section-names; is that another flag that's added by CrOS' compiler wrapper? https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/third_party/toolchain-utils/compiler_wrapper/config.go;l=110 Looks like no. It doesn't use `-fno-unique-section-names` or `-ffunction-sections`. > > We've already seen that GCC can place main in .text.startup without > -ffunction-sections. There may be other non -ffunction-sections cases > for .text.hot.* or .text.unlikely.*. So it is definitely a good idea to > be more specific even if we don't care about -ffunction-sections for > now. > > >Make sure to group these together. Otherwise these orphan sections may > >be placed outside of the the _stext/_etext boundaries. > > > >Cc: stable@vger.kernel.org > >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee > >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 > >Link: https://reviews.llvm.org/D79600 > >Reported-by: Jian Cai <jiancai@google.com> > >Debugged-by: Luis Lozano <llozano@google.com> > >Suggested-by: Fāng-ruì Sòng <maskray@google.com> > >Tested-by: Luis Lozano <llozano@google.com> > >Tested-by: Manoj Gupta <manojgupta@google.com> > >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > >--- > > include/asm-generic/vmlinux.lds.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > >index d7c7c7f36c4a..fe5aaef169e3 100644 > >--- a/include/asm-generic/vmlinux.lds.h > >+++ b/include/asm-generic/vmlinux.lds.h > >@@ -560,7 +560,9 @@ > > */ > > #define TEXT_TEXT \ > > ALIGN_FUNCTION(); \ > >- *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ > >+ *(.text.hot .text.hot.*) \ > >+ *(TEXT_MAIN .text.fixup) \ > >+ *(.text.unlikely .text.unlikely.*) \ > > NOINSTR_TEXT \ > > *(.text..refcount) \ > > *(.ref.text) \ > >-- > >2.27.0.290.gba653c62da-goog > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-22 23:04 ` Nick Desaulniers @ 2020-06-22 23:04 ` Nick Desaulniers 2020-06-22 23:15 ` Fāng-ruì Sòng 1 sibling, 0 replies; 15+ messages in thread From: Nick Desaulniers @ 2020-06-22 23:04 UTC (permalink / raw) To: Fāng-ruì Sòng Cc: Arnd Bergmann, clang-built-linux, Kees Cook, # 3.4.x, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML On Wed, Jun 17, 2020 at 2:27 PM Fāng-ruì Sòng <maskray@google.com> wrote: > > > On 2020-06-17, Nick Desaulniers wrote: > >ld.bfd's internal linker script considers .text.hot AND .text.hot.* to > >be part of .text, as well as .text.unlikely and .text.unlikely.*. > > >ld.lld will produce .text.hot.*/.text.unlikely.* sections. > > Correction to this sentence. lld is not relevant here. > > -ffunction-sections combined with profile-guided optimization can > produce .text.hot.* .text.unlikely.* sections. Newer clang may produce > .text.hot. .text.unlikely. (without suffix, but with a trailing dot) > when -fno-unique-section-names is specified, as an optimization to make > .strtab smaller. Then why was the bug report reporting https://reviews.llvm.org/D79600 as the result of a bisection, if LLD is not relevant? Was the bisection wrong? The upstream report wasn't initially public, for no good reason. So I didn't include it, but if we end up taking v1, this should have Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 The kernel doesn't use -fno-unique-section-names; is that another flag that's added by CrOS' compiler wrapper? https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/third_party/toolchain-utils/compiler_wrapper/config.go;l=110 Looks like no. It doesn't use `-fno-unique-section-names` or `-ffunction-sections`. > > We've already seen that GCC can place main in .text.startup without > -ffunction-sections. There may be other non -ffunction-sections cases > for .text.hot.* or .text.unlikely.*. So it is definitely a good idea to > be more specific even if we don't care about -ffunction-sections for > now. > > >Make sure to group these together. Otherwise these orphan sections may > >be placed outside of the the _stext/_etext boundaries. > > > >Cc: stable@vger.kernel.org > >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee > >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 > >Link: https://reviews.llvm.org/D79600 > >Reported-by: Jian Cai <jiancai@google.com> > >Debugged-by: Luis Lozano <llozano@google.com> > >Suggested-by: Fāng-ruì Sòng <maskray@google.com> > >Tested-by: Luis Lozano <llozano@google.com> > >Tested-by: Manoj Gupta <manojgupta@google.com> > >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > >--- > > include/asm-generic/vmlinux.lds.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > >index d7c7c7f36c4a..fe5aaef169e3 100644 > >--- a/include/asm-generic/vmlinux.lds.h > >+++ b/include/asm-generic/vmlinux.lds.h > >@@ -560,7 +560,9 @@ > > */ > > #define TEXT_TEXT \ > > ALIGN_FUNCTION(); \ > >- *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ > >+ *(.text.hot .text.hot.*) \ > >+ *(TEXT_MAIN .text.fixup) \ > >+ *(.text.unlikely .text.unlikely.*) \ > > NOINSTR_TEXT \ > > *(.text..refcount) \ > > *(.ref.text) \ > >-- > >2.27.0.290.gba653c62da-goog > > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-22 23:04 ` Nick Desaulniers 2020-06-22 23:04 ` Nick Desaulniers @ 2020-06-22 23:15 ` Fāng-ruì Sòng 2020-06-22 23:15 ` Fāng-ruì Sòng 2020-06-25 18:47 ` [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections Nick Desaulniers 1 sibling, 2 replies; 15+ messages in thread From: Fāng-ruì Sòng @ 2020-06-22 23:15 UTC (permalink / raw) To: Nick Desaulniers Cc: Arnd Bergmann, clang-built-linux, Kees Cook, # 3.4.x, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML On 2020-06-22, Nick Desaulniers wrote: >On Wed, Jun 17, 2020 at 2:27 PM Fāng-ruì Sòng <maskray@google.com> wrote: >> >> >> On 2020-06-17, Nick Desaulniers wrote: >> >ld.bfd's internal linker script considers .text.hot AND .text.hot.* to >> >be part of .text, as well as .text.unlikely and .text.unlikely.*. >> >> >ld.lld will produce .text.hot.*/.text.unlikely.* sections. >> >> Correction to this sentence. lld is not relevant here. >> >> -ffunction-sections combined with profile-guided optimization can >> produce .text.hot.* .text.unlikely.* sections. Newer clang may produce >> .text.hot. .text.unlikely. (without suffix, but with a trailing dot) >> when -fno-unique-section-names is specified, as an optimization to make >> .strtab smaller. > >Then why was the bug report reporting https://reviews.llvm.org/D79600 >as the result of a bisection, if LLD is not relevant? Was the >bisection wrong? https://reviews.llvm.org/D79600 is an LLVM codegen change, unrelated to LLD.. (As described in the patch, LLD's -z keep-text-section-prefix only recognizes ".text.exit.*", not ".text.exit") >The upstream report wasn't initially public, for no good reason. So I >didn't include it, but if we end up taking v1, this should have > >Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 Thanks for making it public. >The kernel doesn't use -fno-unique-section-names; is that another flag >that's added by CrOS' compiler wrapper? >https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/third_party/toolchain-utils/compiler_wrapper/config.go;l=110 >Looks like no. It doesn't use `-fno-unique-section-names` or >`-ffunction-sections`. -fno-unique-section-names is a very rare option. It is not supported by GCC (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95095 ). clang users use it very rarely, probably because not many people care about additional strings taken by section names ".text.hot.a" ".text.hot.b" ".text.hot.c" in the string table ".strtab" (clang since some point of 2018 uses .strtab instead of .shstrtab which enables more string sharing). > > > >> >> We've already seen that GCC can place main in .text.startup without >> -ffunction-sections. There may be other non -ffunction-sections cases >> for .text.hot.* or .text.unlikely.*. So it is definitely a good idea to >> be more specific even if we don't care about -ffunction-sections for >> now. >> >> >Make sure to group these together. Otherwise these orphan sections may >> >be placed outside of the the _stext/_etext boundaries. >> > >> >Cc: stable@vger.kernel.org >> >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee >> >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 >> >Link: https://reviews.llvm.org/D79600 >> >Reported-by: Jian Cai <jiancai@google.com> >> >Debugged-by: Luis Lozano <llozano@google.com> >> >Suggested-by: Fāng-ruì Sòng <maskray@google.com> >> >Tested-by: Luis Lozano <llozano@google.com> >> >Tested-by: Manoj Gupta <manojgupta@google.com> >> >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >> >--- >> > include/asm-generic/vmlinux.lds.h | 4 +++- >> > 1 file changed, 3 insertions(+), 1 deletion(-) >> > >> >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h >> >index d7c7c7f36c4a..fe5aaef169e3 100644 >> >--- a/include/asm-generic/vmlinux.lds.h >> >+++ b/include/asm-generic/vmlinux.lds.h >> >@@ -560,7 +560,9 @@ >> > */ >> > #define TEXT_TEXT \ >> > ALIGN_FUNCTION(); \ >> >- *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ >> >+ *(.text.hot .text.hot.*) \ >> >+ *(TEXT_MAIN .text.fixup) \ >> >+ *(.text.unlikely .text.unlikely.*) \ >> > NOINSTR_TEXT \ >> > *(.text..refcount) \ >> > *(.ref.text) \ >> >-- >> >2.27.0.290.gba653c62da-goog >> > > > > >-- >Thanks, >~Nick Desaulniers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too 2020-06-22 23:15 ` Fāng-ruì Sòng @ 2020-06-22 23:15 ` Fāng-ruì Sòng 2020-06-25 18:47 ` [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections Nick Desaulniers 1 sibling, 0 replies; 15+ messages in thread From: Fāng-ruì Sòng @ 2020-06-22 23:15 UTC (permalink / raw) To: Nick Desaulniers Cc: Arnd Bergmann, clang-built-linux, Kees Cook, # 3.4.x, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML On 2020-06-22, Nick Desaulniers wrote: >On Wed, Jun 17, 2020 at 2:27 PM Fāng-ruì Sòng <maskray@google.com> wrote: >> >> >> On 2020-06-17, Nick Desaulniers wrote: >> >ld.bfd's internal linker script considers .text.hot AND .text.hot.* to >> >be part of .text, as well as .text.unlikely and .text.unlikely.*. >> >> >ld.lld will produce .text.hot.*/.text.unlikely.* sections. >> >> Correction to this sentence. lld is not relevant here. >> >> -ffunction-sections combined with profile-guided optimization can >> produce .text.hot.* .text.unlikely.* sections. Newer clang may produce >> .text.hot. .text.unlikely. (without suffix, but with a trailing dot) >> when -fno-unique-section-names is specified, as an optimization to make >> .strtab smaller. > >Then why was the bug report reporting https://reviews.llvm.org/D79600 >as the result of a bisection, if LLD is not relevant? Was the >bisection wrong? https://reviews.llvm.org/D79600 is an LLVM codegen change, unrelated to LLD.. (As described in the patch, LLD's -z keep-text-section-prefix only recognizes ".text.exit.*", not ".text.exit") >The upstream report wasn't initially public, for no good reason. So I >didn't include it, but if we end up taking v1, this should have > >Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 Thanks for making it public. >The kernel doesn't use -fno-unique-section-names; is that another flag >that's added by CrOS' compiler wrapper? >https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/third_party/toolchain-utils/compiler_wrapper/config.go;l=110 >Looks like no. It doesn't use `-fno-unique-section-names` or >`-ffunction-sections`. -fno-unique-section-names is a very rare option. It is not supported by GCC (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95095 ). clang users use it very rarely, probably because not many people care about additional strings taken by section names ".text.hot.a" ".text.hot.b" ".text.hot.c" in the string table ".strtab" (clang since some point of 2018 uses .strtab instead of .shstrtab which enables more string sharing). > > > >> >> We've already seen that GCC can place main in .text.startup without >> -ffunction-sections. There may be other non -ffunction-sections cases >> for .text.hot.* or .text.unlikely.*. So it is definitely a good idea to >> be more specific even if we don't care about -ffunction-sections for >> now. >> >> >Make sure to group these together. Otherwise these orphan sections may >> >be placed outside of the the _stext/_etext boundaries. >> > >> >Cc: stable@vger.kernel.org >> >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee >> >Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 >> >Link: https://reviews.llvm.org/D79600 >> >Reported-by: Jian Cai <jiancai@google.com> >> >Debugged-by: Luis Lozano <llozano@google.com> >> >Suggested-by: Fāng-ruì Sòng <maskray@google.com> >> >Tested-by: Luis Lozano <llozano@google.com> >> >Tested-by: Manoj Gupta <manojgupta@google.com> >> >Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >> >--- >> > include/asm-generic/vmlinux.lds.h | 4 +++- >> > 1 file changed, 3 insertions(+), 1 deletion(-) >> > >> >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h >> >index d7c7c7f36c4a..fe5aaef169e3 100644 >> >--- a/include/asm-generic/vmlinux.lds.h >> >+++ b/include/asm-generic/vmlinux.lds.h >> >@@ -560,7 +560,9 @@ >> > */ >> > #define TEXT_TEXT \ >> > ALIGN_FUNCTION(); \ >> >- *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ >> >+ *(.text.hot .text.hot.*) \ >> >+ *(TEXT_MAIN .text.fixup) \ >> >+ *(.text.unlikely .text.unlikely.*) \ >> > NOINSTR_TEXT \ >> > *(.text..refcount) \ >> > *(.ref.text) \ >> >-- >> >2.27.0.290.gba653c62da-goog >> > > > > >-- >Thanks, >~Nick Desaulniers ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-06-22 23:15 ` Fāng-ruì Sòng 2020-06-22 23:15 ` Fāng-ruì Sòng @ 2020-06-25 18:47 ` Nick Desaulniers 2020-06-25 18:47 ` Nick Desaulniers 2020-07-01 21:54 ` Nick Desaulniers 1 sibling, 2 replies; 15+ messages in thread From: Nick Desaulniers @ 2020-06-25 18:47 UTC (permalink / raw) To: Arnd Bergmann Cc: Fāng-ruì Sòng, Nick Desaulniers, stable, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, linux-kernel, clang-built-linux Basically, consider .text.{hot|unlikely|unknown}.* part of .text, too. When compiling with profiling information (collected via PGO instrumentations or AutoFDO sampling), Clang will separate code into .text.hot, .text.unlikely, or .text.unknown sections based on profiling information. After D79600 (clang-11), these sections will have a trailing `.` suffix, ie. .text.hot., .text.unlikely., .text.unknown.. When using -ffunction-sections together with profiling infomation, either explicitly (FGKASLR) or implicitly (LTO), code may be placed in sections following the convention: .text.hot.<foo>, .text.unlikely.<bar>, .text.unknown.<baz> where <foo>, <bar>, and <baz> are functions. (This produces one section per function; we generally try to merge these all back via linker script so that we don't have 50k sections). For the above cases, we need to teach our linker scripts that such sections might exist and that we'd explicitly like them grouped together, otherwise we can wind up with code outside of the _stext/_etext boundaries that might not be mapped properly for some architectures, resulting in boot failures. If the linker script is not told about possible input sections, then where the section is placed as output is a heuristic-laiden mess that's non-portable between linkers (ie. BFD and LLD), and has resulted in many hard to debug bugs. Kees Cook is working on cleaning this up by adding --orphan-handling=warn linker flag used in ARCH=powerpc to additional architectures. In the case of linker scripts, borrowing from the Zen of Python: explicit is better than implicit. Also, ld.bfd's internal linker script considers .text.hot AND .text.hot.* to be part of .text, as well as .text.unlikely and .text.unlikely.*. I didn't see support for .text.unknown.*, and didn't see Clang producing such code in our kernel builds, but I see code in LLVM that can produce such section names if profiling information is missing. That may point to a larger issue with generating or collecting profiles, but I would much rather be safe and explicit than have to debug yet another issue related to orphan section placement. Cc: stable@vger.kernel.org Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 Link: https://reviews.llvm.org/D79600 Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 Reported-by: Jian Cai <jiancai@google.com> Debugged-by: Luis Lozano <llozano@google.com> Suggested-by: Fāng-ruì Sòng <maskray@google.com> Tested-by: Luis Lozano <llozano@google.com> Tested-by: Manoj Gupta <manojgupta@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes V1 -> V2: * Add .text.unknown.*. It's not strictly necessary for us yet, but I really worry that it could become a problem for us. Either way, I'm happy to drop for a V3, but I'm suggesting we not. * Beef up commit message. * Drop references to LLD; the LLVM change had nothing to do with LLD. I've realized I have a Pavlovian-response to changes from Fāng-ruì that I associate with LLD. I'm seeking professional help for my ailment. Forgive me. * Add link to now public CrOS bug. include/asm-generic/vmlinux.lds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index d7c7c7f36c4a..245c1af4c057 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -560,7 +560,10 @@ */ #define TEXT_TEXT \ ALIGN_FUNCTION(); \ - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ + *(.text.hot .text.hot.*) \ + *(TEXT_MAIN .text.fixup) \ + *(.text.unlikely .text.unlikely.*) \ + *(.text.unknown .text.unknown.*) \ NOINSTR_TEXT \ *(.text..refcount) \ *(.ref.text) \ -- 2.27.0.111.gc72c7da667-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-06-25 18:47 ` [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections Nick Desaulniers @ 2020-06-25 18:47 ` Nick Desaulniers 2020-07-01 21:54 ` Nick Desaulniers 1 sibling, 0 replies; 15+ messages in thread From: Nick Desaulniers @ 2020-06-25 18:47 UTC (permalink / raw) To: Arnd Bergmann Cc: Fāng-ruì Sòng, Nick Desaulniers, stable, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, linux-kernel, clang-built-linux Basically, consider .text.{hot|unlikely|unknown}.* part of .text, too. When compiling with profiling information (collected via PGO instrumentations or AutoFDO sampling), Clang will separate code into .text.hot, .text.unlikely, or .text.unknown sections based on profiling information. After D79600 (clang-11), these sections will have a trailing `.` suffix, ie. .text.hot., .text.unlikely., .text.unknown.. When using -ffunction-sections together with profiling infomation, either explicitly (FGKASLR) or implicitly (LTO), code may be placed in sections following the convention: .text.hot.<foo>, .text.unlikely.<bar>, .text.unknown.<baz> where <foo>, <bar>, and <baz> are functions. (This produces one section per function; we generally try to merge these all back via linker script so that we don't have 50k sections). For the above cases, we need to teach our linker scripts that such sections might exist and that we'd explicitly like them grouped together, otherwise we can wind up with code outside of the _stext/_etext boundaries that might not be mapped properly for some architectures, resulting in boot failures. If the linker script is not told about possible input sections, then where the section is placed as output is a heuristic-laiden mess that's non-portable between linkers (ie. BFD and LLD), and has resulted in many hard to debug bugs. Kees Cook is working on cleaning this up by adding --orphan-handling=warn linker flag used in ARCH=powerpc to additional architectures. In the case of linker scripts, borrowing from the Zen of Python: explicit is better than implicit. Also, ld.bfd's internal linker script considers .text.hot AND .text.hot.* to be part of .text, as well as .text.unlikely and .text.unlikely.*. I didn't see support for .text.unknown.*, and didn't see Clang producing such code in our kernel builds, but I see code in LLVM that can produce such section names if profiling information is missing. That may point to a larger issue with generating or collecting profiles, but I would much rather be safe and explicit than have to debug yet another issue related to orphan section placement. Cc: stable@vger.kernel.org Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 Link: https://reviews.llvm.org/D79600 Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 Reported-by: Jian Cai <jiancai@google.com> Debugged-by: Luis Lozano <llozano@google.com> Suggested-by: Fāng-ruì Sòng <maskray@google.com> Tested-by: Luis Lozano <llozano@google.com> Tested-by: Manoj Gupta <manojgupta@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes V1 -> V2: * Add .text.unknown.*. It's not strictly necessary for us yet, but I really worry that it could become a problem for us. Either way, I'm happy to drop for a V3, but I'm suggesting we not. * Beef up commit message. * Drop references to LLD; the LLVM change had nothing to do with LLD. I've realized I have a Pavlovian-response to changes from Fāng-ruì that I associate with LLD. I'm seeking professional help for my ailment. Forgive me. * Add link to now public CrOS bug. include/asm-generic/vmlinux.lds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index d7c7c7f36c4a..245c1af4c057 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -560,7 +560,10 @@ */ #define TEXT_TEXT \ ALIGN_FUNCTION(); \ - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ + *(.text.hot .text.hot.*) \ + *(TEXT_MAIN .text.fixup) \ + *(.text.unlikely .text.unlikely.*) \ + *(.text.unknown .text.unknown.*) \ NOINSTR_TEXT \ *(.text..refcount) \ *(.ref.text) \ -- 2.27.0.111.gc72c7da667-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-06-25 18:47 ` [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections Nick Desaulniers 2020-06-25 18:47 ` Nick Desaulniers @ 2020-07-01 21:54 ` Nick Desaulniers 2020-07-02 8:19 ` Arnd Bergmann 1 sibling, 1 reply; 15+ messages in thread From: Nick Desaulniers @ 2020-07-01 21:54 UTC (permalink / raw) To: Arnd Bergmann Cc: Fāng-ruì Sòng, # 3.4.x, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML, clang-built-linux Hi Arnd, I usually wait longer to bump threads for review, but we have a holiday in the US so we're off tomorrow and Friday. scripts/get_maintainer.pl recommend you for this patch. Would you take a look at it for us, please? On Thu, Jun 25, 2020 at 11:48 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > Basically, consider .text.{hot|unlikely|unknown}.* part of .text, too. > > When compiling with profiling information (collected via PGO > instrumentations or AutoFDO sampling), Clang will separate code into > .text.hot, .text.unlikely, or .text.unknown sections based on profiling > information. After D79600 (clang-11), these sections will have a > trailing `.` suffix, ie. .text.hot., .text.unlikely., .text.unknown.. > > When using -ffunction-sections together with profiling infomation, > either explicitly (FGKASLR) or implicitly (LTO), code may be placed in > sections following the convention: > .text.hot.<foo>, .text.unlikely.<bar>, .text.unknown.<baz> > where <foo>, <bar>, and <baz> are functions. (This produces one section > per function; we generally try to merge these all back via linker script > so that we don't have 50k sections). > > For the above cases, we need to teach our linker scripts that such > sections might exist and that we'd explicitly like them grouped > together, otherwise we can wind up with code outside of the > _stext/_etext boundaries that might not be mapped properly for some > architectures, resulting in boot failures. > > If the linker script is not told about possible input sections, then > where the section is placed as output is a heuristic-laiden mess that's > non-portable between linkers (ie. BFD and LLD), and has resulted in many > hard to debug bugs. Kees Cook is working on cleaning this up by adding > --orphan-handling=warn linker flag used in ARCH=powerpc to additional > architectures. In the case of linker scripts, borrowing from the Zen of > Python: explicit is better than implicit. > > Also, ld.bfd's internal linker script considers .text.hot AND > .text.hot.* to be part of .text, as well as .text.unlikely and > .text.unlikely.*. I didn't see support for .text.unknown.*, and didn't > see Clang producing such code in our kernel builds, but I see code in > LLVM that can produce such section names if profiling information is > missing. That may point to a larger issue with generating or collecting > profiles, but I would much rather be safe and explicit than have to > debug yet another issue related to orphan section placement. > > Cc: stable@vger.kernel.org > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 > Link: https://reviews.llvm.org/D79600 > Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 > Reported-by: Jian Cai <jiancai@google.com> > Debugged-by: Luis Lozano <llozano@google.com> > Suggested-by: Fāng-ruì Sòng <maskray@google.com> > Tested-by: Luis Lozano <llozano@google.com> > Tested-by: Manoj Gupta <manojgupta@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes V1 -> V2: > * Add .text.unknown.*. It's not strictly necessary for us yet, but I > really worry that it could become a problem for us. Either way, I'm > happy to drop for a V3, but I'm suggesting we not. > * Beef up commit message. > * Drop references to LLD; the LLVM change had nothing to do with LLD. > I've realized I have a Pavlovian-response to changes from Fāng-ruì > that I associate with LLD. I'm seeking professional help for my > ailment. Forgive me. > * Add link to now public CrOS bug. > > include/asm-generic/vmlinux.lds.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index d7c7c7f36c4a..245c1af4c057 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -560,7 +560,10 @@ > */ > #define TEXT_TEXT \ > ALIGN_FUNCTION(); \ > - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ > + *(.text.hot .text.hot.*) \ > + *(TEXT_MAIN .text.fixup) \ > + *(.text.unlikely .text.unlikely.*) \ > + *(.text.unknown .text.unknown.*) \ > NOINSTR_TEXT \ > *(.text..refcount) \ > *(.ref.text) \ > -- > 2.27.0.111.gc72c7da667-goog > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-07-01 21:54 ` Nick Desaulniers @ 2020-07-02 8:19 ` Arnd Bergmann 2020-07-02 15:57 ` Kees Cook 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2020-07-02 8:19 UTC (permalink / raw) To: Nick Desaulniers Cc: Fāng-ruì Sòng, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML, clang-built-linux, Kees Cook On Wed, Jul 1, 2020 at 11:54 PM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > Hi Arnd, > I usually wait longer to bump threads for review, but we have a > holiday in the US so we're off tomorrow and Friday. > scripts/get_maintainer.pl recommend you for this patch. Would you > take a look at it for us, please? Hi Nick While I'm listed as the maintainer for include/asm-generic, linker scripts are really not my expertise and I have no way of knowing whether the change is good or not. Your description looks very reasonable of course and I have no problem with having someone else pick it up. You mentioned that Kees is already looking at some related work and he's already done more changes to this file than anyone else. If he can provide an Ack for this patch, you can add mine as well to send it to akpm, or I can pick it up in the asm-generic tree. Arnd > On Thu, Jun 25, 2020 at 11:48 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > Basically, consider .text.{hot|unlikely|unknown}.* part of .text, too. > > > > When compiling with profiling information (collected via PGO > > instrumentations or AutoFDO sampling), Clang will separate code into > > .text.hot, .text.unlikely, or .text.unknown sections based on profiling > > information. After D79600 (clang-11), these sections will have a > > trailing `.` suffix, ie. .text.hot., .text.unlikely., .text.unknown.. > > > > When using -ffunction-sections together with profiling infomation, > > either explicitly (FGKASLR) or implicitly (LTO), code may be placed in > > sections following the convention: > > .text.hot.<foo>, .text.unlikely.<bar>, .text.unknown.<baz> > > where <foo>, <bar>, and <baz> are functions. (This produces one section > > per function; we generally try to merge these all back via linker script > > so that we don't have 50k sections). > > > > For the above cases, we need to teach our linker scripts that such > > sections might exist and that we'd explicitly like them grouped > > together, otherwise we can wind up with code outside of the > > _stext/_etext boundaries that might not be mapped properly for some > > architectures, resulting in boot failures. > > > > If the linker script is not told about possible input sections, then > > where the section is placed as output is a heuristic-laiden mess that's > > non-portable between linkers (ie. BFD and LLD), and has resulted in many > > hard to debug bugs. Kees Cook is working on cleaning this up by adding > > --orphan-handling=warn linker flag used in ARCH=powerpc to additional > > architectures. In the case of linker scripts, borrowing from the Zen of > > Python: explicit is better than implicit. > > > > Also, ld.bfd's internal linker script considers .text.hot AND > > .text.hot.* to be part of .text, as well as .text.unlikely and > > .text.unlikely.*. I didn't see support for .text.unknown.*, and didn't > > see Clang producing such code in our kernel builds, but I see code in > > LLVM that can produce such section names if profiling information is > > missing. That may point to a larger issue with generating or collecting > > profiles, but I would much rather be safe and explicit than have to > > debug yet another issue related to orphan section placement. > > > > Cc: stable@vger.kernel.org > > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee > > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 > > Link: https://reviews.llvm.org/D79600 > > Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 > > Reported-by: Jian Cai <jiancai@google.com> > > Debugged-by: Luis Lozano <llozano@google.com> > > Suggested-by: Fāng-ruì Sòng <maskray@google.com> > > Tested-by: Luis Lozano <llozano@google.com> > > Tested-by: Manoj Gupta <manojgupta@google.com> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > Changes V1 -> V2: > > * Add .text.unknown.*. It's not strictly necessary for us yet, but I > > really worry that it could become a problem for us. Either way, I'm > > happy to drop for a V3, but I'm suggesting we not. > > * Beef up commit message. > > * Drop references to LLD; the LLVM change had nothing to do with LLD. > > I've realized I have a Pavlovian-response to changes from Fāng-ruì > > that I associate with LLD. I'm seeking professional help for my > > ailment. Forgive me. > > * Add link to now public CrOS bug. > > > > include/asm-generic/vmlinux.lds.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > > index d7c7c7f36c4a..245c1af4c057 100644 > > --- a/include/asm-generic/vmlinux.lds.h > > +++ b/include/asm-generic/vmlinux.lds.h > > @@ -560,7 +560,10 @@ > > */ > > #define TEXT_TEXT \ > > ALIGN_FUNCTION(); \ > > - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ > > + *(.text.hot .text.hot.*) \ > > + *(TEXT_MAIN .text.fixup) \ > > + *(.text.unlikely .text.unlikely.*) \ > > + *(.text.unknown .text.unknown.*) \ > > NOINSTR_TEXT \ > > *(.text..refcount) \ > > *(.ref.text) \ > > -- > > 2.27.0.111.gc72c7da667-goog > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-07-02 8:19 ` Arnd Bergmann @ 2020-07-02 15:57 ` Kees Cook 2020-07-08 23:13 ` Nick Desaulniers 0 siblings, 1 reply; 15+ messages in thread From: Kees Cook @ 2020-07-02 15:57 UTC (permalink / raw) To: Arnd Bergmann Cc: Nick Desaulniers, Fāng-ruì Sòng, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML, clang-built-linux On Thu, Jul 02, 2020 at 10:19:40AM +0200, Arnd Bergmann wrote: > On Wed, Jul 1, 2020 at 11:54 PM 'Nick Desaulniers' via Clang Built > Linux <clang-built-linux@googlegroups.com> wrote: > > > > Hi Arnd, > > I usually wait longer to bump threads for review, but we have a > > holiday in the US so we're off tomorrow and Friday. > > scripts/get_maintainer.pl recommend you for this patch. Would you > > take a look at it for us, please? > > Hi Nick > > While I'm listed as the maintainer for include/asm-generic, linker scripts > are really not my expertise and I have no way of knowing whether the > change is good or not. > > Your description looks very reasonable of course and I have no problem > with having someone else pick it up. You mentioned that Kees is already > looking at some related work and he's already done more changes to > this file than anyone else. If he can provide an Ack for this patch, > you can add mine as well to send it to akpm, or I can pick it up in the > asm-generic tree. This looks good to me. Do you want me to carry it as part of the orphan series? (It doesn't look like it'll collide, so that's not needed, but I can if that makes things easier.) Acked-by: Kees Cook <keescook@chromium.org> -Kees > > Arnd > > > On Thu, Jun 25, 2020 at 11:48 AM Nick Desaulniers > > <ndesaulniers@google.com> wrote: > > > > > > Basically, consider .text.{hot|unlikely|unknown}.* part of .text, too. > > > > > > When compiling with profiling information (collected via PGO > > > instrumentations or AutoFDO sampling), Clang will separate code into > > > .text.hot, .text.unlikely, or .text.unknown sections based on profiling > > > information. After D79600 (clang-11), these sections will have a > > > trailing `.` suffix, ie. .text.hot., .text.unlikely., .text.unknown.. > > > > > > When using -ffunction-sections together with profiling infomation, > > > either explicitly (FGKASLR) or implicitly (LTO), code may be placed in > > > sections following the convention: > > > .text.hot.<foo>, .text.unlikely.<bar>, .text.unknown.<baz> > > > where <foo>, <bar>, and <baz> are functions. (This produces one section > > > per function; we generally try to merge these all back via linker script > > > so that we don't have 50k sections). > > > > > > For the above cases, we need to teach our linker scripts that such > > > sections might exist and that we'd explicitly like them grouped > > > together, otherwise we can wind up with code outside of the > > > _stext/_etext boundaries that might not be mapped properly for some > > > architectures, resulting in boot failures. > > > > > > If the linker script is not told about possible input sections, then > > > where the section is placed as output is a heuristic-laiden mess that's > > > non-portable between linkers (ie. BFD and LLD), and has resulted in many > > > hard to debug bugs. Kees Cook is working on cleaning this up by adding > > > --orphan-handling=warn linker flag used in ARCH=powerpc to additional > > > architectures. In the case of linker scripts, borrowing from the Zen of > > > Python: explicit is better than implicit. > > > > > > Also, ld.bfd's internal linker script considers .text.hot AND > > > .text.hot.* to be part of .text, as well as .text.unlikely and > > > .text.unlikely.*. I didn't see support for .text.unknown.*, and didn't > > > see Clang producing such code in our kernel builds, but I see code in > > > LLVM that can produce such section names if profiling information is > > > missing. That may point to a larger issue with generating or collecting > > > profiles, but I would much rather be safe and explicit than have to > > > debug yet another issue related to orphan section placement. > > > > > > Cc: stable@vger.kernel.org > > > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=add44f8d5c5c05e08b11e033127a744d61c26aee > > > Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=1de778ed23ce7492c523d5850c6c6dbb34152655 > > > Link: https://reviews.llvm.org/D79600 > > > Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 > > > Reported-by: Jian Cai <jiancai@google.com> > > > Debugged-by: Luis Lozano <llozano@google.com> > > > Suggested-by: Fāng-ruì Sòng <maskray@google.com> > > > Tested-by: Luis Lozano <llozano@google.com> > > > Tested-by: Manoj Gupta <manojgupta@google.com> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > > Changes V1 -> V2: > > > * Add .text.unknown.*. It's not strictly necessary for us yet, but I > > > really worry that it could become a problem for us. Either way, I'm > > > happy to drop for a V3, but I'm suggesting we not. > > > * Beef up commit message. > > > * Drop references to LLD; the LLVM change had nothing to do with LLD. > > > I've realized I have a Pavlovian-response to changes from Fāng-ruì > > > that I associate with LLD. I'm seeking professional help for my > > > ailment. Forgive me. > > > * Add link to now public CrOS bug. > > > > > > include/asm-generic/vmlinux.lds.h | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > > > index d7c7c7f36c4a..245c1af4c057 100644 > > > --- a/include/asm-generic/vmlinux.lds.h > > > +++ b/include/asm-generic/vmlinux.lds.h > > > @@ -560,7 +560,10 @@ > > > */ > > > #define TEXT_TEXT \ > > > ALIGN_FUNCTION(); \ > > > - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \ > > > + *(.text.hot .text.hot.*) \ > > > + *(TEXT_MAIN .text.fixup) \ > > > + *(.text.unlikely .text.unlikely.*) \ > > > + *(.text.unknown .text.unknown.*) \ > > > NOINSTR_TEXT \ > > > *(.text..refcount) \ > > > *(.ref.text) \ > > > -- > > > 2.27.0.111.gc72c7da667-goog > > > -- Kees Cook ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-07-02 15:57 ` Kees Cook @ 2020-07-08 23:13 ` Nick Desaulniers 2020-07-09 5:43 ` Kees Cook 0 siblings, 1 reply; 15+ messages in thread From: Nick Desaulniers @ 2020-07-08 23:13 UTC (permalink / raw) To: Kees Cook Cc: Arnd Bergmann, Fāng-ruì Sòng, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML, clang-built-linux On Thu, Jul 2, 2020 at 8:57 AM Kees Cook <keescook@chromium.org> wrote: > > This looks good to me. Do you want me to carry it as part of the orphan > series? (It doesn't look like it'll collide, so that's not needed, but I > can if that makes things easier.) > > Acked-by: Kees Cook <keescook@chromium.org> If you would be so kind, I'd owe you yet another beer! -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections 2020-07-08 23:13 ` Nick Desaulniers @ 2020-07-09 5:43 ` Kees Cook 0 siblings, 0 replies; 15+ messages in thread From: Kees Cook @ 2020-07-09 5:43 UTC (permalink / raw) To: Nick Desaulniers Cc: Arnd Bergmann, Fāng-ruì Sòng, Jian Cai, Luis Lozano, Manoj Gupta, linux-arch, LKML, clang-built-linux On Wed, Jul 08, 2020 at 04:13:54PM -0700, Nick Desaulniers wrote: > On Thu, Jul 2, 2020 at 8:57 AM Kees Cook <keescook@chromium.org> wrote: > > > > This looks good to me. Do you want me to carry it as part of the orphan > > series? (It doesn't look like it'll collide, so that's not needed, but I > > can if that makes things easier.) > > > > Acked-by: Kees Cook <keescook@chromium.org> > > If you would be so kind, I'd owe you yet another beer! Yup! It's on my list; I've been clearing other stuff so I can do another revision. (I want to move some things out of discard and into 0-size asserts, and possibly collect Arvind's runtime relocations series too (since it seems basically done but no x86 maintainers have snagged it). -- Kees Cook ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-07-09 5:43 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-17 21:06 [PATCH] vmlinux.lds: consider .text.{hot|unlikely}.* part of .text too Nick Desaulniers 2020-06-17 21:06 ` Nick Desaulniers 2020-06-17 21:27 ` Fāng-ruì Sòng 2020-06-17 21:27 ` Fāng-ruì Sòng 2020-06-22 23:04 ` Nick Desaulniers 2020-06-22 23:04 ` Nick Desaulniers 2020-06-22 23:15 ` Fāng-ruì Sòng 2020-06-22 23:15 ` Fāng-ruì Sòng 2020-06-25 18:47 ` [PATCH v2] vmlinux.lds: add PGO and AutoFDO input sections Nick Desaulniers 2020-06-25 18:47 ` Nick Desaulniers 2020-07-01 21:54 ` Nick Desaulniers 2020-07-02 8:19 ` Arnd Bergmann 2020-07-02 15:57 ` Kees Cook 2020-07-08 23:13 ` Nick Desaulniers 2020-07-09 5:43 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).