diff mbox series

[v7,3/7] Adjust symbol ordering in text output section

Message ID 20241102175115.1769468-4-xur@google.com
State New
Headers show
Series Add AutoFDO and Propeller support for Clang build | expand

Commit Message

Rong Xu Nov. 2, 2024, 5:51 p.m. UTC 
When the -ffunction-sections compiler option is enabled, each function
is placed in a separate section named .text.function_name rather than
putting all functions in a single .text section.

However, using -function-sections can cause problems with the
linker script. The comments included in include/asm-generic/vmlinux.lds.h
note these issues.:
  “TEXT_MAIN here will match .text.fixup and .text.unlikely if dead
   code elimination is enabled, so these sections should be converted
   to use ".." first.”

It is unclear whether there is a straightforward method for converting
a suffix to "..".

This patch modifies the order of subsections within the text output
section. Specifically, it changes current order:
  .text.hot, .text, .text_unlikely, .text.unknown, .text.asan
to the new order:
  .text.asan, .text.unknown, .text_unlikely, .text.hot, .text

Here is the rationale behind the new layout:

The majority of the code resides in three sections: .text.hot, .text,
and .text.unlikely, with .text.unknown containing a negligible amount.
.text.asan is only generated in ASAN builds.

The primary goal is to group code segments based on their execution
frequency (hotness).

First, we want to place .text.hot adjacent to .text. Since we cannot put
.text.hot after .text (Due to constraints with -ffunction-sections,
placing .text.hot after .text is problematic), we need to put
.text.hot before .text.

Then it comes to .text.unlikely, we cannot put it after .text (same
-ffunction-sections issue) . Therefore, we position .text.unlikely
before .text.hot.

.text.unknown and .tex.asan follow the same logic.

This revised ordering effectively reverses the original arrangement (for
.text.unlikely, .text.unknown, and .tex.asan), maintaining a similar level
of affinity between sections.

It also places .text.hot section at the beginning of a page to better
utilize the TLB entry.

Note that the limitation arises because the linker script employs glob
patterns instead of regular expressions for string matching. While there
is a method to maintain the current order using complex patterns, this
significantly complicates the pattern and increases the likelihood of
errors.

This patch also changes vmlinux.lds.S for the sparc64 architecture to
accommodate specific symbol placement requirements.

Co-developed-by: Han Shen <shenhan@google.com>
Signed-off-by: Han Shen <shenhan@google.com>
Signed-off-by: Rong Xu <xur@google.com>
Suggested-by: Sriraman Tallam <tmsriram@google.com>
Suggested-by: Krzysztof Pszeniczny <kpszeniczny@google.com>
Tested-by: Yonghong Song <yonghong.song@linux.dev>
Tested-by: Yabin Cui <yabinc@google.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Kees Cook <kees@kernel.org>
---
 arch/sparc/kernel/vmlinux.lds.S   |  5 +++++
 include/asm-generic/vmlinux.lds.h | 19 ++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

Comments

Guenter Roeck Dec. 1, 2024, 2:31 p.m. UTC | #1 
Hi,

On Sat, Nov 02, 2024 at 10:51:10AM -0700, Rong Xu wrote:
> When the -ffunction-sections compiler option is enabled, each function
> is placed in a separate section named .text.function_name rather than
> putting all functions in a single .text section.
> 
...
> 
> Co-developed-by: Han Shen <shenhan@google.com>
> Signed-off-by: Han Shen <shenhan@google.com>
> Signed-off-by: Rong Xu <xur@google.com>
> Suggested-by: Sriraman Tallam <tmsriram@google.com>
> Suggested-by: Krzysztof Pszeniczny <kpszeniczny@google.com>
> Tested-by: Yonghong Song <yonghong.song@linux.dev>
> Tested-by: Yabin Cui <yabinc@google.com>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
> Reviewed-by: Kees Cook <kees@kernel.org>

With this patch in the tree, the openrisck qemu emulation using
or1ksim_defconfig fails to boot. There is no log output, even with
earlycon enabled.

Bisect log attached.

Guenter

