All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Enable deadcode elimination at link time
@ 2018-11-06 13:39 ` Russell King - ARM Linux
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 13:39 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-efi, Ard Biesheuvel

This series enables elimination of dead code from the kernel at link
time.  The changes are mostly ARM specific, except for one change in
EFI as the EFI stub must not be compiled with -fdata-sections.

 arch/arm/Kconfig                       |  1 +
 arch/arm/boot/compressed/vmlinux.lds.S |  2 +-
 arch/arm/kernel/vmlinux.lds.S          | 10 +++++-----
 arch/arm/kernel/vmlinux.lds.h          | 18 +++++++++---------
 drivers/firmware/efi/libstub/Makefile  |  2 +-
 5 files changed, 17 insertions(+), 16 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 0/4] Enable deadcode elimination at link time
@ 2018-11-06 13:39 ` Russell King - ARM Linux
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 13:39 UTC (permalink / raw)
  To: linux-arm-kernel

This series enables elimination of dead code from the kernel at link
time.  The changes are mostly ARM specific, except for one change in
EFI as the EFI stub must not be compiled with -fdata-sections.

 arch/arm/Kconfig                       |  1 +
 arch/arm/boot/compressed/vmlinux.lds.S |  2 +-
 arch/arm/kernel/vmlinux.lds.S          | 10 +++++-----
 arch/arm/kernel/vmlinux.lds.h          | 18 +++++++++---------
 drivers/firmware/efi/libstub/Makefile  |  2 +-
 5 files changed, 17 insertions(+), 16 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/4] EFI stub: remove -fdata-sections
  2018-11-06 13:39 ` Russell King - ARM Linux
@ 2018-11-06 13:40   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2018-11-06 13:40 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-efi, Ard Biesheuvel

Remove -fdata-sections from the EFI stub build as this causes problems
for the ARM decompressor code.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/firmware/efi/libstub/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index c51627660dbb..98133d0577ab 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -15,7 +15,7 @@ cflags-$(CONFIG_X86)		+= -m$(BITS) -D__KERNEL__ -O2 \
 # disable the stackleak plugin
 cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
 				   $(DISABLE_STACKLEAK_PLUGIN)
-cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
+cflags-$(CONFIG_ARM)		:= $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \
 				   -fno-builtin -fpic \
 				   $(call cc-option,-mno-single-pic-base)
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 1/4] EFI stub: remove -fdata-sections
@ 2018-11-06 13:40   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2018-11-06 13:40 UTC (permalink / raw)
  To: linux-arm-kernel

Remove -fdata-sections from the EFI stub build as this causes problems
for the ARM decompressor code.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/firmware/efi/libstub/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index c51627660dbb..98133d0577ab 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -15,7 +15,7 @@ cflags-$(CONFIG_X86)		+= -m$(BITS) -D__KERNEL__ -O2 \
 # disable the stackleak plugin
 cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
 				   $(DISABLE_STACKLEAK_PLUGIN)
-cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
+cflags-$(CONFIG_ARM)		:= $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \
 				   -fno-builtin -fpic \
 				   $(call cc-option,-mno-single-pic-base)
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/4] ARM: mark critical data with KEEP()
  2018-11-06 13:39 ` Russell King - ARM Linux
  (?)
  (?)
@ 2018-11-06 13:40 ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2018-11-06 13:40 UTC (permalink / raw)
  To: linux-arm-kernel

In order for GC sections to work correctly, we need to instruct the
linker to emit all input sections for entry paths and some linker
built tables.

Ensure that the vectors, tag table, CPU table, machine platform table,
and exception tables are kept.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/kernel/vmlinux.lds.S | 10 +++++-----
 arch/arm/kernel/vmlinux.lds.h | 18 +++++++++---------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 23150c0f0f4d..3da8db076761 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -73,7 +73,7 @@ SECTIONS
 	. = ALIGN(4);
 	__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
 		__start___ex_table = .;
-		ARM_MMU_KEEP(*(__ex_table))
+		ARM_MMU_KEEP(KEEP(*(__ex_table)))
 		__stop___ex_table = .;
 	}
 
@@ -100,24 +100,24 @@ SECTIONS
 	}
 	.init.arch.info : {
 		__arch_info_begin = .;
-		*(.arch.info.init)
+		KEEP(*(.arch.info.init))
 		__arch_info_end = .;
 	}
 	.init.tagtable : {
 		__tagtable_begin = .;
-		*(.taglist.init)
+		KEEP(*(.taglist.init))
 		__tagtable_end = .;
 	}
 #ifdef CONFIG_SMP_ON_UP
 	.init.smpalt : {
 		__smpalt_begin = .;
-		*(.alt.smp.init)
+		KEEP(*(.alt.smp.init))
 		__smpalt_end = .;
 	}
 #endif
 	.init.pv_table : {
 		__pv_table_begin = .;
-		*(.pv_table)
+		KEEP(*(.pv_table))
 		__pv_table_end = .;
 	}
 
diff --git a/arch/arm/kernel/vmlinux.lds.h b/arch/arm/kernel/vmlinux.lds.h
index 8247bc15addc..a83d017f531c 100644
--- a/arch/arm/kernel/vmlinux.lds.h
+++ b/arch/arm/kernel/vmlinux.lds.h
@@ -28,22 +28,22 @@
 #define PROC_INFO							\
 		. = ALIGN(4);						\
 		__proc_info_begin = .;					\
-		*(.proc.info.init)					\
+		KEEP(*(.proc.info.init))				\
 		__proc_info_end = .;
 
 #define HYPERVISOR_TEXT							\
 		__hyp_text_start = .;					\
-		*(.hyp.text)						\
+		KEEP(*(.hyp.text))					\
 		__hyp_text_end = .;
 
 #define IDMAP_TEXT							\
 		ALIGN_FUNCTION();					\
 		__idmap_text_start = .;					\
-		*(.idmap.text)						\
+		KEEP(*(.idmap.text))					\
 		__idmap_text_end = .;					\
 		. = ALIGN(PAGE_SIZE);					\
 		__hyp_idmap_text_start = .;				\
