The BSS section macros in vmlinux.lds.h currently place the .sbss
input section outside the bounds of [__bss_start, __bss_end]. On all
architectures except for microblaze that handle both .sbss and
__bss_start/__bss_end, this is wrong: the .sbss input section is
within the range [__bss_start, __bss_end]. Relatedly, the example
code at the top of the file actually has __bss_start/__bss_end defined
twice; I believe the right fix here is to define them in the
BSS_SECTION macro but not in the BSS macro.
Another problem with the current macros is that several
architectures have an ALIGN(4) or some other small number just before
__bss_stop in their linker scripts. The BSS_SECTION macro currently
hardcodes this to 4; while it should really be an argument. It also
ignores its sbss_align argument; fix that.
mn10300 is the only user at present of any of the macros touched by
this patch. It looks like mn10300 actually was incorrectly converted
to use the new BSS() macro (the alignment of 4 prior to conversion was
a __bss_stop alignment, but the argument to the BSS macro is a start
alignment). So fix this as well.
I'd like acks from Sam and David on this one. Also CCing Paul, since
he has a patch from me which will need to be updated to use
BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
Signed-off-by: Tim Abbott <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: Paul Mundt <[email protected]>
Cc: David Howells <[email protected]>
---
arch/mn10300/kernel/vmlinux.lds.S | 2 +-
include/asm-generic/vmlinux.lds.h | 19 +++++++++----------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/mn10300/kernel/vmlinux.lds.S b/arch/mn10300/kernel/vmlinux.lds.S
index c96ba3d..f4aa079 100644
--- a/arch/mn10300/kernel/vmlinux.lds.S
+++ b/arch/mn10300/kernel/vmlinux.lds.S
@@ -107,7 +107,7 @@ SECTIONS
__init_end = .;
/* freed after init ends here */
- BSS(4)
+ BSS_SECTION(0, PAGE_SIZE, 4)
_end = . ;
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index a553f10..6ad76bf 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -30,9 +30,7 @@
* EXCEPTION_TABLE(...)
* NOTES
*
- * __bss_start = .;
- * BSS_SECTION(0, 0)
- * __bss_stop = .;
+ * BSS_SECTION(0, 0, 0)
* _end = .;
*
* /DISCARD/ : {
@@ -489,7 +487,8 @@
* bss (Block Started by Symbol) - uninitialized data
* zeroed during startup
*/
-#define SBSS \
+#define SBSS(sbss_align) \
+ . = ALIGN(sbss_align); \
.sbss : AT(ADDR(.sbss) - LOAD_OFFSET) { \
*(.sbss) \
*(.scommon) \
@@ -498,12 +497,10 @@
#define BSS(bss_align) \
. = ALIGN(bss_align); \
.bss : AT(ADDR(.bss) - LOAD_OFFSET) { \
- VMLINUX_SYMBOL(__bss_start) = .; \
*(.bss.page_aligned) \
*(.dynbss) \
*(.bss) \
*(COMMON) \
- VMLINUX_SYMBOL(__bss_stop) = .; \
}
/*
@@ -735,8 +732,10 @@
INIT_RAM_FS \
}
-#define BSS_SECTION(sbss_align, bss_align) \
- SBSS \
+#define BSS_SECTION(sbss_align, bss_align, stop_align) \
+ . = ALIGN(sbss_align); \
+ VMLINUX_SYMBOL(__bss_start) = .; \
+ SBSS(sbss_align) \
BSS(bss_align) \
- . = ALIGN(4);
-
+ . = ALIGN(stop_align); \
+ VMLINUX_SYMBOL(__bss_stop) = .;
--
1.6.3.3
Sam, David,
Have you had a chance to look at this patch yet? The lost ALIGN(4) in the
mn10300 linker script that this fixes is a regression introduced by commit
2e8b5a09ebf1f98f02c1988a48415e89d4c25168 (between 2.6.30 and 2.6.31-rc1)
that we probably want to fix for 2.6.31. (That should be the only
functional effect of this patch.)
-Tim Abbott
On Sun, 12 Jul 2009, Tim Abbott wrote:
> The BSS section macros in vmlinux.lds.h currently place the .sbss
> input section outside the bounds of [__bss_start, __bss_end]. On all
> architectures except for microblaze that handle both .sbss and
> __bss_start/__bss_end, this is wrong: the .sbss input section is
> within the range [__bss_start, __bss_end]. Relatedly, the example
> code at the top of the file actually has __bss_start/__bss_end defined
> twice; I believe the right fix here is to define them in the
> BSS_SECTION macro but not in the BSS macro.
>
> Another problem with the current macros is that several
> architectures have an ALIGN(4) or some other small number just before
> __bss_stop in their linker scripts. The BSS_SECTION macro currently
> hardcodes this to 4; while it should really be an argument. It also
> ignores its sbss_align argument; fix that.
>
> mn10300 is the only user at present of any of the macros touched by
> this patch. It looks like mn10300 actually was incorrectly converted
> to use the new BSS() macro (the alignment of 4 prior to conversion was
> a __bss_stop alignment, but the argument to the BSS macro is a start
> alignment). So fix this as well.
>
> I'd like acks from Sam and David on this one. Also CCing Paul, since
> he has a patch from me which will need to be updated to use
> BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
>
> Signed-off-by: Tim Abbott <[email protected]>
> Cc: Sam Ravnborg <[email protected]>
> Cc: Paul Mundt <[email protected]>
> Cc: David Howells <[email protected]>
> ---
> arch/mn10300/kernel/vmlinux.lds.S | 2 +-
> include/asm-generic/vmlinux.lds.h | 19 +++++++++----------
> 2 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/arch/mn10300/kernel/vmlinux.lds.S b/arch/mn10300/kernel/vmlinux.lds.S
> index c96ba3d..f4aa079 100644
> --- a/arch/mn10300/kernel/vmlinux.lds.S
> +++ b/arch/mn10300/kernel/vmlinux.lds.S
> @@ -107,7 +107,7 @@ SECTIONS
> __init_end = .;
> /* freed after init ends here */
>
> - BSS(4)
> + BSS_SECTION(0, PAGE_SIZE, 4)
>
> _end = . ;
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index a553f10..6ad76bf 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -30,9 +30,7 @@
> * EXCEPTION_TABLE(...)
> * NOTES
> *
> - * __bss_start = .;
> - * BSS_SECTION(0, 0)
> - * __bss_stop = .;
> + * BSS_SECTION(0, 0, 0)
> * _end = .;
> *
> * /DISCARD/ : {
> @@ -489,7 +487,8 @@
> * bss (Block Started by Symbol) - uninitialized data
> * zeroed during startup
> */
> -#define SBSS \
> +#define SBSS(sbss_align) \
> + . = ALIGN(sbss_align); \
> .sbss : AT(ADDR(.sbss) - LOAD_OFFSET) { \
> *(.sbss) \
> *(.scommon) \
> @@ -498,12 +497,10 @@
> #define BSS(bss_align) \
> . = ALIGN(bss_align); \
> .bss : AT(ADDR(.bss) - LOAD_OFFSET) { \
> - VMLINUX_SYMBOL(__bss_start) = .; \
> *(.bss.page_aligned) \
> *(.dynbss) \
> *(.bss) \
> *(COMMON) \
> - VMLINUX_SYMBOL(__bss_stop) = .; \
> }
>
> /*
> @@ -735,8 +732,10 @@
> INIT_RAM_FS \
> }
>
> -#define BSS_SECTION(sbss_align, bss_align) \
> - SBSS \
> +#define BSS_SECTION(sbss_align, bss_align, stop_align) \
> + . = ALIGN(sbss_align); \
> + VMLINUX_SYMBOL(__bss_start) = .; \
> + SBSS(sbss_align) \
> BSS(bss_align) \
> - . = ALIGN(4);
> -
> + . = ALIGN(stop_align); \
> + VMLINUX_SYMBOL(__bss_stop) = .;
> --
> 1.6.3.3
>
On Sun, Jul 12, 2009 at 06:23:33PM -0400, Tim Abbott wrote:
> The BSS section macros in vmlinux.lds.h currently place the .sbss
> input section outside the bounds of [__bss_start, __bss_end]. On all
> architectures except for microblaze that handle both .sbss and
> __bss_start/__bss_end, this is wrong: the .sbss input section is
> within the range [__bss_start, __bss_end]. Relatedly, the example
> code at the top of the file actually has __bss_start/__bss_end defined
> twice; I believe the right fix here is to define them in the
> BSS_SECTION macro but not in the BSS macro.
>
> Another problem with the current macros is that several
> architectures have an ALIGN(4) or some other small number just before
> __bss_stop in their linker scripts. The BSS_SECTION macro currently
> hardcodes this to 4; while it should really be an argument. It also
> ignores its sbss_align argument; fix that.
>
> mn10300 is the only user at present of any of the macros touched by
> this patch. It looks like mn10300 actually was incorrectly converted
> to use the new BSS() macro (the alignment of 4 prior to conversion was
> a __bss_stop alignment, but the argument to the BSS macro is a start
> alignment). So fix this as well.
>
> I'd like acks from Sam and David on this one. Also CCing Paul, since
> he has a patch from me which will need to be updated to use
> BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
Path looks good - this is howit should have looked in the first place.
I recall that the alignmnet of 4 came from what is default for ld at least
for i386 but it really is dependent on the routine that clears the bss
section so we better make it configurable. Some archs may prefer to clear
bss in 8 byte chunks...
Could you please resend including the necessary fix for mn10300 so we do not
break bisecting.
thanks,
Sam
On Thu, 16 Jul 2009, Sam Ravnborg wrote:
> On Sun, Jul 12, 2009 at 06:23:33PM -0400, Tim Abbott wrote:
> > I'd like acks from Sam and David on this one. Also CCing Paul, since
> > he has a patch from me which will need to be updated to use
> > BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
>
> Path looks good - this is howit should have looked in the first place.
> I recall that the alignmnet of 4 came from what is default for ld at least
> for i386 but it really is dependent on the routine that clears the bss
> section so we better make it configurable. Some archs may prefer to clear
> bss in 8 byte chunks...
Right, e.g. avr32 uses 8 for this.
> Could you please resend including the necessary fix for mn10300 so we do not
> break bisecting.
I don't understand what you're asking here. The original patch I sent
does include the necessary fix for mn10300 in the first hunk (reproduced
below). I intentionally changed the mn10300 use at the same time as
changing the macros in order to avoid breaking bisecting.
I'd be happy to resend if it'd be helpful.
--- a/arch/mn10300/kernel/vmlinux.lds.S
+++ b/arch/mn10300/kernel/vmlinux.lds.S
@@ -107,7 +107,7 @@ SECTIONS
__init_end = .;
/* freed after init ends here */
- BSS(4)
+ BSS_SECTION(0, PAGE_SIZE, 4)
_end = . ;
-Tim Abbott
On Thu, 16 Jul 2009, Tim Abbott wrote:
> On Thu, 16 Jul 2009, Sam Ravnborg wrote:
>
> > On Sun, Jul 12, 2009 at 06:23:33PM -0400, Tim Abbott wrote:
> > > I'd like acks from Sam and David on this one. Also CCing Paul, since
> > > he has a patch from me which will need to be updated to use
> > > BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
[...]
> > Could you please resend including the necessary fix for mn10300 so we do not
> > break bisecting.
>
> I don't understand what you're asking here. The original patch I sent
> does include the necessary fix for mn10300 in the first hunk (reproduced
> below). I intentionally changed the mn10300 use at the same time as
> changing the macros in order to avoid breaking bisecting.
I realized that you probably meant that I should resend the sh patch that
I have currently in linux-next via Paul's tree that is broken by this
change.
Paul -- the patch below differs from the version have in two ways:
(1) I updated it to use the fixed BSS linker script macros from this
thread.
(2) I dropped the hunk that conflicted with a similar change in the percpu
tree[2], as the version of that change in the percpu tree is better.
[1] http://www.spinics.net/lists/kernel/msg913238.html
[2] http://www.spinics.net/lists/kernel/msg913304.html
-Tim Abbott
sh: Clean up linker script using new linker script macros.
Signed-off-by: Tim Abbott <[email protected]>
Cc: Paul Mundt <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: [email protected]
---
arch/sh/kernel/vmlinux.lds.S | 84 ++++-------------------------------------
1 files changed, 9 insertions(+), 75 deletions(-)
diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
index f53c76a..396d8ee 100644
--- a/arch/sh/kernel/vmlinux.lds.S
+++ b/arch/sh/kernel/vmlinux.lds.S
@@ -50,12 +50,7 @@ SECTIONS
_etext = .; /* End of text section */
} = 0x0009
- . = ALIGN(16); /* Exception table */
- __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {
- __start___ex_table = .;
- *(__ex_table)
- __stop___ex_table = .;
- }
+ EXCEPTION_TABLE(16)
NOTES
RO_DATA(PAGE_SIZE)
@@ -71,69 +66,14 @@ SECTIONS
__uncached_end = .;
}
- . = ALIGN(THREAD_SIZE);
- .data : AT(ADDR(.data) - LOAD_OFFSET) { /* Data */
- *(.data.init_task)
-
- . = ALIGN(L1_CACHE_BYTES);
- *(.data.cacheline_aligned)
-
- . = ALIGN(L1_CACHE_BYTES);
- *(.data.read_mostly)
-
- . = ALIGN(PAGE_SIZE);
- *(.data.page_aligned)
-
- __nosave_begin = .;
- *(.data.nosave)
- . = ALIGN(PAGE_SIZE);
- __nosave_end = .;
-
- DATA_DATA
- CONSTRUCTORS
- }
+ RW_DATA_SECTION(L1_CACHE_BYTES, PAGE_SIZE, THREAD_SIZE)
_edata = .; /* End of data section */
. = ALIGN(PAGE_SIZE); /* Init code and data */
- .init.text : AT(ADDR(.init.text) - LOAD_OFFSET) {
- __init_begin = .;
- _sinittext = .;
- INIT_TEXT
- _einittext = .;
- }
-
- .init.data : AT(ADDR(.init.data) - LOAD_OFFSET) { INIT_DATA }
-
- . = ALIGN(16);
- .init.setup : AT(ADDR(.init.setup) - LOAD_OFFSET) {
- __setup_start = .;
- *(.init.setup)
- __setup_end = .;
- }
-
- .initcall.init : AT(ADDR(.initcall.init) - LOAD_OFFSET) {
- __initcall_start = .;
- INITCALLS
- __initcall_end = .;
- }
-
- .con_initcall.init : AT(ADDR(.con_initcall.init) - LOAD_OFFSET) {
- __con_initcall_start = .;
- *(.con_initcall.init)
- __con_initcall_end = .;
- }
-
- SECURITY_INIT
-
-#ifdef CONFIG_BLK_DEV_INITRD
- . = ALIGN(PAGE_SIZE);
- .init.ramfs : AT(ADDR(.init.ramfs) - LOAD_OFFSET) {
- __initramfs_start = .;
- *(.init.ramfs)
- __initramfs_end = .;
- }
-#endif
+ __init_begin = .;
+ INIT_TEXT_SECTION(PAGE_SIZE)
+ INIT_DATA_SECTION(16)
. = ALIGN(4);
.machvec.init : AT(ADDR(.machvec.init) - LOAD_OFFSET) {
@@ -152,16 +92,10 @@ SECTIONS
.exit.data : AT(ADDR(.exit.data) - LOAD_OFFSET) { EXIT_DATA }
. = ALIGN(PAGE_SIZE);
- .bss : AT(ADDR(.bss) - LOAD_OFFSET) {
- __init_end = .;
- __bss_start = .; /* BSS */
- *(.bss.page_aligned)
- *(.bss)
- *(COMMON)
- . = ALIGN(4);
- _ebss = .; /* uClinux MTD sucks */
- _end = . ;
- }
+ __init_end = .;
+ BSS_SECTION(0, PAGE_SIZE, 4)
+ _ebss = .; /* uClinux MTD sucks */
+ _end = . ;
/*
* When something in the kernel is NOT compiled as a module, the
--
1.6.3.3
On Thu, Jul 16, 2009 at 06:58:50PM -0400, Tim Abbott wrote:
> On Thu, 16 Jul 2009, Tim Abbott wrote:
>
> > On Thu, 16 Jul 2009, Sam Ravnborg wrote:
> >
> > > On Sun, Jul 12, 2009 at 06:23:33PM -0400, Tim Abbott wrote:
> > > > I'd like acks from Sam and David on this one. Also CCing Paul, since
> > > > he has a patch from me which will need to be updated to use
> > > > BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
> [...]
> > > Could you please resend including the necessary fix for mn10300 so we do not
> > > break bisecting.
> >
> > I don't understand what you're asking here. The original patch I sent
> > does include the necessary fix for mn10300 in the first hunk (reproduced
> > below). I intentionally changed the mn10300 use at the same time as
> > changing the macros in order to avoid breaking bisecting.
>
> I realized that you probably meant that I should resend the sh patch that
> I have currently in linux-next via Paul's tree that is broken by this
> change.
No - I simply overlooked that you already had included
the mn10300 fix.
So I have applied your patch to kbuid-fixes.git.
I will push to Linus after some air-time in -next.
Sam
On Thu, Jul 16, 2009 at 06:58:50PM -0400, Tim Abbott wrote:
> On Thu, 16 Jul 2009, Tim Abbott wrote:
>
> > On Thu, 16 Jul 2009, Sam Ravnborg wrote:
> >
> > > On Sun, Jul 12, 2009 at 06:23:33PM -0400, Tim Abbott wrote:
> > > > I'd like acks from Sam and David on this one. Also CCing Paul, since
> > > > he has a patch from me which will need to be updated to use
> > > > BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
> [...]
> > > Could you please resend including the necessary fix for mn10300 so we do not
> > > break bisecting.
> >
> > I don't understand what you're asking here. The original patch I sent
> > does include the necessary fix for mn10300 in the first hunk (reproduced
> > below). I intentionally changed the mn10300 use at the same time as
> > changing the macros in order to avoid breaking bisecting.
>
> I realized that you probably meant that I should resend the sh patch that
> I have currently in linux-next via Paul's tree that is broken by this
> change.
>
> Paul -- the patch below differs from the version have in two ways:
> (1) I updated it to use the fixed BSS linker script macros from this
> thread.
> (2) I dropped the hunk that conflicted with a similar change in the percpu
> tree[2], as the version of that change in the percpu tree is better.
I do not expect Tejun's work to be included until next merge window.
It would be nice if we could queue up a lot of vmlinux.lds
cleanup patches so they are ready for next merge window.
But we should rely on the arch maintainers mostly - as you nicely
have done for sh here.
Sam
On Sat, Jul 18, 2009 at 12:13:36AM +0200, Sam Ravnborg wrote:
> On Thu, Jul 16, 2009 at 06:58:50PM -0400, Tim Abbott wrote:
> > On Thu, 16 Jul 2009, Tim Abbott wrote:
> >
> > > On Thu, 16 Jul 2009, Sam Ravnborg wrote:
> > >
> > > > On Sun, Jul 12, 2009 at 06:23:33PM -0400, Tim Abbott wrote:
> > > > > I'd like acks from Sam and David on this one. Also CCing Paul, since
> > > > > he has a patch from me which will need to be updated to use
> > > > > BSS_SECTION(0, PAGE_SIZE, 4) once this gets merged.
> > [...]
> > > > Could you please resend including the necessary fix for mn10300 so we do not
> > > > break bisecting.
> > >
> > > I don't understand what you're asking here. The original patch I sent
> > > does include the necessary fix for mn10300 in the first hunk (reproduced
> > > below). I intentionally changed the mn10300 use at the same time as
> > > changing the macros in order to avoid breaking bisecting.
> >
> > I realized that you probably meant that I should resend the sh patch that
> > I have currently in linux-next via Paul's tree that is broken by this
> > change.
> >
> > Paul -- the patch below differs from the version have in two ways:
> > (1) I updated it to use the fixed BSS linker script macros from this
> > thread.
> > (2) I dropped the hunk that conflicted with a similar change in the percpu
> > tree[2], as the version of that change in the percpu tree is better.
>
> I do not expect Tejun's work to be included until next merge window.
> It would be nice if we could queue up a lot of vmlinux.lds
> cleanup patches so they are ready for next merge window.
> But we should rely on the arch maintainers mostly - as you nicely
> have done for sh here.
>
I've pulled your kbuild-fixes.git in to a topic branch with Tim's
BSS_SECTION fix applied over top of it, this seemed like the easiest way
to keep my tree building both in and out of -next. Once kbuild-fixes hits
Linus's tree it should all sort itself out anyways.
On Sat, 18 Jul 2009, Sam Ravnborg wrote:
> I do not expect Tejun's work to be included until next merge window.
Yes, but I expect the "sh" patch presumably to go in at the next merge
window as well. Since in this case the conflict was both patches making
similar changes where Tejun's was more extensive, I figured I might as
well save someone from having to resolve this useless merge conflict
during the next merge window.
> It would be nice if we could queue up a lot of vmlinux.lds
> cleanup patches so they are ready for next merge window.
> But we should rely on the arch maintainers mostly - as you nicely
> have done for sh here.
Yes, I'm hoping to have vmlinux.lds cleanup patches for most architectures
before the merge window starts, though as I am quite busy, I can't make
any guarantees.
-Tim Abbott
On Mon, Jul 20, 2009 at 12:33:07PM -0400, Tim Abbott wrote:
> On Sat, 18 Jul 2009, Sam Ravnborg wrote:
> > I do not expect Tejun's work to be included until next merge window.
>
> Yes, but I expect the "sh" patch presumably to go in at the next merge
> window as well. Since in this case the conflict was both patches making
> similar changes where Tejun's was more extensive, I figured I might as
> well save someone from having to resolve this useless merge conflict
> during the next merge window.
>
Correct, the sh change is queued for the next merge window. For 2.6.31 we
are not using the generic macros in the script, so the breakage is
immaterial there. There will be a bit of an ordering issue for the merge
window with regards to the kbuild fixes, but as long as the revised
macros show up at some point then I can merge after that. If kbuild-fixes
merges before the merge window then of course the ordering issue goes
away.
On Tue, Jul 21, 2009 at 01:36:38AM +0900, Paul Mundt wrote:
> On Mon, Jul 20, 2009 at 12:33:07PM -0400, Tim Abbott wrote:
> > On Sat, 18 Jul 2009, Sam Ravnborg wrote:
> > > I do not expect Tejun's work to be included until next merge window.
> >
> > Yes, but I expect the "sh" patch presumably to go in at the next merge
> > window as well. Since in this case the conflict was both patches making
> > similar changes where Tejun's was more extensive, I figured I might as
> > well save someone from having to resolve this useless merge conflict
> > during the next merge window.
> >
> Correct, the sh change is queued for the next merge window. For 2.6.31 we
> are not using the generic macros in the script, so the breakage is
> immaterial there. There will be a bit of an ordering issue for the merge
> window with regards to the kbuild fixes, but as long as the revised
> macros show up at some point then I can merge after that. If kbuild-fixes
> merges before the merge window then of course the ordering issue goes
> away.
I plan to send Linus a pull request later today for the kbuild-fixes stuff.
I need to review the cahnges a last time.
Sam