---
# bad: [bcc8eda6d34934d80b96adb8dc4ff5dfc632a53a] Merge tag 'turbostat-2024.11.30' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux
# good: [2ba9f676d0a2e408aef14d679984c26373bf37b7] Merge tag 'drm-next-2024-11-29' of https://gitlab.freedesktop.org/drm/kernel
git bisect start 'HEAD' '2ba9f676d0a2'
# good: [831c1926ee728c3e747255f7c0f434762e8e863d] Merge tag 'uml-for-linus-6.13-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/uml/linux
git bisect good 831c1926ee728c3e747255f7c0f434762e8e863d
# bad: [6a34dfa15d6edf7e78b8118d862d2db0889cf669] Merge tag 'kbuild-v6.13' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
git bisect bad 6a34dfa15d6edf7e78b8118d862d2db0889cf669
# bad: [e397a603e49cc7c7c113fad9f55a09637f290c34] kbuild: switch from lz4c to lz4 for compression
git bisect bad e397a603e49cc7c7c113fad9f55a09637f290c34
# good: [d6a91e28d11902e6cd5715633ed6f9b6df75de32] kconfig: qconf: remove unnecessary mode check in ConfigItem::updateMenu()
git bisect good d6a91e28d11902e6cd5715633ed6f9b6df75de32
# bad: [0afd73c5f5c606b0f8f8ff036e4f5d6c4b788d02] kbuild: replace two $(abs_objtree) with $(CURDIR) in top Makefile
git bisect bad 0afd73c5f5c606b0f8f8ff036e4f5d6c4b788d02
# bad: [db0b2991ae1aac5ca985ec6fd8ff9bd9b2126c9b] vmlinux.lds.h: Add markers for text_unlikely and text_hot sections
git bisect bad db0b2991ae1aac5ca985ec6fd8ff9bd9b2126c9b
# good: [315ad8780a129e82e2c5c65ee6e970d91a577acb] kbuild: Add AutoFDO support for Clang build
git bisect good 315ad8780a129e82e2c5c65ee6e970d91a577acb
# good: [52892ed6b03a14b961c1df783ed05763758abc73] MIPS: Place __kernel_entry at the beginning of text section
git bisect good 52892ed6b03a14b961c1df783ed05763758abc73
# bad: [0043ecea2399ffc8bfd99ed9dbbe766e7c79293c] vmlinux.lds.h: Adjust symbol ordering in text output section
git bisect bad 0043ecea2399ffc8bfd99ed9dbbe766e7c79293c
# first bad commit: [0043ecea2399ffc8bfd99ed9dbbe766e7c79293c] vmlinux.lds.h: Adjust symbol ordering in text output section
Masahiro Yamada Dec. 2, 2024, 6:39 a.m. UTC | #2 
On Sun, Dec 1, 2024 at 11:31 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Hi,
>
> On Sat, Nov 02, 2024 at 10:51:10AM -0700, Rong Xu wrote:
> > When the -ffunction-sections compiler option is enabled, each function
> > is placed in a separate section named .text.function_name rather than
> > putting all functions in a single .text section.
> >
> ...
> >
> > Co-developed-by: Han Shen <shenhan@google.com>
> > Signed-off-by: Han Shen <shenhan@google.com>
> > Signed-off-by: Rong Xu <xur@google.com>
> > Suggested-by: Sriraman Tallam <tmsriram@google.com>
> > Suggested-by: Krzysztof Pszeniczny <kpszeniczny@google.com>
> > Tested-by: Yonghong Song <yonghong.song@linux.dev>
> > Tested-by: Yabin Cui <yabinc@google.com>
> > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > Reviewed-by: Kees Cook <kees@kernel.org>
>
> With this patch in the tree, the openrisck qemu emulation using
> or1ksim_defconfig fails to boot. There is no log output, even with
> earlycon enabled.

Thanks for the report.
I posted a fix.
https://lore.kernel.org/all/20241202062909.2194341-1-masahiroy@kernel.org/T/#u
diff mbox series

Patch

diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
index d317a843f7ea9..f1b86eb303404 100644
--- a/arch/sparc/kernel/vmlinux.lds.S
+++ b/arch/sparc/kernel/vmlinux.lds.S
@@ -48,6 +48,11 @@  SECTIONS
 	{
 		_text = .;
 		HEAD_TEXT
+	        ALIGN_FUNCTION();
+#ifdef CONFIG_SPARC64
+	        /* Match text section symbols in head_64.S first */
+	        *head_64.o(.text)
+#endif
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index eeadbaeccf88b..fd901951549c0 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -553,19 +553,24 @@ 
  * .text section. Map to function alignment to avoid address changes
  * during second ld run in second ld pass when generating System.map
  *
- * TEXT_MAIN here will match .text.fixup and .text.unlikely if dead
- * code elimination is enabled, so these sections should be converted
- * to use ".." first.
+ * TEXT_MAIN here will match symbols with a fixed pattern (for example,
+ * .text.hot or .text.unlikely) if dead code elimination or
+ * function-section is enabled. Match these symbols first before
+ * TEXT_MAIN to ensure they are grouped together.
+ *
+ * Also placing .text.hot section at the beginning of a page, this
+ * would help the TLB performance.
  */
 #define TEXT_TEXT							\
 		ALIGN_FUNCTION();					\
+		*(.text.asan.* .text.tsan.*)				\
+		*(.text.unknown .text.unknown.*)			\
+		*(.text.unlikely .text.unlikely.*)			\
+		. = ALIGN(PAGE_SIZE);					\
 		*(.text.hot .text.hot.*)				\
 		*(TEXT_MAIN .text.fixup)				\
-		*(.text.unlikely .text.unlikely.*)			\
-		*(.text.unknown .text.unknown.*)			\
 		NOINSTR_TEXT						\
-		*(.ref.text)						\
-		*(.text.asan.* .text.tsan.*)
+		*(.ref.text)
 
 
 /* sched.text is aling to function alignment to secure we have same