-		*(.hyp.idmap.text)					\
+		KEEP(*(.hyp.idmap.text))				\
 		__hyp_idmap_text_end = .;
 
 #define ARM_DISCARD							\
@@ -64,7 +64,7 @@
 #define ARM_TEXT							\
 		IDMAP_TEXT						\
 		__entry_text_start = .;					\
-		*(.entry.text)						\
+		KEEP(*(.entry.text))					\
 		__entry_text_end = .;					\
 		IRQENTRY_TEXT						\
 		SOFTIRQENTRY_TEXT					\
@@ -86,12 +86,12 @@
 	. = ALIGN(8);							\
 	.ARM.unwind_idx : {						\
 		__start_unwind_idx = .;					\
-		*(.ARM.exidx*)						\
+		KEEP(*(.ARM.exidx*))					\
 		__stop_unwind_idx = .;					\
 	}								\
 	.ARM.unwind_tab : {						\
 		__start_unwind_tab = .;					\
-		*(.ARM.extab*)						\
+		KEEP(*(.ARM.extab*))					\
 		__stop_unwind_tab = .;					\
 	}
 
@@ -102,14 +102,14 @@
 #define ARM_VECTORS							\
 	__vectors_start = .;						\
 	.vectors 0xffff0000 : AT(__vectors_start) {			\
-		*(.vectors)						\
+		KEEP(*(.vectors))					\
 	}								\
 	. = __vectors_start + SIZEOF(.vectors);				\
 	__vectors_end = .;						\
 									\
 	__stubs_start = .;						\
 	.stubs ADDR(.vectors) + 0x1000 : AT(__stubs_start) {		\
-		*(.stubs)						\
+		KEEP(*(.stubs))						\
 	}								\
 	. = __stubs_start + SIZEOF(.stubs);				\
 	__stubs_end = .;						\
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 13:39 ` Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  (?)
@ 2018-11-06 13:40 ` Russell King
  2018-11-06 14:08   ` Ard Biesheuvel
  -1 siblings, 1 reply; 22+ messages in thread
From: Russell King @ 2018-11-06 13:40 UTC (permalink / raw)
  To: linux-arm-kernel

When building with -fdata-sections, the BSS data is placed into separate
sections, so we need to ask the linker to group these together into the
.bss output section, so that they're correctly placed.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index 2b963d8e76dd..3b91bc3c606f 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -118,7 +118,7 @@ SECTIONS
 
   . = BSS_START;
   __bss_start = .;
-  .bss			: { *(.bss) }
+  .bss			: { *(.bss) *(.bss.*) }
   _end = .;
 
   . = ALIGN(8);		/* the stack must be 64-bit aligned */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 4/4] ARM: enable LD_DEAD_CODE_DATA_ELIMINATION
  2018-11-06 13:39 ` Russell King - ARM Linux
                   ` (3 preceding siblings ...)
  (?)
@ 2018-11-06 13:40 ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2018-11-06 13:40 UTC (permalink / raw)
  To: linux-arm-kernel

Enable LD_DEAD_CODE_DATA_ELIMINATION, which allows functions that are
not called to be eliminated from the final kernel image.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 91be74d8df65..f7f65d0d5a05 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -82,6 +82,7 @@ config ARM
 	select HAVE_KERNEL_XZ
 	select HAVE_KPROBES if !XIP_KERNEL && !CPU_ENDIAN_BE32 && !CPU_V7M
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
+	select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !FUNCTION_TRACER
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI
 	select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 13:40 ` [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section Russell King
@ 2018-11-06 14:08   ` Ard Biesheuvel
  2018-11-06 14:10     ` Russell King - ARM Linux
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:08 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> When building with -fdata-sections, the BSS data is placed into separate
> sections, so we need to ask the linker to group these together into the
> .bss output section, so that they're correctly placed.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> index 2b963d8e76dd..3b91bc3c606f 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.S
> +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> @@ -118,7 +118,7 @@ SECTIONS
>
>    . = BSS_START;
>    __bss_start = .;
> -  .bss                 : { *(.bss) }
> +  .bss                 : { *(.bss) *(.bss.*) }

Would it make sense to sort these by alignment? Otherwise, I suspect
you may get a lot of padding holes due to the number of different
input sections, each with its own alignment.

>    _end = .;
>
>    . = ALIGN(8);                /* the stack must be 64-bit aligned */
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 14:08   ` Ard Biesheuvel
@ 2018-11-06 14:10     ` Russell King - ARM Linux
  2018-11-06 14:13       ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 14:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > When building with -fdata-sections, the BSS data is placed into separate
> > sections, so we need to ask the linker to group these together into the
> > .bss output section, so that they're correctly placed.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> > index 2b963d8e76dd..3b91bc3c606f 100644
> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> > @@ -118,7 +118,7 @@ SECTIONS
> >
> >    . = BSS_START;
> >    __bss_start = .;
> > -  .bss                 : { *(.bss) }
> > +  .bss                 : { *(.bss) *(.bss.*) }
> 
> Would it make sense to sort these by alignment? Otherwise, I suspect
> you may get a lot of padding holes due to the number of different
> input sections, each with its own alignment.

We don't bother elsewhere in the kernel linker script - do you have
a case where we get lots of padding?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] EFI stub: remove -fdata-sections
  2018-11-06 13:40   ` Russell King
@ 2018-11-06 14:11     ` Ard Biesheuvel
  -1 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:11 UTC (permalink / raw)
  To: Russell King; +Cc: linux-efi, linux-arm-kernel

On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Remove -fdata-sections from the EFI stub build as this causes problems
> for the ARM decompressor code.
>
Just out of curiosity: what kind of problems?

