From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 12:12:30
Hello,
I have 3 different things in this patchset. All arch specific, but all
involve kbuild changes, so I'd like to discuss them with kbuild
maintainers. The goal has been to improve long standing linking
difficulties with the powerpc kernel.
* First, building kernel using thin archives rather than incremental
linking. This seems quite clean and is per-arch, so I hope it should
not be too controversial.
* Second, building kernel using -ffunction-sections -fdata-sections,
--gc-sections. Yes, I'm spinning the wheel again. It was motivated
by tiny codesize regression in the first patch, but the results seem
too good to ignore.
* Third, allowing architecture to run a tool over module after it has
been linked. Powerpc wants to use it in order to relocate "alternate
code" instructions that get don't get linked at their runtime
address. No idea if this is the right approach wrt kbuild, but it
seems to work.
I have included the powerpc code for the first two as a reference. The
third is much bigger and mostly uninteresting for this cc list, but it
can be found here:
https://patchwork.ozlabs.org/patch/651006/
Comments appreciated.
Thanks,
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 12:12:44
From: Stephen Rothwell <redacted>
ld -r is an incremental link used to create built-in.o files in build
subdirectories. It produces relocatable object files containing all
its input files, and these are are then pulled together and relocated
in the final link. Aside from the bloat, this constrains the final
link relocations, which has bitten large powerpc builds with
unresolvable relocations in the final link.
Alan Modra has recommended the kernel use thin archives for linking.
This is an alternative and means that the linker has more information
available to it when it links the kernel.
This patch enables a config option architectures can select, which
causes all built-in.o files to be built as thin archives. built-in.o
files in subdirectories do not get symbol table or index attached,
which improves speed and size. The final link pass creates a
built-in.o archive in the root output directory which includes the
symbol table and index. The linker then uses takes this file to link.
The --whole-archive linker option is required, because the linker now
has visibility to every individual object file, and it will otherwise
just completely avoid including those without external references
(consider a file with EXPORT_SYMBOL or initcall or hardware exceptions
as its only entry points). The traditional built works "by luck" as
built-in.o files are large enough that they're going to get external
references. However this optimisation is unpredictable for the kernel
(due to above external references), ineffective at culling unused, and
costly because the .o files have to be searched for references.
Superior alternatives for link-time culling should be used instead.
Build characteristics for inclink vs thinarc, on a small powerpc64le
pseries VM with a modest .config:
inclink thinarc
sizes
vmlinux 15 618 680 15 625 028
sum of all built-in.o 56 091 808 1 054 334
sum excluding root built-in.o 151 430
find -name built-in.o | xargs rm ; time make vmlinux
real 22.772s 21.143s
user 13.280s 13.430s
sys 4.310s 2.750s
- Final kernel pulled in only about 6K more, which shows how
ineffective the object file culling is.
- Build performance looks improved due to less pagecache activity.
On IO constrained systems it could be a bigger win.
- Build size saving is significant.
Side note, the toochain understands archives, so there's some tricks,
$ ar t built-in.o # list all files you linked with
$ size built-in.o # and their sizes
$ objdump -d built-in.o # disassembly (unrelocated) with filenames
Implementation by sfr, minor tweaks by npiggin.
Cc: linux-kbuild@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Segher Boessenkool <redacted>
Cc: Alan Modra <redacted>
Signed-off-by: Stephen Rothwell <redacted>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/Kconfig | 6 +++++
scripts/Makefile.build | 23 +++++++++++++---
scripts/link-vmlinux.sh | 71 +++++++++++++++++++++++++++++++++++++++++--------
3 files changed, 85 insertions(+), 15 deletions(-)
@@ -358,12 +358,22 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;# Rule to compile a set of .o files into one .o file#ifdef builtin-target-quiet_cmd_link_o_target=LD$@++ifdef CONFIG_THIN_ARCHIVES+cmd_make_builtin=rm-f$@;$(AR)rcST$(KBUILD_ARFLAGS)+cmd_make_empty_builtin=rm-f$@;$(AR)rcST$(KBUILD_ARFLAGS)+quiet_cmd_link_o_target=AR$@+else+cmd_make_builtin=$(LD)$(ld_flags)-r-o+cmd_make_empty_builtin=rm-f$@;$(AR)rcs$(KBUILD_ARFLAGS)+quiet_cmd_link_o_target=LD$@+endif+# If the list of objects to link is empty, just create an empty built-in.ocmd_link_o_target=$(if$(strip$(obj-y)),\-$(LD)$(ld_flags)-r-o$@$(filter$(obj-y),$^)\+$(cmd_make_builtin)$@$(filter$(obj-y),$^)\$(cmd_secanalysis),\-rm-f$@;$(AR)rcs$(KBUILD_ARFLAGS)$@)+$(cmd_make_empty_builtin)$@)$(builtin-target):$(obj-y)FORCE$(callif_changed,link_o_target)
@@ -37,12 +37,40 @@ info()fi}+# Thin archive build here makes a final archive with+# symbol table and indexes from vmlinux objects, which can be+# used as input to linker.+#+# Traditional incremental style of link does not require this step+#+# built-in.o output file+#+archive_builtin()+{+if[-n"${CONFIG_THIN_ARCHIVES}"];then+infoARbuilt-in.o+rm-fbuilt-in.o;+${AR}rcsT${KBUILD_ARFLAGS}built-in.o\+${KBUILD_VMLINUX_INIT}\+${KBUILD_VMLINUX_MAIN}+fi+}+# Link of vmlinux.o used for section mismatch analysis# ${1} output file modpost_link(){-${LD}${LDFLAGS}-r-o${1}${KBUILD_VMLINUX_INIT}\---start-group${KBUILD_VMLINUX_MAIN}--end-group+localobjects++if[-n"${CONFIG_THIN_ARCHIVES}"];then+objects="--whole-archive built-in.o"+else+objects="${KBUILD_VMLINUX_INIT} \+--start-group\+${KBUILD_VMLINUX_MAIN}\+--end-group"+fi+${LD}${LDFLAGS}-r-o${1}${objects}}# Link of vmlinux
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 12:12:51
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections. It requires some work (documented) to ensure all
unreferenced entrypoints are live, and requires toolchain and
build verification, so it is made a per-arch option for now.
On a random powerpc64le build, this yelds a significant size saving,
it boots and runs fine, but there is a lot I haven't tested as yet,
so these savings may be reduced if there are bugs in the link.
text data bss dec filename
11169741 1180744 1923176 14273661 vmlinux
10445269 1004127 1919707 13369103 vmlinux.dce
~700K text, ~170K data, 6% removed from kernel image size.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Makefile | 10 ++++++++
arch/Kconfig | 13 ++++++++++
include/asm-generic/vmlinux.lds.h | 52 ++++++++++++++++++++++-----------------
include/linux/compiler.h | 18 ++++++++++++++
include/linux/export.h | 30 +++++++++++-----------
include/linux/init.h | 38 ++++++++++------------------
init/Makefile | 2 ++
7 files changed, 100 insertions(+), 63 deletions(-)
@@ -42,27 +43,26 @@ extern struct module __this_module;#ifdef CONFIG_MODVERSIONS/* Mark the CRC weak since genksyms apparently decides not to*generateachecksumsforsomesymbols*/-#define __CRC_SYMBOL(sym, sec) \-extern__visiblevoid*__crc_##sym__attribute__((weak));\-staticconstunsignedlong__kcrctab_##sym\-__used\-__attribute__((section("___kcrctab"sec"+"#sym),unused))\+#define __CRC_SYMBOL(sym, sec) \+extern__visiblevoid*__crc_##sym__attribute__((weak));\+staticconstunsignedlong__kcrctab_##sym\+__used\+__attribute__((section("___kcrctab"sec"+"#sym),used))\=(unsignedlong)&__crc_##sym;#else#define __CRC_SYMBOL(sym, sec)#endif/* For every exported symbol, place a struct in the __ksymtab section */-#define ___EXPORT_SYMBOL(sym, sec) \-externtypeof(sym)sym;\-__CRC_SYMBOL(sym,sec)\-staticconstchar__kstrtab_##sym[]\-__attribute__((section("__ksymtab_strings"),aligned(1)))\-=VMLINUX_SYMBOL_STR(sym);\-externconststructkernel_symbol__ksymtab_##sym;\-__visibleconststructkernel_symbol__ksymtab_##sym\-__used\-__attribute__((section("___ksymtab"sec"+"#sym),unused))\+#define ___EXPORT_SYMBOL(sym, sec) \+externtypeof(sym)sym;\+__CRC_SYMBOL(sym,sec)\+staticconstchar__kstrtab_##sym[]\+__attribute__((section("__ksymtab_strings"),aligned(1)))\+=VMLINUX_SYMBOL_STR(sym);\+staticconststructkernel_symbol__ksymtab_##sym\+__used\+__attribute__((section("___ksymtab"sec"+"#sym),used))\={(unsignedlong)&sym,__kstrtab_##sym}#if defined(__KSYM_DEPS__)
@@ -156,24 +156,8 @@ extern bool initcall_debug;#ifndef __ASSEMBLY__-#ifdef CONFIG_LTO-/* Work around a LTO gcc problem: when there is no reference to a variable-*inamoduleitwillbemovedtotheendoftheprogram.Thiscauses-*reorderingofinitcallswhichthekerneldoesnotlike.-*Addadummyreferencefunctiontoavoidthis.Thefunctionis-*deletedbythelinker.-*/-#define LTO_REFERENCE_INITCALL(x) \-;/* yes this is needed */\-static__used__exitvoid*reference_##x(void)\-{\-return&x;\-}-#else-#define LTO_REFERENCE_INITCALL(x)-#endif--/* initcalls are now grouped by functionality into separate +/*+*initcallsarenowgroupedbyfunctionalityintoseparate*subsections.Orderinginsidethesubsectionsisdetermined*bylinkorder.*Forbackwardscompatibility,initcall()putsthecallin
@@ -2,6 +2,8 @@# Makefile for the linux kernel.#+ccflags-y:=-fno-function-sections-fno-data-sections+obj-y:=main.oversion.omounts.oifneq ($(CONFIG_BLK_DEV_INITRD),y)obj-y+=noinitramfs.o
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 12:12:57
Add an option for architectures to pass over modules after they are
linked. powerpc will use this to fix up alternate instruction patch
relocations.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Documentation/kbuild/makefiles.txt | 6 ++++++
Makefile | 1 +
scripts/Makefile.modpost | 8 ++++++++
3 files changed, 15 insertions(+)
@@ -952,6 +952,12 @@ When kbuild executes, the following steps are followed (roughly): $(KBUILD_ARFLAGS) set by the top level Makefile to "D" (deterministic mode) if this option is supported by $(AR).+ KBUILD_MODPOST_TOOL Arch-specific command to run after module link++ $(KBUILD_MODPOST_TOOL) is used to add an arch-specific pass over+ modules after their final link. E.g., powerpc uses this to adjust+ relative branches of "alternate code patching" sections.+ ARCH_CPPFLAGS, ARCH_AFLAGS, ARCH_CFLAGS Overrides the kbuild defaults These variables are appended to the KBUILD_CPPFLAGS,
@@ -421,6 +421,7 @@ export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULEexportKBUILD_AFLAGS_MODULEKBUILD_CFLAGS_MODULEKBUILD_LDFLAGS_MODULEexportKBUILD_AFLAGS_KERNELKBUILD_CFLAGS_KERNELexportKBUILD_ARFLAGS+exportKBUILD_MODPOST_TOOL# When compiling out-of-tree modules, put MODVERDIR in the module# tree rather than in the kernel tree. The kernel tree might
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 12:13:02
From: Stephen Rothwell <redacted>
Some change to the way we invoke ar is required so it can be used
by scripts/link-vmlinux.sh
Signed-off-by: Stephen Rothwell <redacted>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/Makefile | 6 ++++--
arch/powerpc/platforms/Kconfig.cputype | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 13:33:02
On Fri, 5 Aug 2016 22:11:58 +1000
Nicholas Piggin [off-list ref] wrote:
Hello,
I have 3 different things in this patchset. All arch specific, but all
involve kbuild changes, so I'd like to discuss them with kbuild
maintainers. The goal has been to improve long standing linking
difficulties with the powerpc kernel.
Here's a 30 second hack of an x86 patch. It seems to build and
boot defconfig in a really quick kvm test.
For x86-64 machine building x86 target, defconfig,
make -j8 vmlinux time:
orig thinarc thinarc+dce
real 4m58.865s 4m59.747s 5m20.028s
user 15m14.428s 15m13.868s 15m17.012s
sys 0m57.296s 0m55.904s 0m58.416s
build output directory size:
orig thinarc thinarc+dce
317M 257M 285M
vmlinux size:
text data bss dec filename
10192338 4360136 1105920 15658394 vmlinux
10186739 4356040 1105920 15648699 vmlinux.thinarc
9580486 3759880 1011712 14352078 vmlinux.thinarc+dce
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-05 13:56:54
On Fri, 5 Aug 2016 22:12:01 +1000
Nicholas Piggin [off-list ref] wrote:
Add an option for architectures to pass over modules after they are
linked. powerpc will use this to fix up alternate instruction patch
relocations.
For that matter, now I think about it, I'd like to have this generic
postmod pass for the vmlinux as well. And it would be to call into
the arch Makefile rather than just supply a tool.
Currently powerpc deals with it by adding dependencies on its zImage
target, but it would be really nice to be able to fix that while we're
here too. Is that going to work?
Thanks,
Nick
From: kbuild test robot <hidden> Date: 2016-08-06 03:51:27
Hi Stephen,
[auto build test ERROR on kbuild/for-next]
[also build test ERROR on v4.7]
[cannot apply to linus/master linux/master next-20160805]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/kbuild-changes-thin-archives-gc-sections/20160805-202258
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild.git for-next
config: um-allnoconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=um
All errors (new ones prefixed by >>):
init/built-in.o:(.bss+0x4): multiple definition of `reset_devices'
init/built-in.o:(.bss+0x4): first defined here
init/built-in.o: In function `prepare_namespace':
(.init.text+0x988): multiple definition of `prepare_namespace'
init/built-in.o:(.init.text+0x988): first defined here
init/built-in.o:(.init.data+0x1070): multiple definition of `late_time_init'
init/built-in.o:(.init.data+0x1070): first defined here
init/built-in.o: In function `load_default_modules':
(.init.text+0x2f8): multiple definition of `load_default_modules'
init/built-in.o:(.init.text+0x2f8): first defined here
init/built-in.o:(.bss+0x10): multiple definition of `system_state'
init/built-in.o:(.bss+0x10): first defined here
init/built-in.o: In function `mount_root':
(.init.text+0x987): multiple definition of `mount_root'
init/built-in.o:(.init.text+0x987): first defined here
init/built-in.o: In function `mount_block_root':
(.init.text+0x836): multiple definition of `mount_block_root'
init/built-in.o:(.init.text+0x836): first defined here
init/built-in.o:(.bss+0x14): multiple definition of `early_boot_irqs_disabled'
init/built-in.o:(.bss+0x14): first defined here
init/built-in.o:(.data+0x0): multiple definition of `loops_per_jiffy'
init/built-in.o:(.data+0x0): first defined here
init/built-in.o:(.bss+0xc): multiple definition of `saved_command_line'
init/built-in.o:(.bss+0xc): first defined here
init/built-in.o: In function `calibrate_delay':
(.text+0x2e0): multiple definition of `calibrate_delay'
init/built-in.o:(.text+0x2e0): first defined here
init/built-in.o: In function `do_one_initcall':
(.init.text+0x57a): multiple definition of `do_one_initcall'
init/built-in.o:(.init.text+0x57a): first defined here
init/built-in.o:(.rodata+0x20): multiple definition of `linux_proc_banner'
init/built-in.o:(.rodata+0x20): first defined here
init/built-in.o:(.init.data+0x20e4): multiple definition of `rd_doload'
init/built-in.o:(.init.data+0x20e4): first defined here
init/built-in.o:(.data+0x620): multiple definition of `init_task'
init/built-in.o:(.data+0x620): first defined here
init/built-in.o:(.data+0x460): multiple definition of `init_uts_ns'
init/built-in.o:(.data+0x460): first defined here
init/built-in.o:(.data+0x20): multiple definition of `envp_init'
init/built-in.o:(.data+0x20): first defined here
init/built-in.o: In function `name_to_dev_t':
(.text+0x70): multiple definition of `name_to_dev_t'
init/built-in.o:(.text+0x70): first defined here
init/built-in.o:(.data+0x618): multiple definition of `root_mountflags'
init/built-in.o:(.data+0x618): first defined here
init/built-in.o:(.bss+0x8): multiple definition of `static_key_initialized'
init/built-in.o:(.bss+0x8): first defined here
init/built-in.o: In function `start_kernel':
(.init.text+0x351): multiple definition of `start_kernel'
init/built-in.o:(.init.text+0x351): first defined here
quoted
init/built-in.o:(.bss+0x30): multiple definition of `Version_263936'
init/built-in.o:(.bss+0x30): first defined here
init/built-in.o: In function `parse_early_options':
(.init.text+0x2f9): multiple definition of `parse_early_options'
init/built-in.o:(.init.text+0x2f9): first defined here
init/built-in.o: In function `parse_early_param':
(.init.text+0x31a): multiple definition of `parse_early_param'
init/built-in.o:(.init.text+0x31a): first defined here
init/built-in.o:(.bss+0x40): multiple definition of `preset_lpj'
init/built-in.o:(.bss+0x40): first defined here
init/built-in.o:(.data..init_task+0x0): multiple definition of `init_thread_union'
init/built-in.o:(.data..init_task+0x0): first defined here
init/built-in.o: In function `init_rootfs':
(.init.text+0x80a): multiple definition of `init_rootfs'
init/built-in.o:(.init.text+0x80a): first defined here
init/built-in.o:(.rodata+0x80): multiple definition of `linux_banner'
init/built-in.o:(.rodata+0x80): first defined here
init/built-in.o:(.bss+0x34): multiple definition of `ROOT_DEV'
init/built-in.o:(.bss+0x34): first defined here
init/built-in.o:(.bss+0x0): multiple definition of `initcall_debug'
init/built-in.o:(.bss+0x0): first defined here
init/built-in.o:(.init.data+0x1080): multiple definition of `boot_command_line'
init/built-in.o:(.init.data+0x1080): first defined here
init/built-in.o:(.bss+0x44): multiple definition of `lpj_fine'
init/built-in.o:(.bss+0x44): first defined here
collect2: error: ld returned 1 exit status
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Sam Ravnborg <hidden> Date: 2016-08-06 20:16:34
On Fri, Aug 05, 2016 at 10:12:01PM +1000, Nicholas Piggin wrote:
quoted hunk
Add an option for architectures to pass over modules after they are
linked. powerpc will use this to fix up alternate instruction patch
relocations.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Documentation/kbuild/makefiles.txt | 6 ++++++
Makefile | 1 +
scripts/Makefile.modpost | 8 ++++++++
3 files changed, 15 insertions(+)
@@ -952,6 +952,12 @@ When kbuild executes, the following steps are followed (roughly): $(KBUILD_ARFLAGS) set by the top level Makefile to "D" (deterministic mode) if this option is supported by $(AR).+ KBUILD_MODPOST_TOOL Arch-specific command to run after module link++ $(KBUILD_MODPOST_TOOL) is used to add an arch-specific pass over+ modules after their final link. E.g., powerpc uses this to adjust+ relative branches of "alternate code patching" sections.+
This needs documentation in kbuild.txt, where there is a
nearly full lst of KBUILD_ variables.
Sam
From: Sam Ravnborg <hidden> Date: 2016-08-06 20:19:44
Hi Nicholas.
On Fri, Aug 05, 2016 at 10:11:59PM +1000, Nicholas Piggin wrote:
From: Stephen Rothwell <redacted>
ld -r is an incremental link used to create built-in.o files in build
subdirectories. It produces relocatable object files containing all
its input files, and these are are then pulled together and relocated
in the final link. Aside from the bloat, this constrains the final
link relocations, which has bitten large powerpc builds with
unresolvable relocations in the final link.
Alan Modra has recommended the kernel use thin archives for linking.
This is an alternative and means that the linker has more information
available to it when it links the kernel.
This patch enables a config option architectures can select,
If we want to do this, then I suggest to make the logic reverse.
Architectures that for some reasons cannot use this should
have the possibility to avoid it. But let it be enabled by default.
which
causes all built-in.o files to be built as thin archives. built-in.o
files in subdirectories do not get symbol table or index attached,
which improves speed and size. The final link pass creates a
built-in.o archive in the root output directory which includes the
symbol table and index. The linker then uses takes this file to link.
The --whole-archive linker option is required, because the linker now
has visibility to every individual object file, and it will otherwise
just completely avoid including those without external references
(consider a file with EXPORT_SYMBOL or initcall or hardware exceptions
as its only entry points). The traditional built works "by luck" as
built-in.o files are large enough that they're going to get external
references. However this optimisation is unpredictable for the kernel
(due to above external references), ineffective at culling unused, and
costly because the .o files have to be searched for references.
Superior alternatives for link-time culling should be used instead.
Build characteristics for inclink vs thinarc, on a small powerpc64le
pseries VM with a modest .config:
inclink thinarc
sizes
vmlinux 15 618 680 15 625 028
sum of all built-in.o 56 091 808 1 054 334
sum excluding root built-in.o 151 430
find -name built-in.o | xargs rm ; time make vmlinux
real 22.772s 21.143s
user 13.280s 13.430s
sys 4.310s 2.750s
- Final kernel pulled in only about 6K more, which shows how
ineffective the object file culling is.
- Build performance looks improved due to less pagecache activity.
On IO constrained systems it could be a bigger win.
- Build size saving is significant.
Good to see this old proposal picked up again!
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
See below for an updated simple patch on top of mainline.
Build statistics for "make defconfig" on my i7 box:
find -name built-in.o; xargs rm; time make -j16 vmlinux
standard singlelink delta
real 0m6.368s 0m7.040s +672ms
user 0m15.577s 0m14.960s -617ms
sys 0m7.601s 0m6.226s -1375ms
vmlinux size: standard singlelink delta
text 10.250.675 10.250.675 0
data 4.369.632 4.374.816 +5184
bss 1.110.016 1.110.016 0
I had expected to see improvements in build time - but
we serialize the heavy link phase, so it is actually slower.
I did not investigate why data section got larger,
but I think you already touch the reasons.
The patch does not change how we link modules.
Please consider if this approach is better / worse than
using archieves.
Note that this patch remove the possibility to run section
mismatch anylysis on a per-directory basis.
Sam
@@ -360,10 +360,9 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;ifdef builtin-targetquiet_cmd_link_o_target=LD$@# If the list of objects to link is empty, just create an empty built-in.o-cmd_link_o_target=$(if$(strip$(obj-y)),\-$(LD)$(ld_flags)-r-o$@$(filter$(obj-y),$^)\-$(cmd_secanalysis),\-rm-f$@;$(AR)rcs$(KBUILD_ARFLAGS)$@)+cmd_link_o_target=$(if$(filter$(obj-y),$^),\+echoINPUT\($(filter$(obj-y),$^)\)>$@,\+echo"/* empty */">$@)$(builtin-target):$(obj-y)FORCE$(callif_changed,link_o_target)
From: Sam Ravnborg <hidden> Date: 2016-08-06 20:19:45
On Fri, Aug 05, 2016 at 10:12:00PM +1000, Nicholas Piggin wrote:
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections.
Can you please try to come up with a less cryptic name.
"DCE" may make sense for you today.
Bot the naive reader will benefit from the longer and
more explcit form.
It requires some work (documented) to ensure all
quoted hunk
unreferenced entrypoints are live, and requires toolchain and
build verification, so it is made a per-arch option for now.
On a random powerpc64le build, this yelds a significant size saving,
it boots and runs fine, but there is a lot I haven't tested as yet,
so these savings may be reduced if there are bugs in the link.
text data bss dec filename
11169741 1180744 1923176 14273661 vmlinux
10445269 1004127 1919707 13369103 vmlinux.dce
~700K text, ~170K data, 6% removed from kernel image size.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Makefile | 10 ++++++++
arch/Kconfig | 13 ++++++++++
include/asm-generic/vmlinux.lds.h | 52 ++++++++++++++++++++++-----------------
include/linux/compiler.h | 18 ++++++++++++++
include/linux/export.h | 30 +++++++++++-----------
include/linux/init.h | 38 ++++++++++------------------
init/Makefile | 2 ++
7 files changed, 100 insertions(+), 63 deletions(-)
From: Stephen Rothwell <hidden> Date: 2016-08-07 01:49:49
Hi Sam,
On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg [off-list ref] wrote:
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
The problem with that idea was that (at least for some versions of
binutils in use at the time) we hit a static limit to the number of
object files and ld just stopped at that point. :-(
See below for an updated simple patch on top of mainline.
So, I guess it was fixed, but do we know in what version?
--
Cheers,
Stephen Rothwell
From: Alan Modra <hidden> Date: 2016-08-07 03:35:06
On Sun, Aug 07, 2016 at 11:49:46AM +1000, Stephen Rothwell wrote:
Hi Sam,
On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg [off-list ref] wrote:
quoted
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
The problem with that idea was that (at least for some versions of
binutils in use at the time) we hit a static limit to the number of
object files and ld just stopped at that point. :-(
quoted
See below for an updated simple patch on top of mainline.
So, I guess it was fixed, but do we know in what version?
From: Nicolas Pitre <hidden> Date: 2016-08-07 04:17:23
On Sun, 7 Aug 2016, Stephen Rothwell wrote:
Hi Sam,
On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg [off-list ref] wrote:
quoted
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
The problem with that idea was that (at least for some versions of
binutils in use at the time) we hit a static limit to the number of
object files and ld just stopped at that point. :-(
I played with a nearly identical patch to work around the inability to
do LTO and 'ld -r' with mainline binutils. However this also requires
major changes to modpost otherwise it becomes ineffective.
Nicolas
From: Nicolas Pitre <hidden> Date: 2016-08-07 05:33:50
On Fri, 5 Aug 2016, Nicholas Piggin wrote:
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections. It requires some work (documented) to ensure all
unreferenced entrypoints are live, and requires toolchain and
build verification, so it is made a per-arch option for now.
On a random powerpc64le build, this yelds a significant size saving,
it boots and runs fine, but there is a lot I haven't tested as yet,
so these savings may be reduced if there are bugs in the link.
text data bss dec filename
11169741 1180744 1923176 14273661 vmlinux
10445269 1004127 1919707 13369103 vmlinux.dce
~700K text, ~170K data, 6% removed from kernel image size.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
I played with that too. However this needs distinct sections for
exception tables and the like otherwise the backward references from the
final exception table to those functions responsible for those exception
entries has the effect of pulling in all those functions even if their
entry point is never referenced, making --gc-sections less effective.
I managed to fix this only with a change to gas (accepted upstream).
But once that is solved, you then have the missing forward reference
problem i.e. nothing actually references those individual exception
entry sections and ld happily drops them all. Having a KEEP() on each of
them is unworkable and defeats the purpose anyway. That requires a
dummy reloc to trick ld into pulling in those sections when the parent
section is also pulled in.
Please see attached a subset of the slides I presented at ELC and Linaro
Connect last year to illustrate those issues.
Also attached a sample patch partially implementing those changes.
In short I'm very glad to see that this might steer interest across
multiple architectures. I felt like this was becoming much more
intrusive than I expected and that maybe LTO was a better bet after all.
But LTO has its evils too and I'm willing to look at gc-sections again
if there is interest from others as well.
Nicolas
At the risk of being told you (kernel people) have already considerd
this I thought I should mention that the above isn't ideal. (Nor is
gcc's choice of .text.hot for hot sections, which clashes with
--function-sections for a function called "hot" but that's another
story.)
You'd really like all the hot sections and cold sections to be
together, for better cache locality. So the line ought to have been
*(.text.hot) *(.text) *(.text.fixup) *(.text.unlikely)
That would put all .text.hot sections together. Similarly for
.text.unlikely. The trap of course is that this only works if
.text.fixup from one object file can be placed relatively far away
from .text in the same object file.
If it can, then Nicholas' patch should be:
*(.text.hot .text.hot.*) *(.text.unlikely .text.unlikely.*) *(.text .text.*)
If you can't put .text.fixup too far away then you may as well just use
*(.text .text.*)
--
Alan Modra
Australia Development Lab, IBM
From: Andreas Schwab <hidden> Date: 2016-08-07 11:41:55
On So, Aug 07 2016, Alan Modra [off-list ref] wrote:
At the risk of being told you (kernel people) have already considerd
this I thought I should mention that the above isn't ideal. (Nor is
gcc's choice of .text.hot for hot sections, which clashes with
--function-sections for a function called "hot" but that's another
story.)
Or --function-sections should use a unique prefix, like
.text.functions.*
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
From: Sam Ravnborg <hidden> Date: 2016-08-07 14:41:03
On Sun, Aug 07, 2016 at 11:49:46AM +1000, Stephen Rothwell wrote:
Hi Sam,
On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg [off-list ref] wrote:
quoted
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
The problem with that idea was that (at least for some versions of
binutils in use at the time) we hit a static limit to the number of
object files and ld just stopped at that point. :-(
The ld bug was caused by opening too many linked definitions files.
We can workaround this by expanding the files.
I gave this a quick spin - see below.
Note - I have no idea if using thin archived or this method is better.
But it seems just wrong to me that we convert to thin archives when
we really do not need to do so.
Note - this was a quick spin. It build fine here and thats it.
Sam
@@ -360,10 +360,16 @@ $(sort $(subdir-obj-y)): $(subdir-ym) ;ifdef builtin-targetquiet_cmd_link_o_target=LD$@# If the list of objects to link is empty, just create an empty built-in.o-cmd_link_o_target=$(if$(strip$(obj-y)),\-$(LD)$(ld_flags)-r-o$@$(filter$(obj-y),$^)\-$(cmd_secanalysis),\-rm-f$@;$(AR)rcs$(KBUILD_ARFLAGS)$@)+cmd_link_o_target=\+$(if$(filter$(obj-y), $^), \+echo$(foreachfile,$(filter$(obj-y),$^),\+$(if$(filter%/built-in.o,$(file)),\+$(shellcat$(file)),\+$(file)\+)\+)\+,echo""\+)>$@$(builtin-target):$(obj-y)FORCE$(callif_changed,link_o_target)
@@ -162,6 +162,23 @@ case "${KCONFIG_CONFIG}" in."./${KCONFIG_CONFIG}"esac+# Expand built-in.o file as they just list .o files+forfin${KBUILD_VMLINUX_INIT};do+if[$(basename$f)=built-in.o];then+vmlinux_init="${vmlinux_init}$(cat$f)"+else+vmlinux_init="${vmlinux_init}$f"+fi+done++forfin${KBUILD_VMLINUX_MAIN};do+if[$(basename$f)=built-in.o];then+vmlinux_main="${vmlinux_main}$(cat$f)"+else+vmlinux_main="${vmlinux_main}$f"+fi+done+#link vmlinux.o infoLDvmlinux.o modpost_linkvmlinux.o
On Friday, August 5, 2016 10:11:58 PM CEST Nicholas Piggin wrote:
Hello,
I have 3 different things in this patchset. All arch specific, but all
involve kbuild changes, so I'd like to discuss them with kbuild
maintainers. The goal has been to improve long standing linking
difficulties with the powerpc kernel.
* First, building kernel using thin archives rather than incremental
linking. This seems quite clean and is per-arch, so I hope it should
not be too controversial.
* Second, building kernel using -ffunction-sections -fdata-sections,
--gc-sections. Yes, I'm spinning the wheel again. It was motivated
by tiny codesize regression in the first patch, but the results seem
too good to ignore.
* Third, allowing architecture to run a tool over module after it has
been linked. Powerpc wants to use it in order to relocate "alternate
code" instructions that get don't get linked at their runtime
address. No idea if this is the right approach wrt kbuild, but it
seems to work.
I have included the powerpc code for the first two as a reference. The
third is much bigger and mostly uninteresting for this cc list, but it
can be found here:
https://patchwork.ozlabs.org/patch/651006/
Comments appreciated.
I've started tested this a bit on ARM now. The first things I noticed
are:
1. /home/arnd/cross-gcc/bin/arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of wchar_t values across objects may fail
(actually this one has been present since the first version that
stopped using recursive linking, I did 971a69db7dc0 ("Xen: don't
warn about 2-byte wchar_t in efi") when I first saw the bug, but
that fix no longer works and we have to do this differently
2. big-endian builds on ARM stopped working, I now get
22:53:02 CC init/do_mounts_md.o
22:53:02 LD init/mounts.o
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-ld: init/do_mounts.o: compiled for a big endian system and target is little endian
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-ld: failed to merge target specific data of file init/do_mounts.o
The problem seems to be that we don't pass the correct linker
flags any more, it should be using --be8 from
arch/arm/Makefile:LDFLAGS_vmlinux += --be8
arch/arm/Makefile:LDFLAGS_MODULE += --be8
but that somehow is lost.
3. drivers/firmware/efi/libstub/lib.a: error adding symbols: Archive has no index; run ranlib to add one
haven't investigated at all, turned off EFI for now.
Arnd
On Sunday, August 7, 2016 7:27:39 PM CEST Alan Modra wrote:
If it can, then Nicholas' patch should be:
*(.text.hot .text.hot.*) *(.text.unlikely .text.unlikely.*) *(.text .text.*)
If you can't put .text.fixup too far away then you may as well just use
*(.text .text.*)
but that failed to link an allyesconfig kernel because of references
from .fixup to .text.*. Trying your version now:
*(.text.hot .text.hot.*) *(.text.unlikely .text.unlikely.*) *(.text .text.*)
Arnd
From: Alan Modra <hidden> Date: 2016-08-07 23:49:54
On Sun, Aug 07, 2016 at 10:26:19PM +0200, Arnd Bergmann wrote:
quoted hunk
On Sunday, August 7, 2016 7:27:39 PM CEST Alan Modra wrote:
quoted
If it can, then Nicholas' patch should be:
*(.text.hot .text.hot.*) *(.text.unlikely .text.unlikely.*) *(.text .text.*)
If you can't put .text.fixup too far away then you may as well just use
*(.text .text.*)
Which means this is guaranteed to fail when you test it properly using
gcc's profiling options, in order to generate .text.hot* and/or
.text.unlikely* sections.
It seems to me the right thing to do would be to change kernel asm to
generate .text.foo.fixup for any .text.foo section. A gas feature
available with binutils-2.26 enabled by --sectname-subst might help
with implementing that.
--
Alan Modra
Australia Development Lab, IBM
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 03:19:51
On Sun, 7 Aug 2016 16:40:54 +0200
Sam Ravnborg [off-list ref] wrote:
On Sun, Aug 07, 2016 at 11:49:46AM +1000, Stephen Rothwell wrote:
quoted
Hi Sam,
On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg [off-list ref] wrote:
quoted
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
The problem with that idea was that (at least for some versions of
binutils in use at the time) we hit a static limit to the number of
object files and ld just stopped at that point. :-(
The ld bug was caused by opening too many linked definitions files.
We can workaround this by expanding the files.
I gave this a quick spin - see below.
Note - I have no idea if using thin archived or this method is better.
But it seems just wrong to me that we convert to thin archives when
we really do not need to do so.
Note - this was a quick spin. It build fine here and thats it.
Is there a reason to prefer using linker scripts rather than thin
archives? I thought the former was possibly a bit less robust, and
the latter a smaller change for scripts and toolchain in terms
of "almost behaving like an object file".
I don't have a strong preference although do have a couple of
(out of tree) scripts that expect objdump to work on built-in.o
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 03:25:44
On Sat, 6 Aug 2016 22:10:45 +0200
Sam Ravnborg [off-list ref] wrote:
Hi Nicholas.
On Fri, Aug 05, 2016 at 10:11:59PM +1000, Nicholas Piggin wrote:
quoted
From: Stephen Rothwell <redacted>
ld -r is an incremental link used to create built-in.o files in build
subdirectories. It produces relocatable object files containing all
its input files, and these are are then pulled together and relocated
in the final link. Aside from the bloat, this constrains the final
link relocations, which has bitten large powerpc builds with
unresolvable relocations in the final link.
Alan Modra has recommended the kernel use thin archives for linking.
This is an alternative and means that the linker has more information
available to it when it links the kernel.
This patch enables a config option architectures can select,
If we want to do this, then I suggest to make the logic reverse.
Architectures that for some reasons cannot use this should
have the possibility to avoid it. But let it be enabled by default.
I was thinking the build matrix (architectures x build options x toolchains)
is a bit too large to switch it for everybody. I've far from even tested it
for a fraction of powerpc builds. I would prefer arch maintainers to switch
it themselves, but I do hope we can move everybody and just remove the old
method within a few releases.
But I'm happy to go with whatever arch and kbuild maintainers prefer, so I
appreciate any discussion on it.
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 03:29:12
On Sat, 6 Aug 2016 22:14:23 +0200
Sam Ravnborg [off-list ref] wrote:
On Fri, Aug 05, 2016 at 10:12:00PM +1000, Nicholas Piggin wrote:
quoted
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections.
Can you please try to come up with a less cryptic name.
"DCE" may make sense for you today.
Bot the naive reader will benefit from the longer and
more explcit form.
Yes that's a good idea. The name sucks.
We don't seem to consistently have a prefix for build configuration
options, but we could start? KBUILD_LD_DEAD_CODE_DATA_ELIMINATION?
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 03:30:10
On Sat, 6 Aug 2016 22:16:29 +0200
Sam Ravnborg [off-list ref] wrote:
On Fri, Aug 05, 2016 at 10:12:01PM +1000, Nicholas Piggin wrote:
quoted
Add an option for architectures to pass over modules after they are
linked. powerpc will use this to fix up alternate instruction patch
relocations.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Documentation/kbuild/makefiles.txt | 6 ++++++
Makefile | 1 +
scripts/Makefile.modpost | 8 ++++++++
3 files changed, 15 insertions(+)
@@ -952,6 +952,12 @@ When kbuild executes, the following steps are followed (roughly): $(KBUILD_ARFLAGS) set by the top level Makefile to "D" (deterministic mode) if this option is supported by $(AR).+ KBUILD_MODPOST_TOOL Arch-specific command to run after module link++ $(KBUILD_MODPOST_TOOL) is used to add an arch-specific pass over+ modules after their final link. E.g., powerpc uses this to adjust+ relative branches of "alternate code patching" sections.+
This needs documentation in kbuild.txt, where there is a
nearly full lst of KBUILD_ variables.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 03:42:45
On Sun, 7 Aug 2016 01:33:45 -0400 (EDT)
Nicolas Pitre [off-list ref] wrote:
On Fri, 5 Aug 2016, Nicholas Piggin wrote:
quoted
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections. It requires some work (documented) to ensure all
unreferenced entrypoints are live, and requires toolchain and
build verification, so it is made a per-arch option for now.
On a random powerpc64le build, this yelds a significant size saving,
it boots and runs fine, but there is a lot I haven't tested as yet,
so these savings may be reduced if there are bugs in the link.
text data bss dec filename
11169741 1180744 1923176 14273661 vmlinux
10445269 1004127 1919707 13369103 vmlinux.dce
~700K text, ~170K data, 6% removed from kernel image size.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
I played with that too. However this needs distinct sections for
exception tables and the like otherwise the backward references from the
final exception table to those functions responsible for those exception
entries has the effect of pulling in all those functions even if their
entry point is never referenced, making --gc-sections less effective.
I managed to fix this only with a change to gas (accepted upstream).
But once that is solved, you then have the missing forward reference
problem i.e. nothing actually references those individual exception
entry sections and ld happily drops them all. Having a KEEP() on each of
them is unworkable and defeats the purpose anyway. That requires a
dummy reloc to trick ld into pulling in those sections when the parent
section is also pulled in.
Right, although we don't *need* those things just for enabling
--gc-sections, do we? It may not be 100% optimal, but it's enough
to avoid the regression when switching to --whole-archive build
option.
Please see attached a subset of the slides I presented at ELC and Linaro
Connect last year to illustrate those issues.
Also attached a sample patch partially implementing those changes.
In short I'm very glad to see that this might steer interest across
multiple architectures. I felt like this was becoming much more
intrusive than I expected and that maybe LTO was a better bet after all.
But LTO has its evils too and I'm willing to look at gc-sections again
if there is interest from others as well.
Your results are impressive, and I don't want to stand in the way of
either LTO or improving accuracy of --gc-sections. But both are things
that can be built on top of this patch, I think. We don't need to do
the entire intrusive changes all at once.
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 03:54:01
On Sun, 07 Aug 2016 22:23:02 +0200
Arnd Bergmann [off-list ref] wrote:
On Friday, August 5, 2016 10:11:58 PM CEST Nicholas Piggin wrote:
quoted
Hello,
I have 3 different things in this patchset. All arch specific, but all
involve kbuild changes, so I'd like to discuss them with kbuild
maintainers. The goal has been to improve long standing linking
difficulties with the powerpc kernel.
* First, building kernel using thin archives rather than incremental
linking. This seems quite clean and is per-arch, so I hope it should
not be too controversial.
* Second, building kernel using -ffunction-sections -fdata-sections,
--gc-sections. Yes, I'm spinning the wheel again. It was motivated
by tiny codesize regression in the first patch, but the results seem
too good to ignore.
* Third, allowing architecture to run a tool over module after it has
been linked. Powerpc wants to use it in order to relocate "alternate
code" instructions that get don't get linked at their runtime
address. No idea if this is the right approach wrt kbuild, but it
seems to work.
I have included the powerpc code for the first two as a reference. The
third is much bigger and mostly uninteresting for this cc list, but it
can be found here:
https://patchwork.ozlabs.org/patch/651006/
Comments appreciated.
I've started tested this a bit on ARM now. The first things I noticed
are:
1. /home/arnd/cross-gcc/bin/arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of wchar_t values across objects may fail
(actually this one has been present since the first version that
stopped using recursive linking, I did 971a69db7dc0 ("Xen: don't
warn about 2-byte wchar_t in efi") when I first saw the bug, but
that fix no longer works and we have to do this differently
2. big-endian builds on ARM stopped working, I now get
22:53:02 CC init/do_mounts_md.o
22:53:02 LD init/mounts.o
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-ld: init/do_mounts.o: compiled for a big endian system and target is little endian
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-ld: failed to merge target specific data of file init/do_mounts.o
The problem seems to be that we don't pass the correct linker
flags any more, it should be using --be8 from
arch/arm/Makefile:LDFLAGS_vmlinux += --be8
arch/arm/Makefile:LDFLAGS_MODULE += --be8
but that somehow is lost.
These both seem like linker flags are being lost for some reason, but I
don't really know why. Maybe ARM's Makefile. I don't have an ARM build setup
but I'll try to help track it down when I get some time.
3. drivers/firmware/efi/libstub/lib.a: error adding symbols: Archive has no index; run ranlib to add one
haven't investigated at all, turned off EFI for now.
Okay this looks like a bug in patch 2 (caused by me, not Stephen!)
For thin archives, cmd_link_l_target should be:
cmd_link_l_target = rm -f $@; $(AR) rcT$(KBUILD_ARFLAGS) $@ $(lib-y)
(note no "S" option).
Thanks,
Nick
From: Nicolas Pitre <hidden> Date: 2016-08-08 04:12:42
On Mon, 8 Aug 2016, Nicholas Piggin wrote:
On Sun, 7 Aug 2016 01:33:45 -0400 (EDT)
Nicolas Pitre [off-list ref] wrote:
quoted
On Fri, 5 Aug 2016, Nicholas Piggin wrote:
quoted
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections. It requires some work (documented) to ensure all
unreferenced entrypoints are live, and requires toolchain and
build verification, so it is made a per-arch option for now.
On a random powerpc64le build, this yelds a significant size saving,
it boots and runs fine, but there is a lot I haven't tested as yet,
so these savings may be reduced if there are bugs in the link.
text data bss dec filename
11169741 1180744 1923176 14273661 vmlinux
10445269 1004127 1919707 13369103 vmlinux.dce
~700K text, ~170K data, 6% removed from kernel image size.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
I played with that too. However this needs distinct sections for
exception tables and the like otherwise the backward references from the
final exception table to those functions responsible for those exception
entries has the effect of pulling in all those functions even if their
entry point is never referenced, making --gc-sections less effective.
I managed to fix this only with a change to gas (accepted upstream).
But once that is solved, you then have the missing forward reference
problem i.e. nothing actually references those individual exception
entry sections and ld happily drops them all. Having a KEEP() on each of
them is unworkable and defeats the purpose anyway. That requires a
dummy reloc to trick ld into pulling in those sections when the parent
section is also pulled in.
Right, although we don't *need* those things just for enabling
--gc-sections, do we? It may not be 100% optimal, but it's enough
to avoid the regression when switching to --whole-archive build
option.
Oh absolutely.
Your results are impressive, and I don't want to stand in the way of
either LTO or improving accuracy of --gc-sections. But both are things
that can be built on top of this patch, I think.
Indeed. Those patches are certainly welcome. They represent half of the
job already. I just wanted to provide some insight about the whole
picture in case someone else notices those flaws I have identified.
Nicolas
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-08-08 04:27:52
On Mon, 8 Aug 2016 00:12:37 -0400 (EDT)
Nicolas Pitre [off-list ref] wrote:
On Mon, 8 Aug 2016, Nicholas Piggin wrote:
quoted
On Sun, 7 Aug 2016 01:33:45 -0400 (EDT)
Nicolas Pitre [off-list ref] wrote:
quoted
On Fri, 5 Aug 2016, Nicholas Piggin wrote:
quoted
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections. It requires some work (documented) to ensure all
unreferenced entrypoints are live, and requires toolchain and
build verification, so it is made a per-arch option for now.
On a random powerpc64le build, this yelds a significant size saving,
it boots and runs fine, but there is a lot I haven't tested as yet,
so these savings may be reduced if there are bugs in the link.
text data bss dec filename
11169741 1180744 1923176 14273661 vmlinux
10445269 1004127 1919707 13369103 vmlinux.dce
~700K text, ~170K data, 6% removed from kernel image size.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
I played with that too. However this needs distinct sections for
exception tables and the like otherwise the backward references from the
final exception table to those functions responsible for those exception
entries has the effect of pulling in all those functions even if their
entry point is never referenced, making --gc-sections less effective.
I managed to fix this only with a change to gas (accepted upstream).
But once that is solved, you then have the missing forward reference
problem i.e. nothing actually references those individual exception
entry sections and ld happily drops them all. Having a KEEP() on each of
them is unworkable and defeats the purpose anyway. That requires a
dummy reloc to trick ld into pulling in those sections when the parent
section is also pulled in.
Right, although we don't *need* those things just for enabling
--gc-sections, do we? It may not be 100% optimal, but it's enough
to avoid the regression when switching to --whole-archive build
option.
Oh absolutely.
quoted
Your results are impressive, and I don't want to stand in the way of
either LTO or improving accuracy of --gc-sections. But both are things
that can be built on top of this patch, I think.
Indeed. Those patches are certainly welcome. They represent half of the
job already. I just wanted to provide some insight about the whole
picture in case someone else notices those flaws I have identified.
Okay thanks, I appreciate you taking a look. I wanted to be sure I
wasn't missing some bug here.
Smaller kernel is nice for large systems because it means smaller
icache/dcache footprint and fewer branch trampolines, so I'm always
happy to see that effort. I will certainly help test LTO or some of
these other gc-sections improvements on powerpc.
Thanks,
Nick
From: Sam Ravnborg <hidden> Date: 2016-08-08 04:46:34
On Mon, Aug 08, 2016 at 01:19:41PM +1000, Nicholas Piggin wrote:
On Sun, 7 Aug 2016 16:40:54 +0200
Sam Ravnborg [off-list ref] wrote:
quoted
On Sun, Aug 07, 2016 at 11:49:46AM +1000, Stephen Rothwell wrote:
quoted
Hi Sam,
On Sat, 6 Aug 2016 22:10:45 +0200 Sam Ravnborg [off-list ref] wrote:
quoted
Did you by any chance evalue the use of INPUT in linker files.
Stephen back then (again based on proposal from Alan Modra),
also made an implementation using INPUT.
The problem with that idea was that (at least for some versions of
binutils in use at the time) we hit a static limit to the number of
object files and ld just stopped at that point. :-(
The ld bug was caused by opening too many linked definitions files.
We can workaround this by expanding the files.
I gave this a quick spin - see below.
Note - I have no idea if using thin archived or this method is better.
But it seems just wrong to me that we convert to thin archives when
we really do not need to do so.
Note - this was a quick spin. It build fine here and thats it.
Is there a reason to prefer using linker scripts rather than thin
archives? I thought the former was possibly a bit less robust, and
the latter a smaller change for scripts and toolchain in terms
of "almost behaving like an object file".
The only valid reason I can come up with is that it is simpler
to build a list of .o files than it is to generate a number
of thin archives.
I persuaded "linker scripts" mainly because I had the patch floating
and I was triggered by the powerpc discussions to resurface it again.
I don't have a strong preference although do have a couple of
(out of tree) scripts that expect objdump to work on built-in.o
You are likely not alone here.
Unless someone else comes up with any good reason lets stay with
thin archives.
Sam
From: Sam Ravnborg <hidden> Date: 2016-08-08 04:50:01
On Mon, Aug 08, 2016 at 01:29:03PM +1000, Nicholas Piggin wrote:
On Sat, 6 Aug 2016 22:14:23 +0200
Sam Ravnborg [off-list ref] wrote:
quoted
On Fri, Aug 05, 2016 at 10:12:00PM +1000, Nicholas Piggin wrote:
quoted
Introduce LINKER_DCE option for architectures to select if they want
to build with -ffunction-sections, -fdata-sections, and link with
--gc-sections.
Can you please try to come up with a less cryptic name.
"DCE" may make sense for you today.
Bot the naive reader will benefit from the longer and
more explcit form.
Yes that's a good idea. The name sucks.
We don't seem to consistently have a prefix for build configuration
options, but we could start? KBUILD_LD_DEAD_CODE_DATA_ELIMINATION?
For cc related options we sort of have: KCONFIG_CC IIRC.
So for LD related options we could use the shorter CONFIG_LD_ prefix.
Sam
On Monday, August 8, 2016 1:25:32 PM CEST Nicholas Piggin wrote:
On Sat, 6 Aug 2016 22:10:45 +0200
Sam Ravnborg [off-list ref] wrote:
quoted
Hi Nicholas.
On Fri, Aug 05, 2016 at 10:11:59PM +1000, Nicholas Piggin wrote:
quoted
From: Stephen Rothwell <redacted>
ld -r is an incremental link used to create built-in.o files in build
subdirectories. It produces relocatable object files containing all
its input files, and these are are then pulled together and relocated
in the final link. Aside from the bloat, this constrains the final
link relocations, which has bitten large powerpc builds with
unresolvable relocations in the final link.
Alan Modra has recommended the kernel use thin archives for linking.
This is an alternative and means that the linker has more information
available to it when it links the kernel.
This patch enables a config option architectures can select,
If we want to do this, then I suggest to make the logic reverse.
Architectures that for some reasons cannot use this should
have the possibility to avoid it. But let it be enabled by default.
I was thinking the build matrix (architectures x build options x toolchains)
is a bit too large to switch it for everybody. I've far from even tested it
for a fraction of powerpc builds. I would prefer arch maintainers to switch
it themselves, but I do hope we can move everybody and just remove the old
method within a few releases.
But I'm happy to go with whatever arch and kbuild maintainers prefer, so I
appreciate any discussion on it.
How about making the option visible for everyone with CONFIG_EXPERT?
That way you don't get it by default, but you do get it with randconfig
and allmodconfig build testing.
Arnd
On Monday, August 8, 2016 9:19:47 AM CEST Alan Modra wrote:
On Sun, Aug 07, 2016 at 10:26:19PM +0200, Arnd Bergmann wrote:
quoted
On Sunday, August 7, 2016 7:27:39 PM CEST Alan Modra wrote:
quoted
If it can, then Nicholas' patch should be:
*(.text.hot .text.hot.*) *(.text.unlikely .text.unlikely.*) *(.text .text.*)
If you can't put .text.fixup too far away then you may as well just use
*(.text .text.*)
Which means this is guaranteed to fail when you test it properly using
gcc's profiling options, in order to generate .text.hot* and/or
.text.unlikely* sections.
I've investigated further and it seems that "*(.text.fixup) *(.text .text.*)"
fails just because we list .text.fixup twice. The .text.fixup section
was originally[1] introduced to work around the same link error that
it is causing now: if we use recursive linking, merging .text and .text.fixup
helps avoid the problems of sections that are >32MB before the final
link.
I have reverted that patch now, so ARM uses ".fixup" again like every
other architecture does, and now "*(.fixup) *(.text .text.*)" works
correctly, while ""*(.fixup) *(.text .fixup .text.*)" also fails
the same way that I saw before:
drivers/scsi/sg.o:(.fixup+0x4): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0xc): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x14): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x1c): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x24): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x2c): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x34): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x3c): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
drivers/scsi/sg.o:(.fixup+0x44): relocation truncated to fit: R_ARM_THM_JUMP24 against `.text.sg_ioctl'
I don't understand what led Andi Kleen to also move .text.hot and
.text.unlikely together with .text [2], but this may have
been a related issue.
It seems to me the right thing to do would be to change kernel asm to
generate .text.foo.fixup for any .text.foo section. A gas feature
available with binutils-2.26 enabled by --sectname-subst might help
with implementing that.
From: Alan Modra <hidden> Date: 2016-08-08 23:50:23
On Mon, Aug 08, 2016 at 05:14:27PM +0200, Arnd Bergmann wrote:
I have reverted that patch now, so ARM uses ".fixup" again like every
other architecture does, and now "*(.fixup) *(.text .text.*)" works
correctly, while ""*(.fixup) *(.text .fixup .text.*)" also fails
the same way that I saw before:
That is really odd. The linker isn't supposed to treat those script
snippets differently. First match for .fixup wins.
$ cat > fixup1.s <<\EOF
.global _start
.text
_start:
.dc.a .L2
.L1:
.section ".fixup","ax",%progbits
.L2:
.dc.a .L1
EOF
$ cat > fixup2.s <<\EOF
.section ".text.xyz","ax",%progbits
.dc.a .L2
.L1:
.section ".fixup","ax",%progbits
.L2:
.dc.a .L1
EOF
$ cat > fixup.lnk <<\EOF
SECTIONS {
.text : { *(.fixup) *(.text .fixup .text.*) }
}
EOF
$ as -o fixup1.o fixup1.s
$ as -o fixup2.o fixup2.s
$ ld -o fixup -T fixup.lnk -Map fixup.map fixup1.o fixup2.o
$ cat fixup.map
Memory Configuration
Name Origin Length Attributes
*default* 0x0000000000000000 0xffffffffffffffff
Linker script and memory map
.text 0x0000000000000000 0x10
*(.fixup)
.fixup 0x0000000000000000 0x4 fixup1.o
.fixup 0x0000000000000004 0x4 fixup2.o
*(.text .fixup .text.*)
.text 0x0000000000000008 0x4 fixup1.o
0x0000000000000008 _start
.text 0x000000000000000c 0x0 fixup2.o
.text.xyz 0x000000000000000c 0x4 fixup2.o
[snip]
--
Alan Modra
Australia Development Lab, IBM
I don't understand what led Andi Kleen to also move .text.hot and
.text.unlikely together with .text [2], but this may have
been a related issue.
The goal was just to move .hot and .unlikely all together, so that
they are clustered and use the minimum amount of cache. On x86 doesn't
matter where they are exactly, as long as each is together.
If they are not explicitely listed then the linker interleaves
them with the normal text, which defeats the purpose.
-Andi
On Tuesday, August 9, 2016 9:20:16 AM CEST Alan Modra wrote:
On Mon, Aug 08, 2016 at 05:14:27PM +0200, Arnd Bergmann wrote:
quoted
I have reverted that patch now, so ARM uses ".fixup" again like every
other architecture does, and now "*(.fixup) *(.text .text.*)" works
correctly, while ""*(.fixup) *(.text .fixup .text.*)" also fails
the same way that I saw before:
That is really odd. The linker isn't supposed to treat those script
snippets differently. First match for .fixup wins.
Sorry for my mistake. I checked again and cannot reproduce what I
thought I saw earlier. "*(.fixup) *(.text .text.*)" fails
as would be expected.
Arnd
On Monday, August 8, 2016 8:16:05 PM CEST Andi Kleen wrote:
quoted
I don't understand what led Andi Kleen to also move .text.hot and
.text.unlikely together with .text [2], but this may have
been a related issue.
[2] https://lkml.org/lkml/2015/7/19/377
The goal was just to move .hot and .unlikely all together, so that
they are clustered and use the minimum amount of cache. On x86 doesn't
matter where they are exactly, as long as each is together.
If they are not explicitely listed then the linker interleaves
them with the normal text, which defeats the purpose.
I still don't see it, my reading of your patch is that you did
the opposite, by changing the description that puts all .text.hot
in front of .text, and all .text.unlikely after exit.text into
one that mixes them with .text. What am I missing here?
Arnd
On Wed, Aug 10, 2016 at 12:29:29AM +0200, Arnd Bergmann wrote:
On Monday, August 8, 2016 8:16:05 PM CEST Andi Kleen wrote:
quoted
quoted
I don't understand what led Andi Kleen to also move .text.hot and
.text.unlikely together with .text [2], but this may have
been a related issue.
[2] https://lkml.org/lkml/2015/7/19/377
The goal was just to move .hot and .unlikely all together, so that
they are clustered and use the minimum amount of cache. On x86 doesn't
matter where they are exactly, as long as each is together.
If they are not explicitely listed then the linker interleaves
them with the normal text, which defeats the purpose.
I still don't see it, my reading of your patch is that you did
the opposite, by changing the description that puts all .text.hot
in front of .text, and all .text.unlikely after exit.text into
one that mixes them with .text. What am I missing here?
.text.hot is actually not used, the critical part is .text.unlikely
which was not listed and was interleaved before the patch.
-Andi
--
ak@linux.intel.com -- Speaking for myself only
On Wed, Aug 10, 2016 at 12:29:29AM +0200, Arnd Bergmann wrote:
On Monday, August 8, 2016 8:16:05 PM CEST Andi Kleen wrote:
quoted
quoted
I don't understand what led Andi Kleen to also move .text.hot and
.text.unlikely together with .text [2], but this may have
been a related issue.
[2] https://lkml.org/lkml/2015/7/19/377
The goal was just to move .hot and .unlikely all together, so that
they are clustered and use the minimum amount of cache. On x86 doesn't
matter where they are exactly, as long as each is together.
If they are not explicitely listed then the linker interleaves
them with the normal text, which defeats the purpose.
I still don't see it, my reading of your patch is that you did
the opposite, by changing the description that puts all .text.hot
in front of .text, and all .text.unlikely after exit.text into
one that mixes them with .text. What am I missing here?
No it doesn't mix .unlikely with .text, .unlikely is all in one place.
-Andi
--
ak@linux.intel.com -- Speaking for myself only