| Message ID | 20170726114139.GN31807@n2100.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 26 Jul 2017 12:41:39 +0100 Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Wed, Jul 26, 2017 at 01:23:15PM +0200, Robert Jarzmik wrote: > > Robert Jarzmik <robert.jarzmik@free.fr> writes: > > > Sure, it like this : > > > c06f22c8 D user_pmd_table > > > c06f22cc d __warned.19178 > > > c06f22cd d clean_addr > > > > > > And I have no idea how to link that __warned.19178 with the WARN_xxx() statement ... > > > > > > This bisection I made points me to : > > > # first bad commit: [799c43415442414b1032580c47684cb709dfed6d] kbuild: thin archives make default for all archs > > > > > > Reverting this commit makes my kernel boot again. > > > > Hi Russell, > > > > I think I know where this is comming from. > > - in file: ~/mio_linux/kernel/include/asm-generic/vmlinux.lds.h:206 > > - the definition of DATA_DATA looks like : > > (*.data .data.[0-9a-zA-Z_]*) > > ... > > *(.data.unlikely) > > ... > > > > This doesn't look good to me, as .data.unlikely fullfills the first wildcard, > > and the linker is allowed to mix .data and .data.unlikely symbols to its > > pleasure, while .data.unlikely symbols (at least in my case) are not 4 bytes > > multiples. > > Obviously, the mixing of .data.unlikely with the normal .data sections > is undesirable - that looks like it's come from commit b67067f1176d > ("kbuild: allow archs to select link dead code/data elimination") dated > 2016, which according to git describe --contains was merged in v4.9-rc1. > So it's unlikely to be directly caused by this, although this commit is > implicated. > > I wonder if Nick realised this side effect (that .data.unlikely is no > longer grouped together) - maybe we need to rename .data.unlikely? > (Adding Nick and Michal). I did to some degree -- hence TEXT was not done that way -- but obviously did not look closely enough. The right short term fix I think is to make those wildcards depend only on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION. One way to avoid clashes with compiler generated section names is to use ".." as separators in our section names. Anyway I'll code up a fix for the linker script that we can get into 4.12, thanks for pointing it out. Thanks, Nick
Nicholas Piggin <npiggin@gmail.com> writes: > On Wed, 26 Jul 2017 12:41:39 +0100 > Russell King - ARM Linux <linux@armlinux.org.uk> wrote: >> Obviously, the mixing of .data.unlikely with the normal .data sections >> is undesirable - that looks like it's come from commit b67067f1176d >> ("kbuild: allow archs to select link dead code/data elimination") dated >> 2016, which according to git describe --contains was merged in v4.9-rc1. >> So it's unlikely to be directly caused by this, although this commit is >> implicated. >> >> I wonder if Nick realised this side effect (that .data.unlikely is no >> longer grouped together) - maybe we need to rename .data.unlikely? >> (Adding Nick and Michal). > > I did to some degree -- hence TEXT was not done that way -- but obviously > did not look closely enough. > > The right short term fix I think is to make those wildcards depend only on > CONFIG_LD_DEAD_CODE_DATA_ELIMINATION. > > One way to avoid clashes with compiler generated section names is to use > ".." as separators in our section names. > > Anyway I'll code up a fix for the linker script that we can get into 4.12, > thanks for pointing it out. Ok, cool. I'd like to be copied to your patch so that I can test it please. For what is worth, your patch, Russell, fixes the issue as well, I just tested it. Cheers.
On Wed, 26 Jul 2017 23:02:46 +0200 Robert Jarzmik <robert.jarzmik@free.fr> wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > > > On Wed, 26 Jul 2017 12:41:39 +0100 > > Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > >> Obviously, the mixing of .data.unlikely with the normal .data sections > >> is undesirable - that looks like it's come from commit b67067f1176d > >> ("kbuild: allow archs to select link dead code/data elimination") dated > >> 2016, which according to git describe --contains was merged in v4.9-rc1. > >> So it's unlikely to be directly caused by this, although this commit is > >> implicated. > >> > >> I wonder if Nick realised this side effect (that .data.unlikely is no > >> longer grouped together) - maybe we need to rename .data.unlikely? > >> (Adding Nick and Michal). > > > > I did to some degree -- hence TEXT was not done that way -- but obviously > > did not look closely enough. > > > > The right short term fix I think is to make those wildcards depend only on > > CONFIG_LD_DEAD_CODE_DATA_ELIMINATION. > > > > One way to avoid clashes with compiler generated section names is to use > > ".." as separators in our section names. > > > > Anyway I'll code up a fix for the linker script that we can get into 4.12, > > thanks for pointing it out. > Ok, cool. I'd like to be copied to your patch so that I can test it please. Hmm, I intended to cc you, but somehow failed that too. No wonder my code has bugs :\ It's gone to linux-kbuild and linux-arm-kernel http://marc.info/?l=linux-kbuild&m=150107320226315&q=raw Or msg me privately I'll resend it to you. Thanks, Nick
Nicholas Piggin <npiggin@gmail.com> writes: > Hmm, I intended to cc you, but somehow failed that too. No wonder my > code has bugs :\ > > It's gone to linux-kbuild and linux-arm-kernel > > http://marc.info/?l=linux-kbuild&m=150107320226315&q=raw I verified your patch, it works. Could you please add : Reported-by: Robert Jarzmik <robert.jarzmik@free.fr> ... Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers.
On Wed, Jul 26, 2017 at 11:02:46PM +0200, Robert Jarzmik wrote: > For what is worth, your patch, Russell, fixes the issue as well, I just > tested it. Shall I take that as permission to add a Tested-by line to the patch?
Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Wed, Jul 26, 2017 at 11:02:46PM +0200, Robert Jarzmik wrote: >> For what is worth, your patch, Russell, fixes the issue as well, I just >> tested it. > > Shall I take that as permission to add a Tested-by line to the patch? Most certainly. Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers.
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S index f2d2f9398adb..10ad44f3886a 100644 --- a/arch/arm/kernel/entry-armv.S +++ b/arch/arm/kernel/entry-armv.S @@ -721,6 +721,7 @@ ENDPROC(__und_usr) */ .pushsection .data + .align 2 ENTRY(fp_enter) .word no_fp .popsection @@ -1026,6 +1027,7 @@ ENDPROC(vector_\name) W(b) vector_fiq .data + .align 2 .globl cr_alignment cr_alignment: diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S index 04286fd9e09c..6b1148cafffd 100644 --- a/arch/arm/kernel/head.S +++ b/arch/arm/kernel/head.S @@ -556,6 +556,7 @@ ENDPROC(__fixup_smp) .word __smpalt_end .pushsection .data + .align 2 .globl smp_on_up smp_on_up: ALT_SMP(.long 1) @@ -716,6 +717,7 @@ ENTRY(fixup_pv_table) ENDPROC(fixup_pv_table) .data + .align 2 .globl __pv_phys_pfn_offset .type __pv_phys_pfn_offset, %object __pv_phys_pfn_offset: diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S index ec7e7377d423..60146e32619a 100644 --- a/arch/arm/kernel/hyp-stub.S +++ b/arch/arm/kernel/hyp-stub.S @@ -31,6 +31,7 @@ * zeroing of .bss would clobber it. */ .data + .align 2 ENTRY(__boot_cpu_mode) .long 0 .text diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S index 49fadbda8c63..81cd4d43b3ec 100644 --- a/arch/arm/kernel/iwmmxt.S +++ b/arch/arm/kernel/iwmmxt.S @@ -367,6 +367,7 @@ ENTRY(iwmmxt_task_release) ENDPROC(iwmmxt_task_release) .data + .align 2 concan_owner: .word 0 diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S index 0f6c1000582c..9f08d214d05a 100644 --- a/arch/arm/kernel/sleep.S +++ b/arch/arm/kernel/sleep.S @@ -171,6 +171,7 @@ ENDPROC(cpu_resume_arm) .long mpidr_hash - . @ mpidr_hash struct offset .data + .align 2 .type sleep_save_sp, #object ENTRY(sleep_save_sp) .space SLEEP_SAVE_SP_SZ @ struct sleep_save_sp diff --git a/arch/arm/mm/cache-v4wb.S b/arch/arm/mm/cache-v4wb.S index 2522f8c8fbb1..a5084ec70c6e 100644 --- a/arch/arm/mm/cache-v4wb.S +++ b/arch/arm/mm/cache-v4wb.S @@ -47,6 +47,7 @@ #define CACHE_DLIMIT (CACHE_DSIZE * 4) .data + .align 2 flush_base: .long FLUSH_BASE .text diff --git a/arch/arm/mm/proc-xscale.S b/arch/arm/mm/proc-xscale.S index b6bbfdb6dfdc..3d75b7972fd1 100644 --- a/arch/arm/mm/proc-xscale.S +++ b/arch/arm/mm/proc-xscale.S @@ -104,6 +104,7 @@ .endm .data + .align 2 clean_addr: .word CLEAN_ADDR .text