In any case: it doesn't make sense to obsess about .data size
optimizations here, so

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/firmware/efi/libstub/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index c51627660dbb..98133d0577ab 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -15,7 +15,7 @@ cflags-$(CONFIG_X86)          += -m$(BITS) -D__KERNEL__ -O2 \
>  # disable the stackleak plugin
>  cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
>                                    $(DISABLE_STACKLEAK_PLUGIN)
> -cflags-$(CONFIG_ARM)           := $(subst -pg,,$(KBUILD_CFLAGS)) \
> +cflags-$(CONFIG_ARM)           := $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \
>                                    -fno-builtin -fpic \
>                                    $(call cc-option,-mno-single-pic-base)
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/4] EFI stub: remove -fdata-sections
@ 2018-11-06 14:11     ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Remove -fdata-sections from the EFI stub build as this causes problems
> for the ARM decompressor code.
>
Just out of curiosity: what kind of problems?

In any case: it doesn't make sense to obsess about .data size
optimizations here, so

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/firmware/efi/libstub/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index c51627660dbb..98133d0577ab 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -15,7 +15,7 @@ cflags-$(CONFIG_X86)          += -m$(BITS) -D__KERNEL__ -O2 \
>  # disable the stackleak plugin
>  cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
>                                    $(DISABLE_STACKLEAK_PLUGIN)
> -cflags-$(CONFIG_ARM)           := $(subst -pg,,$(KBUILD_CFLAGS)) \
> +cflags-$(CONFIG_ARM)           := $(filter-out -pg -fdata-sections,$(KBUILD_CFLAGS)) \
>                                    -fno-builtin -fpic \
>                                    $(call cc-option,-mno-single-pic-base)
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 14:10     ` Russell King - ARM Linux
@ 2018-11-06 14:13       ` Ard Biesheuvel
  2018-11-06 14:29         ` Russell King - ARM Linux
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 15:10, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> > When building with -fdata-sections, the BSS data is placed into separate
>> > sections, so we need to ask the linker to group these together into the
>> > .bss output section, so that they're correctly placed.
>> >
>> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> > ---
>> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
>> > index 2b963d8e76dd..3b91bc3c606f 100644
>> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> > @@ -118,7 +118,7 @@ SECTIONS
>> >
>> >    . = BSS_START;
>> >    __bss_start = .;
>> > -  .bss                 : { *(.bss) }
>> > +  .bss                 : { *(.bss) *(.bss.*) }
>>
>> Would it make sense to sort these by alignment? Otherwise, I suspect
>> you may get a lot of padding holes due to the number of different
>> input sections, each with its own alignment.
>
> We don't bother elsewhere in the kernel linker script - do you have
> a case where we get lots of padding?
>

With -fdata-sections? How else are we ensuring that each .bss item in
its own section is not placed such that its alignment results in a
padding hole?

WIthout -fdata-sections, this is done per object, and GCC takes care
of this itself.

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] EFI stub: remove -fdata-sections
  2018-11-06 14:11     ` Ard Biesheuvel
@ 2018-11-06 14:21       ` Russell King - ARM Linux
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 14:21 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel

On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > Remove -fdata-sections from the EFI stub build as this causes problems
> > for the ARM decompressor code.
> >
> Just out of curiosity: what kind of problems?

I'm afraid I don't remember - these patches are March 2018.  I suspect
it was a result of a randconfig build failing to link, as I don't have
any 32-bit ARM configs that have the EFI stub enabled.

In any case, this will break:

#
# ARM discards the .data section because it disallows r/w data in the
# decompressor. So move our .data to .data.efistub, which is preserved
# explicitly by the decompressor linker script.
#
STUBCOPY_FLAGS-$(CONFIG_ARM)    += --rename-section .data=.data.efistub

because with -fdata-sections enabled, there is no longer a single .data
section, but multiple .data.* sections.  Hence, the rename no longer
works, and all the EFI stub data ends up being discarded by the
decompressor link stage.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/4] EFI stub: remove -fdata-sections
@ 2018-11-06 14:21       ` Russell King - ARM Linux
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 14:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > Remove -fdata-sections from the EFI stub build as this causes problems
> > for the ARM decompressor code.
> >
> Just out of curiosity: what kind of problems?

I'm afraid I don't remember - these patches are March 2018.  I suspect
it was a result of a randconfig build failing to link, as I don't have
any 32-bit ARM configs that have the EFI stub enabled.

In any case, this will break:

#
# ARM discards the .data section because it disallows r/w data in the
# decompressor. So move our .data to .data.efistub, which is preserved
# explicitly by the decompressor linker script.
#
STUBCOPY_FLAGS-$(CONFIG_ARM)    += --rename-section .data=.data.efistub

because with -fdata-sections enabled, there is no longer a single .data
section, but multiple .data.* sections.  Hence, the rename no longer
works, and all the EFI stub data ends up being discarded by the
decompressor link stage.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] EFI stub: remove -fdata-sections
  2018-11-06 14:21       ` Russell King - ARM Linux
@ 2018-11-06 14:22         ` Ard Biesheuvel
  -1 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:22 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: linux-efi, linux-arm-kernel

On 6 November 2018 at 15:21, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> > Remove -fdata-sections from the EFI stub build as this causes problems
>> > for the ARM decompressor code.
>> >
>> Just out of curiosity: what kind of problems?
>
> I'm afraid I don't remember - these patches are March 2018.  I suspect
> it was a result of a randconfig build failing to link, as I don't have
> any 32-bit ARM configs that have the EFI stub enabled.
>
> In any case, this will break:
>
> #
> # ARM discards the .data section because it disallows r/w data in the
> # decompressor. So move our .data to .data.efistub, which is preserved
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM)    += --rename-section .data=.data.efistub
>
> because with -fdata-sections enabled, there is no longer a single .data
> section, but multiple .data.* sections.  Hence, the rename no longer
> works, and all the EFI stub data ends up being discarded by the
> decompressor link stage.
>

