Message ID | 1253119592-19598-2-git-send-email-tabbott@ksplice.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
From: Tim Abbott <tabbott@ksplice.com> Date: Wed, 16 Sep 2009 12:46:32 -0400 > @@ -51,70 +51,27 @@ SECTIONS > _etext = .; > > RO_DATA(PAGE_SIZE) > - .data : { > - DATA_DATA > - CONSTRUCTORS > - } > .data1 : { > *(.data1) > } > - . = ALIGN(SMP_CACHE_BYTES); > - .data.cacheline_aligned : { > - *(.data.cacheline_aligned) > - } > - . = ALIGN(SMP_CACHE_BYTES); > - .data.read_mostly : { > - *(.data.read_mostly) > - } > + RW_DATA_SECTION(SMP_CACHE_BYTES, 0, THREAD_SIZE) > + Can you do this cleanup without moving the relative locations of .data and .data1 sections? -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 16 Sep 2009, David Miller wrote: > Can you do this cleanup without moving the relative locations of .data > and .data1 sections? Yes, if you just swap RW_DATA_SECTION and .data1 so it looks like RW_DATA_SECTION(SMP_CACHE_BYTES, 0, THREAD_SIZE) .data1 : { *(.data1) } instead, that would preserve their relative locations. Currently, switching to RW_DATA_SECTION would still result in a change in their relative position that .data.page_aligned and .data.nosave would be between .data and .data1 (not sure if that is relevant on sparc). (this will change when <http://lkml.org/lkml/2009/9/16/396> is merged). -Tim Abbott -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Tim Abbott <tabbott@ksplice.com> Date: Wed, 16 Sep 2009 13:27:43 -0400 (EDT) > On Wed, 16 Sep 2009, David Miller wrote: > >> Can you do this cleanup without moving the relative locations of .data >> and .data1 sections? > > Yes, if you just swap RW_DATA_SECTION and .data1 so it looks like > > RW_DATA_SECTION(SMP_CACHE_BYTES, 0, THREAD_SIZE) > .data1 : { > *(.data1) > } > > instead, that would preserve their relative locations. > > Currently, switching to RW_DATA_SECTION would still result in a change in > their relative position that .data.page_aligned and .data.nosave would be > between .data and .data1 (not sure if that is relevant on sparc). (this > will change when <http://lkml.org/lkml/2009/9/16/396> is merged). I don't know which, if any, are relevant or could cause problems. It's hard for me to ACK this because it's not a straight nop transformation, which we could at least presume would function properly if the macros were implemented correctly. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 16 Sep 2009, David Miller wrote: > From: Tim Abbott <tabbott@ksplice.com> > Date: Wed, 16 Sep 2009 13:27:43 -0400 (EDT) > > > On Wed, 16 Sep 2009, David Miller wrote: > > > >> Can you do this cleanup without moving the relative locations of .data > >> and .data1 sections? > > > > Yes, if you just swap RW_DATA_SECTION and .data1 so it looks like > > > > RW_DATA_SECTION(SMP_CACHE_BYTES, 0, THREAD_SIZE) > > .data1 : { > > *(.data1) > > } > > > > instead, that would preserve their relative locations. > > > > Currently, switching to RW_DATA_SECTION would still result in a change in > > their relative position that .data.page_aligned and .data.nosave would be > > between .data and .data1 (not sure if that is relevant on sparc). (this > > will change when <http://lkml.org/lkml/2009/9/16/396> is merged). > > I don't know which, if any, are relevant or could cause problems. The kind of problem I've seen on other architectures is if there are short-range (e.g. 2-byte) relative relocations between two sections, and you insert a new section between them, they end up too far apart and the kernel fails to link. I don't know whether the sparc architecture has that kind of short relocation issue, but that's what I'd be worried about with section order changes. The other potential issue is sections moving past linker script defined symbols such as __init_end, so that the section might be allocated differently. The only change of that form in this patch is that it moves .data.init_task before _edata, which on sparc is only used to print how memory is used by different data types. The other thing I should mention is that I've not boot-tested this; I've only build-tested it with a sparc64 cross-compiler. So that should be done before merging this. > It's hard for me to ACK this because it's not a straight nop > transformation, which we could at least presume would function > properly if the macros were implemented correctly. Would it help if I were to split the patch into first rearranging the code to look like the macros and then applying the macros, so that you can see more easily exactly what is changing? -Tim Abbott -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 16, 2009 at 10:30:19AM -0700, David Miller wrote: > From: Tim Abbott <tabbott@ksplice.com> > Date: Wed, 16 Sep 2009 13:27:43 -0400 (EDT) > > > On Wed, 16 Sep 2009, David Miller wrote: > > > >> Can you do this cleanup without moving the relative locations of .data > >> and .data1 sections? > > > > Yes, if you just swap RW_DATA_SECTION and .data1 so it looks like > > > > RW_DATA_SECTION(SMP_CACHE_BYTES, 0, THREAD_SIZE) > > .data1 : { > > *(.data1) > > } > > > > instead, that would preserve their relative locations. > > > > Currently, switching to RW_DATA_SECTION would still result in a change in > > their relative position that .data.page_aligned and .data.nosave would be > > between .data and .data1 (not sure if that is relevant on sparc). (this > > will change when <http://lkml.org/lkml/2009/9/16/396> is merged). > > I don't know which, if any, are relevant or could cause problems. > > It's hard for me to ACK this because it's not a straight nop > transformation, which we could at least presume would function > properly if the macros were implemented correctly. As you most likely are aware the linker scripts has diverged a lot over time between different architectures. So whatever fits the ordering of one architecture fails on another architecture. Tim is doing a huge effort to bring some sanity into this area which I appreciate a lot! Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Tim Abbott <tabbott@ksplice.com> Date: Wed, 16 Sep 2009 14:03:48 -0400 (EDT) > On Wed, 16 Sep 2009, David Miller wrote: > >> It's hard for me to ACK this because it's not a straight nop >> transformation, which we could at least presume would function >> properly if the macros were implemented correctly. > > Would it help if I were to split the patch into first rearranging the code > to look like the macros and then applying the macros, so that you can see > more easily exactly what is changing? No, it wouldn't :-) The issue is that I can't just say from reading the patch that it will absolutely work. But I'm willing to take the risk and we can revert if testing shows it breaks things, so: Acked-by: David S. Miller <davem@davemloft.net> -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S index 866390f..4e59925 100644 --- a/arch/sparc/kernel/vmlinux.lds.S +++ b/arch/sparc/kernel/vmlinux.lds.S @@ -51,70 +51,27 @@ SECTIONS _etext = .; RO_DATA(PAGE_SIZE) - .data : { - DATA_DATA - CONSTRUCTORS - } .data1 : { *(.data1) } - . = ALIGN(SMP_CACHE_BYTES); - .data.cacheline_aligned : { - *(.data.cacheline_aligned) - } - . = ALIGN(SMP_CACHE_BYTES); - .data.read_mostly : { - *(.data.read_mostly) - } + RW_DATA_SECTION(SMP_CACHE_BYTES, 0, THREAD_SIZE) + /* End of data section */ _edata = .; - /* init_task */ - . = ALIGN(THREAD_SIZE); - .data.init_task : { - *(.data.init_task) - } .fixup : { __start___fixup = .; *(.fixup) __stop___fixup = .; } - . = ALIGN(16); - __ex_table : { - __start___ex_table = .; - *(__ex_table) - __stop___ex_table = .; - } + EXCEPTION_TABLE(16) NOTES . = ALIGN(PAGE_SIZE); - .init.text : { - __init_begin = .; - _sinittext = .; - INIT_TEXT - _einittext = .; - } + __init_begin = ALIGN(PAGE_SIZE); + INIT_TEXT_SECTION(PAGE_SIZE) __init_text_end = .; - .init.data : { - INIT_DATA - } - . = ALIGN(16); - .init.setup : { - __setup_start = .; - *(.init.setup) - __setup_end = .; - } - .initcall.init : { - __initcall_start = .; - INITCALLS - __initcall_end = .; - } - .con_initcall.init : { - __con_initcall_start = .; - *(.con_initcall.init) - __con_initcall_end = .; - } - SECURITY_INIT + INIT_DATA_SECTION(16) . = ALIGN(4); .tsb_ldquad_phys_patch : { @@ -146,29 +103,11 @@ SECTIONS __sun4v_2insn_patch_end = .; } -#ifdef CONFIG_BLK_DEV_INITRD - . = ALIGN(PAGE_SIZE); - .init.ramfs : { - __initramfs_start = .; - *(.init.ramfs) - __initramfs_end = .; - } -#endif - PERCPU(PAGE_SIZE) . = ALIGN(PAGE_SIZE); __init_end = .; - __bss_start = .; - .sbss : { - *(.sbss) - *(.scommon) - } - .bss : { - *(.dynbss) - *(.bss) - *(COMMON) - } + BSS_SECTION(0, 0, 0) _end = . ; STABS_DEBUG