Re: powerpc allyesconfig / allmodconfig linux-next next-20160729 - next-20160729 build failures

[Arnd Bergmann <arnd@arndb.de>]

On Thursday, August 4, 2016 9:47:13 PM CEST Nicholas Piggin wrote:
> On Thu, 04 Aug 2016 12:37:41 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday, August 4, 2016 11:00:49 AM CEST Arnd Bergmann wrote:
> > > I tried this
> > > 
> > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > index b5e40ed86e60..89bca1a25916 100755
> > > --- a/scripts/link-vmlinux.sh
> > > +++ b/scripts/link-vmlinux.sh
> > > @@ -44,7 +44,7 @@ modpost_link()
> > >         local objects
> > >  
> > >         if [ -n "${CONFIG_THIN_ARCHIVES}" ]; then
> > > -               objects="--whole-archive ${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN} --no-whole-archive"
> > > +               objects="${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN}"
> > >         else
> > >                 objects="${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group"
> > >         fi
> > > 
> > > but that did not seem to change anything, the extra symbols are
> > > still there. I have not tried to understand what that actually
> > > does, so maybe I misunderstood your suggestion.
> > >   
> > 
> > On a second attempt, I did the same change for vmlinux instead of the
> > module (d'oh), and got a link failure instead:
> > 
> > 
> > arch/arm/mm/proc-xscale.o: In function `cpu_xscale_do_resume':
> > (.text+0x3d4): undefined reference to `cpu_resume_mmu'
> > arch/arm/kernel/setup.o: In function `setup_arch':
> > ...
> > 
> > However, I also see a link failure in some rare configurations
> > with just your patch:
> > 
> > arch/arm/lib/lib.a(io-acorn.o): In function `outsl':
> > (.text+0x38): undefined reference to `printk'
> > 
> > The problem being a file in a library object that is not referenced,
> > but that references another symbol that is not defined
> > (CONFIG_PRINTK=n).
> 
> The first problem is the existing link system is buggy. I think an
> unconditional switch to --whole-archive (at least for modular kernels)
> should probably be done anyway. For example, on powerpc when building
> with --whole-archive, I have:
> 
> +dma_noop_alloc
> +dma_noop_free
> +dma_noop_map_page
> +dma_noop_mapping_error
> +dma_noop_map_sg
> +dma_noop_ops
> +dma_noop_supported
> +fdt_add_reservemap_entry
> +fdt_begin_node
> +fdt_create
> +fdt_create_empty_tree
> +fdt_end_node
> +fdt_errtable
> +find_cpio_data
> +ioremap_page_range
> 
> find_cpio_data is unnecessary and it's a codesize regression to link it.
> But dma_noop_ops and ioremap_page_range are exported symbols. If I
> reference dma_noop_ops from some random module with otherwise unpatched
> kernel:
> 
> ERROR: "dma_noop_ops" [drivers/char/bsr.ko] undefined!

Right, but only on s390, which is the one architecture using this.
I think we should just have a Kconfig symbol for this file that
gets selected by any architecture that needs it.

This is also what we have ended up doing for almost all other
files in lib/

> The real problem is that our linkage requirements are like a shared
> library when we build modular.
> 
> We could build a list of exports and make it link objects with those
> symbols, to solve this, but IMO that's just wasting lipstick on a pig.
> But I will to propose a patch to always use --whole-archive, thin
> archives or not, and transition all archs over to it in a few release
> cycles. It just works by luck right now.
>
> Why is it a pig? Because having the linker to notice no external
> references and just skipping the .o completely is trying to use a hammer
> as a scalpel. It's just not a very effective way to eliminate dead code
> --  I pulled in only a handful of unneeded functions by switching it.

If we do that, we may just as well get rid of $(lib-y) in the process and
always use $(obj-y).

> I mean it is a quick simple feature that probably works well enough with
> simple build systems. But not an advanced one that builds almost
> everything on demand and also has loadable modules and must act like a
> shared library.
> 
> Real linker DCE is a valid optimisation that can't be replaced by the
> build system of course, but we need to do it properly. Here's what I'm
> working on.
> 
> It applies on top of the previous patch I sent, plus some powerpc stuff
> I'm working on that you should be able to just ignore for another arch.
> it's a WIP, but if you can see if it works for arm that would be cool.
> 
> It doesn't actually build allyesconfig after this,
> ld: .tmp_vmlinux1: Too many sections: 220655 (>= 65280)
> 
> But on a more reasonable configuration (ppc64le)
>     text      data   bss            dec   filename
> 11191672   1183536   1923820   14299028   vmlinux
> 10625528    861895   1919707   13407130	  vmlinux.thin+gc
> 
> 10M-552K   1M-314K         ~   13M-870K

Nice!

> And it actually boots too, which is fairly astounding considering that
> it lost half a meg of code and 1/3 of its data. I'm not completely sure
> I've not done something wrong...

Nicolas Pitre has done some related work, adding him to Cc. IIRC we have
actually had multiple implementations of -ffunction-sections/--gc-sections
in the past that people have used in production, but none of them
ever made it upstream.

One question is whether we should bother with --gc-sections at all,
or use full LTO instead.

	Arnd
---
(full patch quoted below for Nico, no further comments)

> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index e75e17c..1594072 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -104,6 +104,10 @@ LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
>  LDFLAGS_vmlinux	+= --emit-relocs
>  KBUILD_LDFLAGS_MODULE += --emit-relocs
>  
> +KBUILD_CFLAGS	+= -ffunction-sections -fdata-sections
> +LDFLAGS_vmlinux	+= --gc-sections
> +
> +
>  ifeq ($(CONFIG_PPC64),y)
>  ifeq ($(call cc-option-yn,-mcmodel=medium),y)
>  	# -mcmodel=medium breaks modules because it uses 32bit offsets from
> @@ -234,6 +238,8 @@ KBUILD_CFLAGS += $(cpu-as-y)
>  archscripts: scripts_basic
>  	$(Q)$(MAKE) $(build)=arch/powerpc/tools
>  
> +CFLAGS_head_$(CONFIG_WORD_SIZE).o = -fno-function-sections
> +
>  head-y				:= arch/powerpc/kernel/head_$(CONFIG_WORD_SIZE).o
>  head-$(CONFIG_8xx)		:= arch/powerpc/kernel/head_8xx.o
>  head-$(CONFIG_40x)		:= arch/powerpc/kernel/head_40x.o
> @@ -245,6 +251,7 @@ head-$(CONFIG_PPC_FPU)		+= arch/powerpc/kernel/fpu.o
>  head-$(CONFIG_ALTIVEC)		+= arch/powerpc/kernel/vector.o
>  head-$(CONFIG_PPC_OF_BOOT_TRAMPOLINE)  += arch/powerpc/kernel/prom_init.o
>  
> +
>  core-y				+= arch/powerpc/kernel/ \
>  				   arch/powerpc/mm/ \
>  				   arch/powerpc/lib/ \
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 2da380f..b356e59 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -4,7 +4,10 @@
>  
>  CFLAGS_ptrace.o		+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
>  
> +ccflags-y		+= -fno-function-sections -fno-data-sections
> +
>  subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> +subdir-ccflags-y	+= -fno-function-sections -fno-data-sections
>  
>  ifeq ($(CONFIG_PPC64),y)
>  CFLAGS_prom_init.o	+= $(NO_MINIMAL_TOC)
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index 959c131..0856d62 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -56,16 +56,16 @@ SECTIONS
>  	 * in order to optimize stub generation.
>  	 */
>  	.head.text : AT(ADDR(.head.text) - LOAD_OFFSET) {
> -		*(.head.text.first_256B);
> +		KEEP(*(.head.text.first_256B));
>  #ifndef CONFIG_PPC_BOOK3S
>  		. = 0x100;
>  #else
> -		*(.head.text.real_vectors);
> -		*(.head.text.real_trampolines);
> -		*(.head.text.virt_vectors);
> -		*(.head.text.virt_trampolines);
> +		KEEP(*(.head.text.real_vectors));
> +		KEEP(*(.head.text.real_trampolines));
> +		KEEP(*(.head.text.virt_vectors));
> +		KEEP(*(.head.text.virt_trampolines));
>  #if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV)
> -		*(.head.data.fwnmi_page);
> +		KEEP(*(.head.data.fwnmi_page));
>  		. = 0x8000;
>  #else
>  		. = 0x7000;
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 6a67ab9..3a35719 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -312,76 +312,76 @@
>  	/* Kernel symbol table: Normal symbols */			\
>  	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
>  		VMLINUX_SYMBOL(__start___ksymtab) = .;			\
> -		*(SORT(___ksymtab+*))					\
> +		KEEP(*(SORT(___ksymtab+*)))				\
>  		VMLINUX_SYMBOL(__stop___ksymtab) = .;			\
>  	}								\
>  									\
>  	/* Kernel symbol table: GPL-only symbols */			\
>  	__ksymtab_gpl     : AT(ADDR(__ksymtab_gpl) - LOAD_OFFSET) {	\
>  		VMLINUX_SYMBOL(__start___ksymtab_gpl) = .;		\
> -		*(SORT(___ksymtab_gpl+*))				\
> +		KEEP(*(SORT(___ksymtab_gpl+*)))				\
>  		VMLINUX_SYMBOL(__stop___ksymtab_gpl) = .;		\
>  	}								\
>  									\
>  	/* Kernel symbol table: Normal unused symbols */		\
>  	__ksymtab_unused  : AT(ADDR(__ksymtab_unused) - LOAD_OFFSET) {	\
>  		VMLINUX_SYMBOL(__start___ksymtab_unused) = .;		\
> -		*(SORT(___ksymtab_unused+*))				\
> +		KEEP(*(SORT(___ksymtab_unused+*)))			\
>  		VMLINUX_SYMBOL(__stop___ksymtab_unused) = .;		\
>  	}								\
>  									\
>  	/* Kernel symbol table: GPL-only unused symbols */		\
>  	__ksymtab_unused_gpl : AT(ADDR(__ksymtab_unused_gpl) - LOAD_OFFSET) { \
>  		VMLINUX_SYMBOL(__start___ksymtab_unused_gpl) = .;	\
> -		*(SORT(___ksymtab_unused_gpl+*))			\
> +		KEEP(*(SORT(___ksymtab_unused_gpl+*)))			\
>  		VMLINUX_SYMBOL(__stop___ksymtab_unused_gpl) = .;	\
>  	}								\
>  									\
>  	/* Kernel symbol table: GPL-future-only symbols */		\
>  	__ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \
>  		VMLINUX_SYMBOL(__start___ksymtab_gpl_future) = .;	\
> -		*(SORT(___ksymtab_gpl_future+*))			\
> +		KEEP(*(SORT(___ksymtab_gpl_future+*)))			\
>  		VMLINUX_SYMBOL(__stop___ksymtab_gpl_future) = .;	\
>  	}								\
>  									\
>  	/* Kernel symbol table: Normal symbols */			\
>  	__kcrctab         : AT(ADDR(__kcrctab) - LOAD_OFFSET) {		\
>  		VMLINUX_SYMBOL(__start___kcrctab) = .;			\
> -		*(SORT(___kcrctab+*))					\
> +		KEEP(*(SORT(___kcrctab+*)))				\
>  		VMLINUX_SYMBOL(__stop___kcrctab) = .;			\
>  	}								\
>  									\
>  	/* Kernel symbol table: GPL-only symbols */			\
>  	__kcrctab_gpl     : AT(ADDR(__kcrctab_gpl) - LOAD_OFFSET) {	\
>  		VMLINUX_SYMBOL(__start___kcrctab_gpl) = .;		\
> -		*(SORT(___kcrctab_gpl+*))				\
> +		KEEP(*(SORT(___kcrctab_gpl+*)))				\
>  		VMLINUX_SYMBOL(__stop___kcrctab_gpl) = .;		\
>  	}								\
>  									\
>  	/* Kernel symbol table: Normal unused symbols */		\
>  	__kcrctab_unused  : AT(ADDR(__kcrctab_unused) - LOAD_OFFSET) {	\
>  		VMLINUX_SYMBOL(__start___kcrctab_unused) = .;		\
> -		*(SORT(___kcrctab_unused+*))				\
> +		KEEP(*(SORT(___kcrctab_unused+*)))			\
>  		VMLINUX_SYMBOL(__stop___kcrctab_unused) = .;		\
>  	}								\
>  									\
>  	/* Kernel symbol table: GPL-only unused symbols */		\
>  	__kcrctab_unused_gpl : AT(ADDR(__kcrctab_unused_gpl) - LOAD_OFFSET) { \
>  		VMLINUX_SYMBOL(__start___kcrctab_unused_gpl) = .;	\
> -		*(SORT(___kcrctab_unused_gpl+*))			\
> +		KEEP(*(SORT(___kcrctab_unused_gpl+*)))			\
>  		VMLINUX_SYMBOL(__stop___kcrctab_unused_gpl) = .;	\
>  	}								\
>  									\
>  	/* Kernel symbol table: GPL-future-only symbols */		\
>  	__kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \
>  		VMLINUX_SYMBOL(__start___kcrctab_gpl_future) = .;	\
> -		*(SORT(___kcrctab_gpl_future+*))			\
> +		KEEP(*(SORT(___kcrctab_gpl_future+*)))			\
>  		VMLINUX_SYMBOL(__stop___kcrctab_gpl_future) = .;	\
>  	}								\
>  									\
>  	/* Kernel symbol table: strings */				\
>          __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {	\
> -		*(__ksymtab_strings)					\
> +		KEEP(*(__ksymtab_strings))				\
>  	}								\
>  									\
>  	/* __*init sections */						\
> @@ -519,6 +519,7 @@
>  
>  /* init and exit section handling */
>  #define INIT_DATA							\
> +	KEEP(*(SORT(___kentry+*)))					\
>  	*(.init.data)							\
>  	MEM_DISCARD(init.data)						\
>  	KERNEL_CTORS()							\
> @@ -695,9 +696,9 @@
>  #define INIT_RAM_FS							\
>  	. = ALIGN(4);							\
>  	VMLINUX_SYMBOL(__initramfs_start) = .;				\
> -	*(.init.ramfs)							\
> +	KEEP(*(.init.ramfs))						\
>  	. = ALIGN(8);							\
> -	*(.init.ramfs.info)
> +	KEEP(*(.init.ramfs.info))
>  #else
>  #define INIT_RAM_FS
>  #endif
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 2f9ccbe..a921862 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -46,7 +46,7 @@ extern struct module __this_module;
>  	extern __visible void *__crc_##sym __attribute__((weak));		\
>  	static const unsigned long __kcrctab_##sym		\
>  	__used							\
> -	__attribute__((section("___kcrctab" sec "+" #sym), unused))	\
> +	__attribute__((section("___kcrctab" sec "+" #sym ",\"a\",@note #"), used))	\
>  	= (unsigned long) &__crc_##sym;
>  #else
>  #define __CRC_SYMBOL(sym, sec)
> @@ -57,12 +57,12 @@ extern struct module __this_module;
>  	extern typeof(sym) sym;					\
>  	__CRC_SYMBOL(sym, sec)					\
>  	static const char __kstrtab_##sym[]			\
> -	__attribute__((section("__ksymtab_strings"), aligned(1))) \
> +	__attribute__((section("__ksymtab_strings" ",\"a\",@note #"), aligned(1))) \
>  	= VMLINUX_SYMBOL_STR(sym);				\
>  	extern const struct kernel_symbol __ksymtab_##sym;	\
>  	__visible const struct kernel_symbol __ksymtab_##sym	\
>  	__used							\
> -	__attribute__((section("___ksymtab" sec "+" #sym), unused))	\
> +	__attribute__((section("___ksymtab" sec "+" #sym ",\"a\",@note #"), used))	\
>  	= { (unsigned long)&sym, __kstrtab_##sym }
>  
>  #if defined(__KSYM_DEPS__)
> diff --git a/include/linux/init.h b/include/linux/init.h
> index aedb254..51393f4 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -156,19 +156,20 @@ extern bool initcall_debug;
>  
>  #ifndef __ASSEMBLY__
>  
> -#ifdef CONFIG_LTO
> +#if 1
>  /* Work around a LTO gcc problem: when there is no reference to a variable
>   * in a module it will be moved to the end of the program. This causes
>   * reordering of initcalls which the kernel does not like.
>   * Add a dummy reference function to avoid this. The function is
>   * deleted by the linker.
>   */
> -#define LTO_REFERENCE_INITCALL(x) \
> -	; /* yes this is needed */			\
> -	static __used __exit void *reference_##x(void)	\
> -	{						\
> -		return &x;				\
> -	}
> +#define LTO_REFERENCE_INITCALL(sym) \
> +	extern typeof(sym) sym;					\
> +	/* extern const unsigned long __kentry_##sym; */		\
> +	static /* __visible */ const unsigned long __kentry_##sym		\
> +	__used							\
> +	__attribute__((section("___kentry" "+" #sym ",\"a\",@note #"), used)) \
> +	= (unsigned long)&sym;
>  #else
>  #define LTO_REFERENCE_INITCALL(x)
>  #endif
> @@ -222,16 +223,18 @@ extern bool initcall_debug;
>  
>  #define __initcall(fn) device_initcall(fn)
>  
> -#define __exitcall(fn) \
> -	static exitcall_t __exitcall_##fn __exit_call = fn
> +#define __exitcall(fn)						\
> +	static exitcall_t __exitcall_##fn __exit_call = fn;	\
>  
> -#define console_initcall(fn) \
> -	static initcall_t __initcall_##fn \
> -	__used __section(.con_initcall.init) = fn
> +#define console_initcall(fn)					\
> +	static initcall_t __initcall_##fn			\
> +	__used __section(.con_initcall.init) = fn;		\
> +	LTO_REFERENCE_INITCALL(__initcall_##fn)
>  
> -#define security_initcall(fn) \
> -	static initcall_t __initcall_##fn \
> -	__used __section(.security_initcall.init) = fn
> +#define security_initcall(fn)					\
> +	static initcall_t __initcall_##fn			\
> +	__used __section(.security_initcall.init) = fn;		\
> +	LTO_REFERENCE_INITCALL(__initcall_##fn)
>  
>  struct obs_kernel_param {
>  	const char *str;
> diff --git a/init/Makefile b/init/Makefile
> index 7bc47ee..c4fb455 100644
> --- a/init/Makefile
> +++ b/init/Makefile
> @@ -2,6 +2,8 @@
>  # Makefile for the linux kernel.
>  #
>  
> +ccflags-y := -fno-function-sections -fno-data-sections
> +
>  obj-y                          := main.o version.o mounts.o
>  ifneq ($(CONFIG_BLK_DEV_INITRD),y)
>  obj-y                          += noinitramfs.o
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index ef4658f..fb848af 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -37,17 +37,22 @@ info()
>  	fi
>  }
>  
> +# Grab all the EXPORT_SYMBOL symbols in the vmlinux build
> +# ${1} - output file
> +exports_extract()
> +{
> +	${NM} -g ${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN} |
> +		grep "R __ksymtab_" |
> +		sed 's/.*__ksymtab_\(.*\)$/\1/' > ${1}
> +}
> +
>  # Link of vmlinux.o used for section mismatch analysis
>  # ${1} output file
>  modpost_link()
>  {
>  	local objects
>  
> -	if [ -n "${CONFIG_THIN_ARCHIVES}" ]; then
> -		objects="--whole-archive ${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN} --no-whole-archive"
> -	else
> -		objects="${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group"
> -	fi
> +	objects="--whole-archive ${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN}"
>  	${LD} ${LDFLAGS} -r -o ${1} ${objects}
>  }
>  
> @@ -60,11 +65,7 @@ vmlinux_link()
>  	local objects
>  
>  	if [ "${SRCARCH}" != "um" ]; then
> -		if [ -n "${CONFIG_THIN_ARCHIVES}" ]; then
> -			objects="--whole-archive ${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN} --no-whole-archive"
> -		else
> -			objects="${KBUILD_VMLINUX_INIT} --start-group ${KBUILD_VMLINUX_MAIN} --end-group"
> -		fi
> +		objects="--whole-archive ${KBUILD_VMLINUX_INIT} ${KBUILD_VMLINUX_MAIN}"
>  		${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2}                  \
>  			-T ${lds} ${objects} ${1}
>  	else
>