Ah, of course, that must be it.

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/4] EFI stub: remove -fdata-sections
@ 2018-11-06 14:22         ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 15:21, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 03:11:18PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> > Remove -fdata-sections from the EFI stub build as this causes problems
>> > for the ARM decompressor code.
>> >
>> Just out of curiosity: what kind of problems?
>
> I'm afraid I don't remember - these patches are March 2018.  I suspect
> it was a result of a randconfig build failing to link, as I don't have
> any 32-bit ARM configs that have the EFI stub enabled.
>
> In any case, this will break:
>
> #
> # ARM discards the .data section because it disallows r/w data in the
> # decompressor. So move our .data to .data.efistub, which is preserved
> # explicitly by the decompressor linker script.
> #
> STUBCOPY_FLAGS-$(CONFIG_ARM)    += --rename-section .data=.data.efistub
>
> because with -fdata-sections enabled, there is no longer a single .data
> section, but multiple .data.* sections.  Hence, the rename no longer
> works, and all the EFI stub data ends up being discarded by the
> decompressor link stage.
>

Ah, of course, that must be it.

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 14:13       ` Ard Biesheuvel
@ 2018-11-06 14:29         ` Russell King - ARM Linux
  2018-11-06 14:39           ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 14:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 06, 2018 at 03:13:07PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 15:10, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
> >> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >> > When building with -fdata-sections, the BSS data is placed into separate
> >> > sections, so we need to ask the linker to group these together into the
> >> > .bss output section, so that they're correctly placed.
> >> >
> >> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >> > ---
> >> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> >> > index 2b963d8e76dd..3b91bc3c606f 100644
> >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
> >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> >> > @@ -118,7 +118,7 @@ SECTIONS
> >> >
> >> >    . = BSS_START;
> >> >    __bss_start = .;
> >> > -  .bss                 : { *(.bss) }
> >> > +  .bss                 : { *(.bss) *(.bss.*) }
> >>
> >> Would it make sense to sort these by alignment? Otherwise, I suspect
> >> you may get a lot of padding holes due to the number of different
> >> input sections, each with its own alignment.
> >
> > We don't bother elsewhere in the kernel linker script - do you have
> > a case where we get lots of padding?
> >
> 
> With -fdata-sections? How else are we ensuring that each .bss item in
> its own section is not placed such that its alignment results in a
> padding hole?

Quite simply, we don't care (at the moment).  The alignment does not
come from the size of each individual section, but the alignment
requirements of data within the section.

