2009-09-07 03:10:07

by Tim Abbott

[permalink] [raw]
Subject: [PATCH] blackfin: Cleanup linker script using new linker script macros.

Signed-off-by: Tim Abbott <[email protected]>
Cc: Bryan Wu <[email protected]>
Cc: [email protected]
Cc: Sam Ravnborg <[email protected]>
---
arch/blackfin/kernel/vmlinux.lds.S | 54 +++--------------------------------
1 files changed, 5 insertions(+), 49 deletions(-)

diff --git a/arch/blackfin/kernel/vmlinux.lds.S b/arch/blackfin/kernel/vmlinux.lds.S
index 6ac307c..91f5057 100644
--- a/arch/blackfin/kernel/vmlinux.lds.S
+++ b/arch/blackfin/kernel/vmlinux.lds.S
@@ -96,8 +96,7 @@ SECTIONS
{
__sdata = .;
/* This gets done first, so the glob doesn't suck it in */
- . = ALIGN(32);
- *(.data.cacheline_aligned)
+ CACHELINE_ALIGNED_DATA(32)

#if !L1_DATA_A_LENGTH
. = ALIGN(32);
@@ -116,12 +115,7 @@ SECTIONS
DATA_DATA
CONSTRUCTORS

- /* make sure the init_task is aligned to the
- * kernel thread size so we can locate the kernel
- * stack properly and quickly.
- */
- . = ALIGN(THREAD_SIZE);
- *(.init_task.data)
+ INIT_TASK_DATA(THREAD_SIZE)

__edata = .;
}
@@ -134,39 +128,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
___init_begin = .;

- .init.text :
- {
- . = ALIGN(PAGE_SIZE);
- __sinittext = .;
- INIT_TEXT
- __einittext = .;
- }
- .init.data :
- {
- . = ALIGN(16);
- INIT_DATA
- }
- .init.setup :
- {
- . = ALIGN(16);
- ___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 = .;
- }
+ INIT_TEXT_SECTION(PAGE_SIZE)
+ . = ALIGN(16);
+ INIT_DATA_SECTION(16)
PERCPU(4)
- SECURITY_INIT