See include/asm-generic/vmlinux.lds.h and the definition of *_MAIN
symbols from around line 60.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 14:29         ` Russell King - ARM Linux
@ 2018-11-06 14:39           ` Ard Biesheuvel
  2018-11-06 14:45             ` Russell King - ARM Linux
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 15:29, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 03:13:07PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 15:10, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
>> >> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> >> > When building with -fdata-sections, the BSS data is placed into separate
>> >> > sections, so we need to ask the linker to group these together into the
>> >> > .bss output section, so that they're correctly placed.
>> >> >
>> >> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> >> > ---
>> >> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
>> >> > index 2b963d8e76dd..3b91bc3c606f 100644
>> >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> >> > @@ -118,7 +118,7 @@ SECTIONS
>> >> >
>> >> >    . = BSS_START;
>> >> >    __bss_start = .;
>> >> > -  .bss                 : { *(.bss) }
>> >> > +  .bss                 : { *(.bss) *(.bss.*) }
>> >>
>> >> Would it make sense to sort these by alignment? Otherwise, I suspect
>> >> you may get a lot of padding holes due to the number of different
>> >> input sections, each with its own alignment.
>> >
>> > We don't bother elsewhere in the kernel linker script - do you have
>> > a case where we get lots of padding?
>> >
>>
>> With -fdata-sections? How else are we ensuring that each .bss item in
>> its own section is not placed such that its alignment results in a
>> padding hole?
>
> Quite simply, we don't care (at the moment).  The alignment does not
> come from the size of each individual section, but the alignment
> requirements of data within the section.
>
> See include/asm-generic/vmlinux.lds.h and the definition of *_MAIN
> symbols from around line 60.
>

This seems like an oversight to me: consider something like*

unsigned char bar = 1;
unsigned long long foo = 4;

build it with

arm-linux-gnueabihf-gcc -fdata-sections -o /tmp/foo.o -c /tmp/foo.c

and we end up with the following object file

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
  [ 2] .data             PROGBITS        00000000 000034 000000 00  WA  0   0  1
  [ 3] .bss              NOBITS          00000000 000034 000000 00  WA  0   0  1
  [ 4] .data.bar         PROGBITS        00000000 000034 000001 00  WA  0   0  1
  [ 5] .data.foo         PROGBITS        00000000 000038 000008 00  WA  0   0  8
  ...

Symbol table '.symtab' contains 13 entries:
   Num:    Value  Size Type    Bind   Vis      Ndx Name
    11: 00000000     8 OBJECT  GLOBAL DEFAULT    5 foo
    12: 00000000     1 OBJECT  GLOBAL DEFAULT    4 bar

whereas if I remove the -fdata-sections argument, I get

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
  [ 2] .data             PROGBITS        00000000 000038 000009 00  WA  0   0  8
  [ 3] .bss              NOBITS          00000000 000041 000000 00  WA  0   0  1
  ...

and

Symbol table '.symtab' contains 11 entries:
   Num:    Value  Size Type    Bind   Vis      Ndx Name
   ...
     9: 00000000     8 OBJECT  GLOBAL DEFAULT    2 foo
    10: 00000008     1 OBJECT  GLOBAL DEFAULT    2 bar

In other words, we end up with a 7 byte padding hole by switching to
-fdata-sections, unless we sort the input objects by alignment.

* for my test compiile, -fdata-sections would not produce different
input section so I am using .data for this example.

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 14:39           ` Ard Biesheuvel
@ 2018-11-06 14:45             ` Russell King - ARM Linux
  2018-11-06 15:18               ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 14:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 06, 2018 at 03:39:58PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 15:29, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Tue, Nov 06, 2018 at 03:13:07PM +0100, Ard Biesheuvel wrote:
> >> On 6 November 2018 at 15:10, Russell King - ARM Linux
> >> <linux@armlinux.org.uk> wrote:
> >> > On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
> >> >> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >> >> > When building with -fdata-sections, the BSS data is placed into separate
> >> >> > sections, so we need to ask the linker to group these together into the
> >> >> > .bss output section, so that they're correctly placed.
> >> >> >
> >> >> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >> >> > ---
> >> >> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> >> >> > index 2b963d8e76dd..3b91bc3c606f 100644
> >> >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
> >> >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> >> >> > @@ -118,7 +118,7 @@ SECTIONS
> >> >> >
> >> >> >    . = BSS_START;
> >> >> >    __bss_start = .;
> >> >> > -  .bss                 : { *(.bss) }
> >> >> > +  .bss                 : { *(.bss) *(.bss.*) }
> >> >>
> >> >> Would it make sense to sort these by alignment? Otherwise, I suspect
> >> >> you may get a lot of padding holes due to the number of different
> >> >> input sections, each with its own alignment.
> >> >
> >> > We don't bother elsewhere in the kernel linker script - do you have
> >> > a case where we get lots of padding?
> >> >
> >>
> >> With -fdata-sections? How else are we ensuring that each .bss item in
> >> its own section is not placed such that its alignment results in a
> >> padding hole?
> >
> > Quite simply, we don't care (at the moment).  The alignment does not
> > come from the size of each individual section, but the alignment
> > requirements of data within the section.
> >
> > See include/asm-generic/vmlinux.lds.h and the definition of *_MAIN
> > symbols from around line 60.
> >
> 
> This seems like an oversight to me: consider something like*
> 
> unsigned char bar = 1;
> unsigned long long foo = 4;
> 
> build it with
> 
> arm-linux-gnueabihf-gcc -fdata-sections -o /tmp/foo.o -c /tmp/foo.c
> 
> and we end up with the following object file
> 
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
>   [ 2] .data             PROGBITS        00000000 000034 000000 00  WA  0   0  1
>   [ 3] .bss              NOBITS          00000000 000034 000000 00  WA  0   0  1
>   [ 4] .data.bar         PROGBITS        00000000 000034 000001 00  WA  0   0  1
>   [ 5] .data.foo         PROGBITS        00000000 000038 000008 00  WA  0   0  8
>   ...
> 
> Symbol table '.symtab' contains 13 entries:
>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>     11: 00000000     8 OBJECT  GLOBAL DEFAULT    5 foo
>     12: 00000000     1 OBJECT  GLOBAL DEFAULT    4 bar
> 
> whereas if I remove the -fdata-sections argument, I get
> 
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
>   [ 2] .data             PROGBITS        00000000 000038 000009 00  WA  0   0  8
>   [ 3] .bss              NOBITS          00000000 000041 000000 00  WA  0   0  1
>   ...
> 
> and
> 
> Symbol table '.symtab' contains 11 entries:
>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>    ...
>      9: 00000000     8 OBJECT  GLOBAL DEFAULT    2 foo
>     10: 00000008     1 OBJECT  GLOBAL DEFAULT    2 bar
> 
> In other words, we end up with a 7 byte padding hole by switching to
> -fdata-sections, unless we sort the input objects by alignment.
> 
> * for my test compiile, -fdata-sections would not produce different
> input section so I am using .data for this example.

Nevertheless, changing this is not a subject for a patch series that
targets ARM.  What you've identified is a deficiency in the generic
cross-arch support for this which needs a wider audience to be
involved.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 14:45             ` Russell King - ARM Linux
@ 2018-11-06 15:18               ` Ard Biesheuvel
  2018-11-06 16:06                 ` Russell King - ARM Linux
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 15:45, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 03:39:58PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 15:29, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > On Tue, Nov 06, 2018 at 03:13:07PM +0100, Ard Biesheuvel wrote:
>> >> On 6 November 2018 at 15:10, Russell King - ARM Linux
>> >> <linux@armlinux.org.uk> wrote:
>> >> > On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
>> >> >> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> >> >> > When building with -fdata-sections, the BSS data is placed into separate
>> >> >> > sections, so we need to ask the linker to group these together into the
>> >> >> > .bss output section, so that they're correctly placed.
>> >> >> >
>> >> >> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> >> >> > ---
>> >> >> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
>> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> >
>> >> >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
>> >> >> > index 2b963d8e76dd..3b91bc3c606f 100644
>> >> >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> >> >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> >> >> > @@ -118,7 +118,7 @@ SECTIONS
>> >> >> >
>> >> >> >    . = BSS_START;
>> >> >> >    __bss_start = .;
>> >> >> > -  .bss                 : { *(.bss) }
>> >> >> > +  .bss                 : { *(.bss) *(.bss.*) }
>> >> >>
>> >> >> Would it make sense to sort these by alignment? Otherwise, I suspect
>> >> >> you may get a lot of padding holes due to the number of different
>> >> >> input sections, each with its own alignment.
>> >> >
>> >> > We don't bother elsewhere in the kernel linker script - do you have
>> >> > a case where we get lots of padding?
>> >> >
>> >>
>> >> With -fdata-sections? How else are we ensuring that each .bss item in
>> >> its own section is not placed such that its alignment results in a
>> >> padding hole?
>> >
>> > Quite simply, we don't care (at the moment).  The alignment does not
>> > come from the size of each individual section, but the alignment
>> > requirements of data within the section.
>> >
>> > See include/asm-generic/vmlinux.lds.h and the definition of *_MAIN
>> > symbols from around line 60.
>> >
>>
>> This seems like an oversight to me: consider something like*
>>
>> unsigned char bar = 1;
>> unsigned long long foo = 4;
>>
>> build it with
>>
>> arm-linux-gnueabihf-gcc -fdata-sections -o /tmp/foo.o -c /tmp/foo.c
>>
>> and we end up with the following object file
>>
>> Section Headers:
>>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
>>   [ 2] .data             PROGBITS        00000000 000034 000000 00  WA  0   0  1
>>   [ 3] .bss              NOBITS          00000000 000034 000000 00  WA  0   0  1
>>   [ 4] .data.bar         PROGBITS        00000000 000034 000001 00  WA  0   0  1
>>   [ 5] .data.foo         PROGBITS        00000000 000038 000008 00  WA  0   0  8
>>   ...
>>
>> Symbol table '.symtab' contains 13 entries:
>>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>>     11: 00000000     8 OBJECT  GLOBAL DEFAULT    5 foo
>>     12: 00000000     1 OBJECT  GLOBAL DEFAULT    4 bar
>>
>> whereas if I remove the -fdata-sections argument, I get
>>
>> Section Headers:
>>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
>>   [ 2] .data             PROGBITS        00000000 000038 000009 00  WA  0   0  8
>>   [ 3] .bss              NOBITS          00000000 000041 000000 00  WA  0   0  1
>>   ...
>>
>> and
>>
>> Symbol table '.symtab' contains 11 entries:
>>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>>    ...
>>      9: 00000000     8 OBJECT  GLOBAL DEFAULT    2 foo
>>     10: 00000008     1 OBJECT  GLOBAL DEFAULT    2 bar
>>
>> In other words, we end up with a 7 byte padding hole by switching to
>> -fdata-sections, unless we sort the input objects by alignment.
>>
>> * for my test compiile, -fdata-sections would not produce different
>> input section so I am using .data for this example.
>
> Nevertheless, changing this is not a subject for a patch series that
> targets ARM.  What you've identified is a deficiency in the generic
> cross-arch support for this which needs a wider audience to be
> involved.
>

Fair enough.

For the record, when I build the arm64 defconfig kernel with
LD_DEAD_CODE_ELIMINATION turned on, adding --sort-section=alignment
reduces the image size from

   text    data     bss     dec     hex filename
11951684 6936332 385040 19273056 1261560 vmlinux

to

   text    data     bss     dec     hex filename
11938452 6930404 383625 19252481 125c501 vmlinux

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 15:18               ` Ard Biesheuvel
@ 2018-11-06 16:06                 ` Russell King - ARM Linux
  2018-11-06 16:14                   ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Russell King - ARM Linux @ 2018-11-06 16:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 06, 2018 at 04:18:46PM +0100, Ard Biesheuvel wrote:
> On 6 November 2018 at 15:45, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Tue, Nov 06, 2018 at 03:39:58PM +0100, Ard Biesheuvel wrote:
> >> On 6 November 2018 at 15:29, Russell King - ARM Linux
> >> <linux@armlinux.org.uk> wrote:
> >> > On Tue, Nov 06, 2018 at 03:13:07PM +0100, Ard Biesheuvel wrote:
> >> >> On 6 November 2018 at 15:10, Russell King - ARM Linux
> >> >> <linux@armlinux.org.uk> wrote:
> >> >> > On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
> >> >> >> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >> >> >> > When building with -fdata-sections, the BSS data is placed into separate
> >> >> >> > sections, so we need to ask the linker to group these together into the
> >> >> >> > .bss output section, so that they're correctly placed.
> >> >> >> >
> >> >> >> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >> >> >> > ---
> >> >> >> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
> >> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >> >
> >> >> >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> >> >> >> > index 2b963d8e76dd..3b91bc3c606f 100644
> >> >> >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
> >> >> >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
> >> >> >> > @@ -118,7 +118,7 @@ SECTIONS
> >> >> >> >
> >> >> >> >    . = BSS_START;
> >> >> >> >    __bss_start = .;
> >> >> >> > -  .bss                 : { *(.bss) }
> >> >> >> > +  .bss                 : { *(.bss) *(.bss.*) }
> >> >> >>
> >> >> >> Would it make sense to sort these by alignment? Otherwise, I suspect
> >> >> >> you may get a lot of padding holes due to the number of different
> >> >> >> input sections, each with its own alignment.
> >> >> >
> >> >> > We don't bother elsewhere in the kernel linker script - do you have
> >> >> > a case where we get lots of padding?
> >> >> >
> >> >>
> >> >> With -fdata-sections? How else are we ensuring that each .bss item in
> >> >> its own section is not placed such that its alignment results in a
> >> >> padding hole?
> >> >
> >> > Quite simply, we don't care (at the moment).  The alignment does not
> >> > come from the size of each individual section, but the alignment
> >> > requirements of data within the section.
> >> >
> >> > See include/asm-generic/vmlinux.lds.h and the definition of *_MAIN
> >> > symbols from around line 60.
> >> >
> >>
> >> This seems like an oversight to me: consider something like*
> >>
> >> unsigned char bar = 1;
> >> unsigned long long foo = 4;
> >>
> >> build it with
> >>
> >> arm-linux-gnueabihf-gcc -fdata-sections -o /tmp/foo.o -c /tmp/foo.c
> >>
> >> and we end up with the following object file
> >>
> >> Section Headers:
> >>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> >>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
> >>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
> >>   [ 2] .data             PROGBITS        00000000 000034 000000 00  WA  0   0  1
> >>   [ 3] .bss              NOBITS          00000000 000034 000000 00  WA  0   0  1
> >>   [ 4] .data.bar         PROGBITS        00000000 000034 000001 00  WA  0   0  1
> >>   [ 5] .data.foo         PROGBITS        00000000 000038 000008 00  WA  0   0  8
> >>   ...
> >>
> >> Symbol table '.symtab' contains 13 entries:
> >>    Num:    Value  Size Type    Bind   Vis      Ndx Name
> >>     11: 00000000     8 OBJECT  GLOBAL DEFAULT    5 foo
> >>     12: 00000000     1 OBJECT  GLOBAL DEFAULT    4 bar
> >>
> >> whereas if I remove the -fdata-sections argument, I get
> >>
> >> Section Headers:
> >>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> >>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
> >>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
> >>   [ 2] .data             PROGBITS        00000000 000038 000009 00  WA  0   0  8
> >>   [ 3] .bss              NOBITS          00000000 000041 000000 00  WA  0   0  1
> >>   ...
> >>
> >> and
> >>
> >> Symbol table '.symtab' contains 11 entries:
> >>    Num:    Value  Size Type    Bind   Vis      Ndx Name
> >>    ...
> >>      9: 00000000     8 OBJECT  GLOBAL DEFAULT    2 foo
> >>     10: 00000008     1 OBJECT  GLOBAL DEFAULT    2 bar
> >>
> >> In other words, we end up with a 7 byte padding hole by switching to
> >> -fdata-sections, unless we sort the input objects by alignment.
> >>
> >> * for my test compiile, -fdata-sections would not produce different
> >> input section so I am using .data for this example.
> >
> > Nevertheless, changing this is not a subject for a patch series that
> > targets ARM.  What you've identified is a deficiency in the generic
> > cross-arch support for this which needs a wider audience to be
> > involved.
> >
> 
> Fair enough.
> 
> For the record, when I build the arm64 defconfig kernel with
> LD_DEAD_CODE_ELIMINATION turned on, adding --sort-section=alignment
> reduces the image size from
> 
>    text    data     bss     dec     hex filename
> 11951684 6936332 385040 19273056 1261560 vmlinux
> 
> to
> 
>    text    data     bss     dec     hex filename
> 11938452 6930404 383625 19252481 125c501 vmlinux