/* we have to discard exit text and such at runtime, not link time, to
* handle embedded cross-section references (alt instructions, bug
@@ -181,15 +146,6 @@ SECTIONS
EXIT_DATA
}

- .init.ramfs :
- {
- . = ALIGN(4);
- ___initramfs_start = .;
- *(.init.ramfs)
- . = ALIGN(4);
- ___initramfs_end = .;
- }
-
__l1_lma_start = .;

.text_l1 L1_CODE_START : AT(LOADADDR(.init.ramfs) + SIZEOF(.init.ramfs))
--
1.6.3.3


2009-09-16 03:13:35

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

the larger padding in the initramfs is kind of annoying as i cant see
any need to pad it to PAGE_SIZE. since the initramfs is released with
the rest of the init section, it doesnt need whole pages. a quick
test shows that it does waste a few kb in reality. default build for
BF533-STAMP for example shows 0x1000 difference.

in terms of correctness, this change misses a reference to the now
deleted .init.ramfs:
- .init.ramfs :
- {
- .....
- }
-
.text_l1 L1_CODE_START : AT(LOADADDR(.init.ramfs) + SIZEOF(.init.ramfs))

so that .text_l1 needs to updated to refer to the new section before
it (.exit.data in this case). once i make that change, the resulting
link looks the same (minus the initramfs thing mentioned earlier), and
it does boot.
-mike

2009-09-16 15:59:56

by Tim Abbott

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Tue, 15 Sep 2009, Mike Frysinger wrote:

> the larger padding in the initramfs is kind of annoying as i cant see
> any need to pad it to PAGE_SIZE. since the initramfs is released with
> the rest of the init section, it doesnt need whole pages. a quick
> test shows that it does waste a few kb in reality. default build for
> BF533-STAMP for example shows 0x1000 difference.
>
> in terms of correctness, this change misses a reference to the now
> deleted .init.ramfs:
> - .init.ramfs :
> - {
> - .....
> - }
> -
> .text_l1 L1_CODE_START : AT(LOADADDR(.init.ramfs) + SIZEOF(.init.ramfs))
>
> so that .text_l1 needs to updated to refer to the new section before
> it (.exit.data in this case). once i make that change, the resulting
> link looks the same (minus the initramfs thing mentioned earlier), and
> it does boot.

OK. I guess we should plan to modify the INIT_DATA_SECTION macro to add
another argument specifying an alignment level for .init.ramfs. It'd be
inconvenient to add that right now since there are a lot of patches in
linux-next or otherwise in flight that introduce uses of
INIT_DATA_SECTION, and those patches would all be broken by changing this
now. Once the dust settles on that for this release, I'll submit a patch
adding said argument to INIT_DATA_SECTION.

-Tim Abbott

2009-09-16 16:56:32

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Wed, Sep 16, 2009 at 11:58, Tim Abbott wrote:
> On Tue, 15 Sep 2009, Mike Frysinger wrote:
>> the larger padding in the initramfs is kind of annoying as i cant see
>> any need to pad it to PAGE_SIZE.  since the initramfs is released with
>> the rest of the init section, it doesnt need whole pages.  a quick
>> test shows that it does waste a few kb in reality.  default build for
>> BF533-STAMP for example shows 0x1000 difference.
>
> OK.  I guess we should plan to modify the INIT_DATA_SECTION macro to add
> another argument specifying an alignment level for .init.ramfs.  It'd be
> inconvenient to add that right now since there are a lot of patches in
> linux-next or otherwise in flight that introduce uses of
> INIT_DATA_SECTION, and those patches would all be broken by changing this
> now.  Once the dust settles on that for this release, I'll submit a patch
> adding said argument to INIT_DATA_SECTION.

sounds good to me. too bad the function alignment isnt specified in
arch linkage.h, otherwise i'd suggest we default to that rather than
PAGE_SIZE. as seen in the current Blackfin linker script, we only
need the normal .text alignment of 4 bytes.
-mike

2009-09-16 20:27:01

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Wed, Sep 16, 2009 at 11:58:01AM -0400, Tim Abbott wrote:
> On Tue, 15 Sep 2009, Mike Frysinger wrote:
>
> > the larger padding in the initramfs is kind of annoying as i cant see
> > any need to pad it to PAGE_SIZE. since the initramfs is released with
> > the rest of the init section, it doesnt need whole pages. a quick
> > test shows that it does waste a few kb in reality. default build for
> > BF533-STAMP for example shows 0x1000 difference.
> >
> > in terms of correctness, this change misses a reference to the now
> > deleted .init.ramfs:
> > - .init.ramfs :
> > - {
> > - .....
> > - }
> > -
> > .text_l1 L1_CODE_START : AT(LOADADDR(.init.ramfs) + SIZEOF(.init.ramfs))
> >
> > so that .text_l1 needs to updated to refer to the new section before
> > it (.exit.data in this case). once i make that change, the resulting
> > link looks the same (minus the initramfs thing mentioned earlier), and
> > it does boot.
>
> OK. I guess we should plan to modify the INIT_DATA_SECTION macro to add
> another argument specifying an alignment level for .init.ramfs. It'd be
> inconvenient to add that right now since there are a lot of patches in
> linux-next or otherwise in flight that introduce uses of
> INIT_DATA_SECTION, and those patches would all be broken by changing this
> now. Once the dust settles on that for this release, I'll submit a patch
> adding said argument to INIT_DATA_SECTION.

But this is all discarded during runtime so the added alignment has no cost in the end - no?

Sam

2009-09-16 20:37:56

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Wed, Sep 16, 2009 at 16:26, Sam Ravnborg wrote:
> On Wed, Sep 16, 2009 at 11:58:01AM -0400, Tim Abbott wrote:
>> On Tue, 15 Sep 2009, Mike Frysinger wrote:
>> > the larger padding in the initramfs is kind of annoying as i cant see
>> > any need to pad it to PAGE_SIZE.  since the initramfs is released with
>> > the rest of the init section, it doesnt need whole pages.  a quick
>> > test shows that it does waste a few kb in reality.  default build for
>> > BF533-STAMP for example shows 0x1000 difference.
>> >
>> > in terms of correctness, this change misses a reference to the now
>> > deleted .init.ramfs:
>> > -       .init.ramfs :
>> > -       {
>> > -       .....
>> > -       }
>> > -
>> >        .text_l1 L1_CODE_START : AT(LOADADDR(.init.ramfs) + SIZEOF(.init.ramfs))
>> >
>> > so that .text_l1 needs to updated to refer to the new section before
>> > it (.exit.data in this case).  once i make that change, the resulting
>> > link looks the same (minus the initramfs thing mentioned earlier), and
>> > it does boot.
>>
>> OK.  I guess we should plan to modify the INIT_DATA_SECTION macro to add
>> another argument specifying an alignment level for .init.ramfs.  It'd be
>> inconvenient to add that right now since there are a lot of patches in
>> linux-next or otherwise in flight that introduce uses of
>> INIT_DATA_SECTION, and those patches would all be broken by changing this
>> now.  Once the dust settles on that for this release, I'll submit a patch
>> adding said argument to INIT_DATA_SECTION.
>
> But this is all discarded during runtime so the added alignment has no cost in the end - no?

once things are booted, there should be no difference. but
storage/boot costs increase (you have to store/extract/copy that extra
data). you know how miserly we embedded people like to be ;).
-mike

2009-09-22 15:31:53

by Tim Abbott

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Wed, 16 Sep 2009, Mike Frysinger wrote:

> On Wed, Sep 16, 2009 at 16:26, Sam Ravnborg wrote:
> > On Wed, Sep 16, 2009 at 11:58:01AM -0400, Tim Abbott wrote:
> >> OK. ?I guess we should plan to modify the INIT_DATA_SECTION macro to add
> >> another argument specifying an alignment level for .init.ramfs. ?It'd be
> >> inconvenient to add that right now since there are a lot of patches in
> >> linux-next or otherwise in flight that introduce uses of
> >> INIT_DATA_SECTION, and those patches would all be broken by changing this
> >> now. ?Once the dust settles on that for this release, I'll submit a patch
> >> adding said argument to INIT_DATA_SECTION.
> >
> > But this is all discarded during runtime so the added alignment has no cost in the end - no?
>
> once things are booted, there should be no difference. but
> storage/boot costs increase (you have to store/extract/copy that extra
> data). you know how miserly we embedded people like to be ;).

OK, so how do you want to do this? The options I see are:
(1) we merge this patch now, and add the new alignment argument for -rc2
(2) we add the alignment argument sometime after -rc1 and then merge this
for -rc2

-Tim Abbott

2009-09-22 15:30:49

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Tue, Sep 22, 2009 at 11:29, Tim Abbott wrote:
> On Wed, 16 Sep 2009, Mike Frysinger wrote:
>> On Wed, Sep 16, 2009 at 16:26, Sam Ravnborg wrote:
>> > On Wed, Sep 16, 2009 at 11:58:01AM -0400, Tim Abbott wrote:
>> >> OK.  I guess we should plan to modify the INIT_DATA_SECTION macro to add
>> >> another argument specifying an alignment level for .init.ramfs.  It'd be
>> >> inconvenient to add that right now since there are a lot of patches in
>> >> linux-next or otherwise in flight that introduce uses of
>> >> INIT_DATA_SECTION, and those patches would all be broken by changing this
>> >> now.  Once the dust settles on that for this release, I'll submit a patch
>> >> adding said argument to INIT_DATA_SECTION.
>> >
>> > But this is all discarded during runtime so the added alignment has no cost in the end - no?
>>
>> once things are booted, there should be no difference.  but
>> storage/boot costs increase (you have to store/extract/copy that extra
>> data).  you know how miserly we embedded people like to be ;).
>
> OK, so how do you want to do this?  The options I see are:
> (1) we merge this patch now, and add the new alignment argument for -rc2
> (2) we add the alignment argument sometime after -rc1 and then merge this
> for -rc2

doing it in two steps is OK by me and sounds like it'd be easier for you
-mike

2009-09-22 15:41:50

by Tim Abbott

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Tue, 22 Sep 2009, Mike Frysinger wrote:

> On Tue, Sep 22, 2009 at 11:29, Tim Abbott wrote:
> > On Wed, 16 Sep 2009, Mike Frysinger wrote:
> >> On Wed, Sep 16, 2009 at 16:26, Sam Ravnborg wrote:
> >> > On Wed, Sep 16, 2009 at 11:58:01AM -0400, Tim Abbott wrote:
> >> >> OK. ?I guess we should plan to modify the INIT_DATA_SECTION macro to add
> >> >> another argument specifying an alignment level for .init.ramfs. ?It'd be
> >> >> inconvenient to add that right now since there are a lot of patches in
> >> >> linux-next or otherwise in flight that introduce uses of
> >> >> INIT_DATA_SECTION, and those patches would all be broken by changing this
> >> >> now. ?Once the dust settles on that for this release, I'll submit a patch
> >> >> adding said argument to INIT_DATA_SECTION.
> >> >
> >> > But this is all discarded during runtime so the added alignment has no cost in the end - no?
> >>
> >> once things are booted, there should be no difference. ?but
> >> storage/boot costs increase (you have to store/extract/copy that extra
> >> data). ?you know how miserly we embedded people like to be ;).
> >
> > OK, so how do you want to do this? ?The options I see are:
> > (1) we merge this patch now, and add the new alignment argument for -rc2
> > (2) we add the alignment argument sometime after -rc1 and then merge this
> > for -rc2
>
> doing it in two steps is OK by me and sounds like it'd be easier for you

Both options involve two steps -- but as (1) is obviously easier for me, I
assume that's what you were referring to. Thanks.

So are you going to send this to Linus? I'd be happy to do so, but I'd
need your ack.

-Tim Abbott

2009-09-22 15:46:49

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Tue, Sep 22, 2009 at 11:39, Tim Abbott wrote:
> On Tue, 22 Sep 2009, Mike Frysinger wrote:
>> On Tue, Sep 22, 2009 at 11:29, Tim Abbott wrote:
>> > On Wed, 16 Sep 2009, Mike Frysinger wrote:
>> >> On Wed, Sep 16, 2009 at 16:26, Sam Ravnborg wrote:
>> >> > On Wed, Sep 16, 2009 at 11:58:01AM -0400, Tim Abbott wrote:
>> >> >> OK.  I guess we should plan to modify the INIT_DATA_SECTION macro to add
>> >> >> another argument specifying an alignment level for .init.ramfs.  It'd be
>> >> >> inconvenient to add that right now since there are a lot of patches in
>> >> >> linux-next or otherwise in flight that introduce uses of
>> >> >> INIT_DATA_SECTION, and those patches would all be broken by changing this
>> >> >> now.  Once the dust settles on that for this release, I'll submit a patch
>> >> >> adding said argument to INIT_DATA_SECTION.
>> >> >
>> >> > But this is all discarded during runtime so the added alignment has no cost in the end - no?
>> >>
>> >> once things are booted, there should be no difference.  but
>> >> storage/boot costs increase (you have to store/extract/copy that extra
>> >> data).  you know how miserly we embedded people like to be ;).
>> >
>> > OK, so how do you want to do this?  The options I see are:
>> > (1) we merge this patch now, and add the new alignment argument for -rc2
>> > (2) we add the alignment argument sometime after -rc1 and then merge this
>> > for -rc2
>>
>> doing it in two steps is OK by me and sounds like it'd be easier for you
>
> Both options involve two steps -- but as (1) is obviously easier for me, I
> assume that's what you were referring to.  Thanks.
>
> So are you going to send this to Linus?  I'd be happy to do so, but I'd
> need your ack.

if you apply the .text_l1 update (.init.ramfs -> .exit.data), then you
can add my s-o-b. otherwise the original patch as posted is broken.
-mike

2009-09-22 15:53:27

by Tim Abbott

[permalink] [raw]
Subject: Re: [PATCH] blackfin: Cleanup linker script using new linker script macros.

On Tue, 22 Sep 2009, Mike Frysinger wrote:

> On Tue, Sep 22, 2009 at 11:39, Tim Abbott wrote:
> > So are you going to send this to Linus?  I'd be happy to do so, but I'd
> > need your ack.
>
> if you apply the .text_l1 update (.init.ramfs -> .exit.data), then you
> can add my s-o-b. otherwise the original patch as posted is broken.

Right, I modified the patch in my git tree when you first mentioned it
like so:

- .text_l1 L1_CODE_START : AT(LOADADDR(.init.ramfs) + SIZEOF(.init.ramfs))
+ .text_l1 L1_CODE_START : AT(LOADADDR(.exit.data) + SIZEOF(.exit.data))

-Tim Abbott