I'm not sure that's relevant when this thread is about 32bit ARM.
It's a little like building an x86 kernel and posting the results.
Two entirely different architectures with different results.

Here's the numbers for an imx6 ARM kernel:

   text    data     bss      dec     hex filename
8438980 3255060 9640528 21334568 1458a28 imx6-unpatched/vmlinux
6164770      84    4120  6168974  5e218e imx6-unpatched/arch/arm/boot/compressed/vmlinux
8438980 3255060 9640528 21334568 1458a28 imx6-dc-dis/vmlinux
6164770      84    4120  6168974  5e218e imx6-dc-dis/arch/arm/boot/compressed/vmlinux
8448960 3253388 9639992 21342340 145a884 imx6-dc-ena/vmlinux
6175233      84    4120  6179437  5e4a6d imx6-dc-ena/arch/arm/boot/compressed/vmlinux
8446700 3245700 9639212 21331612 1457e9c imx6-dc-sort/vmlinux
6172284      84    4120  6176488  5e3ee8 imx6-dc-sort/arch/arm/boot/compressed/vmlinux

The interesting thing here is that while the data sizes are reduced
by enabling this option, but the text size actually _increases_ by
more than we save.  So the result is an overall bigger kernel.

Presumably this is because of the literal pools needing to be larger
in the .text segments, and more instructions necessary to access the
data as the compiler can no longer know the relative displacement
between neighbouring data items in the object file.

So, it seems dead-code elimination is not that useful on 32-bit ARM.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section
  2018-11-06 16:06                 ` Russell King - ARM Linux
@ 2018-11-06 16:14                   ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2018-11-06 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 6 November 2018 at 17:06, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 06, 2018 at 04:18:46PM +0100, Ard Biesheuvel wrote:
>> On 6 November 2018 at 15:45, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > On Tue, Nov 06, 2018 at 03:39:58PM +0100, Ard Biesheuvel wrote:
>> >> On 6 November 2018 at 15:29, Russell King - ARM Linux
>> >> <linux@armlinux.org.uk> wrote:
>> >> > On Tue, Nov 06, 2018 at 03:13:07PM +0100, Ard Biesheuvel wrote:
>> >> >> On 6 November 2018 at 15:10, Russell King - ARM Linux
>> >> >> <linux@armlinux.org.uk> wrote:
>> >> >> > On Tue, Nov 06, 2018 at 03:08:06PM +0100, Ard Biesheuvel wrote:
>> >> >> >> On 6 November 2018 at 14:40, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> >> >> >> > When building with -fdata-sections, the BSS data is placed into separate
>> >> >> >> > sections, so we need to ask the linker to group these together into the
>> >> >> >> > .bss output section, so that they're correctly placed.
>> >> >> >> >
>> >> >> >> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> >> >> >> > ---
>> >> >> >> >  arch/arm/boot/compressed/vmlinux.lds.S | 2 +-
>> >> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> >> >
>> >> >> >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
>> >> >> >> > index 2b963d8e76dd..3b91bc3c606f 100644
>> >> >> >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S
>> >> >> >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S
>> >> >> >> > @@ -118,7 +118,7 @@ SECTIONS
>> >> >> >> >
>> >> >> >> >    . = BSS_START;
>> >> >> >> >    __bss_start = .;
>> >> >> >> > -  .bss                 : { *(.bss) }
>> >> >> >> > +  .bss                 : { *(.bss) *(.bss.*) }
>> >> >> >>
>> >> >> >> Would it make sense to sort these by alignment? Otherwise, I suspect
>> >> >> >> you may get a lot of padding holes due to the number of different
>> >> >> >> input sections, each with its own alignment.
>> >> >> >
>> >> >> > We don't bother elsewhere in the kernel linker script - do you have
>> >> >> > a case where we get lots of padding?
>> >> >> >
>> >> >>
>> >> >> With -fdata-sections? How else are we ensuring that each .bss item in
>> >> >> its own section is not placed such that its alignment results in a
>> >> >> padding hole?
>> >> >
>> >> > Quite simply, we don't care (at the moment).  The alignment does not
>> >> > come from the size of each individual section, but the alignment
>> >> > requirements of data within the section.
>> >> >
>> >> > See include/asm-generic/vmlinux.lds.h and the definition of *_MAIN
>> >> > symbols from around line 60.
>> >> >
>> >>
>> >> This seems like an oversight to me: consider something like*
>> >>
>> >> unsigned char bar = 1;
>> >> unsigned long long foo = 4;
>> >>
>> >> build it with
>> >>
>> >> arm-linux-gnueabihf-gcc -fdata-sections -o /tmp/foo.o -c /tmp/foo.c
>> >>
>> >> and we end up with the following object file
>> >>
>> >> Section Headers:
>> >>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>> >>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>> >>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
>> >>   [ 2] .data             PROGBITS        00000000 000034 000000 00  WA  0   0  1
>> >>   [ 3] .bss              NOBITS          00000000 000034 000000 00  WA  0   0  1
>> >>   [ 4] .data.bar         PROGBITS        00000000 000034 000001 00  WA  0   0  1
>> >>   [ 5] .data.foo         PROGBITS        00000000 000038 000008 00  WA  0   0  8
>> >>   ...
>> >>
>> >> Symbol table '.symtab' contains 13 entries:
>> >>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>> >>     11: 00000000     8 OBJECT  GLOBAL DEFAULT    5 foo
>> >>     12: 00000000     1 OBJECT  GLOBAL DEFAULT    4 bar
>> >>
>> >> whereas if I remove the -fdata-sections argument, I get
>> >>
>> >> Section Headers:
>> >>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>> >>   [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>> >>   [ 1] .text             PROGBITS        00000000 000034 000000 00  AX  0   0  1
>> >>   [ 2] .data             PROGBITS        00000000 000038 000009 00  WA  0   0  8
>> >>   [ 3] .bss              NOBITS          00000000 000041 000000 00  WA  0   0  1
>> >>   ...
>> >>
>> >> and
>> >>
>> >> Symbol table '.symtab' contains 11 entries:
>> >>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>> >>    ...
>> >>      9: 00000000     8 OBJECT  GLOBAL DEFAULT    2 foo
>> >>     10: 00000008     1 OBJECT  GLOBAL DEFAULT    2 bar
>> >>
>> >> In other words, we end up with a 7 byte padding hole by switching to
>> >> -fdata-sections, unless we sort the input objects by alignment.
>> >>
>> >> * for my test compiile, -fdata-sections would not produce different
>> >> input section so I am using .data for this example.
>> >
>> > Nevertheless, changing this is not a subject for a patch series that
>> > targets ARM.  What you've identified is a deficiency in the generic
>> > cross-arch support for this which needs a wider audience to be
>> > involved.
>> >
>>
>> Fair enough.
>>
>> For the record, when I build the arm64 defconfig kernel with
>> LD_DEAD_CODE_ELIMINATION turned on, adding --sort-section=alignment
>> reduces the image size from
>>
>>    text    data     bss     dec     hex filename
>> 11951684 6936332 385040 19273056 1261560 vmlinux
>>
>> to
>>
>>    text    data     bss     dec     hex filename
>> 11938452 6930404 383625 19252481 125c501 vmlinux
>
> I'm not sure that's relevant when this thread is about 32bit ARM.
> It's a little like building an x86 kernel and posting the results.
> Two entirely different architectures with different results.
>

You said it was a generic issue not an ARM issue, to which I replied
'fair enough', after which I shared some information that is relevant
to the generic issue under discussion.

> Here's the numbers for an imx6 ARM kernel:
>
>    text    data     bss      dec     hex filename
> 8438980 3255060 9640528 21334568 1458a28 imx6-unpatched/vmlinux
> 6164770      84    4120  6168974  5e218e imx6-unpatched/arch/arm/boot/compressed/vmlinux
> 8438980 3255060 9640528 21334568 1458a28 imx6-dc-dis/vmlinux
> 6164770      84    4120  6168974  5e218e imx6-dc-dis/arch/arm/boot/compressed/vmlinux
> 8448960 3253388 9639992 21342340 145a884 imx6-dc-ena/vmlinux
> 6175233      84    4120  6179437  5e4a6d imx6-dc-ena/arch/arm/boot/compressed/vmlinux
> 8446700 3245700 9639212 21331612 1457e9c imx6-dc-sort/vmlinux
> 6172284      84    4120  6176488  5e3ee8 imx6-dc-sort/arch/arm/boot/compressed/vmlinux
>
> The interesting thing here is that while the data sizes are reduced
> by enabling this option, but the text size actually _increases_ by
> more than we save.  So the result is an overall bigger kernel.
>
> Presumably this is because of the literal pools needing to be larger
> in the .text segments, and more instructions necessary to access the
> data as the compiler can no longer know the relative displacement
> between neighbouring data items in the object file.
>

I guess the compiler can no longer share literal pools between
functions, so they get duplicated into each one.

> So, it seems dead-code elimination is not that useful on 32-bit ARM.
>

Agreed.

^ permalink raw reply	[flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-11-06 16:14 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-06 13:39 [PATCH 0/4] Enable deadcode elimination at link time Russell King - ARM Linux
2018-11-06 13:39 ` Russell King - ARM Linux
2018-11-06 13:40 ` [PATCH 1/4] EFI stub: remove -fdata-sections Russell King
2018-11-06 13:40   ` Russell King
2018-11-06 14:11   ` Ard Biesheuvel
2018-11-06 14:11     ` Ard Biesheuvel
2018-11-06 14:21     ` Russell King - ARM Linux
2018-11-06 14:21       ` Russell King - ARM Linux
2018-11-06 14:22       ` Ard Biesheuvel
2018-11-06 14:22         ` Ard Biesheuvel
2018-11-06 13:40 ` [PATCH 2/4] ARM: mark critical data with KEEP() Russell King
2018-11-06 13:40 ` [PATCH 3/4] ARM: merge -fdata-sections BSS data to .bss section Russell King
2018-11-06 14:08   ` Ard Biesheuvel
2018-11-06 14:10     ` Russell King - ARM Linux
2018-11-06 14:13       ` Ard Biesheuvel
2018-11-06 14:29         ` Russell King - ARM Linux
2018-11-06 14:39           ` Ard Biesheuvel
2018-11-06 14:45             ` Russell King - ARM Linux
2018-11-06 15:18               ` Ard Biesheuvel
2018-11-06 16:06                 ` Russell King - ARM Linux
2018-11-06 16:14                   ` Ard Biesheuvel
2018-11-06 13:40 ` [PATCH 4/4] ARM: enable LD_DEAD_CODE_DATA_ELIMINATION Russell King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.