2010-08-04 11:57:03

by Sam Ravnborg

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

On Fri, May 21, 2010 at 04:58:53PM -0400, Mike Frysinger wrote:
> am i missing something or does the .data.shared_align section lack
> definition in vmlinux.lds.h and all arch vmlinux.lds.S files ?
>
> with the recent change "net: remove time limit in process_backlog()",
> the softnet_data variable changed from "DEFINE_PER_CPU()" to
> "DEFINE_PER_CPU_ALIGNED()" which moved it from the .data section to
> the .data.shared_align section. i'm not saying this patch is wrong,
> just that is what caused me to notice this larger problem. no one
> else in the kernel is using this aligned macro variant, so i imagine
> that's why no one has noticed yet.
>
> since .data.shared_align isnt declared in any vmlinux files that i can
> see, the linker just places it last. this "just works" for most
> people, but when building a ROM kernel on Blackfin systems, it causes
> section overlap errors:
> bfin-uclinux-ld.real: section .init.data [00000000202e06b8 ->
> 00000000202e48b7] overlaps section .data.shared_aligned
> [00000000202e06b8 -> 00000000202e0723]
> i imagine other arches which support the ROM config option and thus do
> funky placement would see similar issues ...
>
> on x86, it is stuck in a dedicated section:
> [ 8] .data PROGBITS ffffffff810ec000 2ec000
> 0303a8 00 WA 0 0 4096
> [ 9] .data.shared_alig PROGBITS ffffffff8111c3c0 31c3c0
> 0000c8 00 WA 0 0 64
>
> the ifdef forest in asm-generic/percpu.h is beyond a quick glance &
> fix, so i leave it up to someone else ;)

as there any resolution on this?
I briefly looked at it some time ago.
And it looks like a plain oversight.

But I was also a bit lost in the forest of ifdef land...

Sam


2010-08-04 18:12:27

by Mike Frysinger

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

On Wed, Aug 4, 2010 at 07:56, Sam Ravnborg wrote:
> On Fri, May 21, 2010 at 04:58:53PM -0400, Mike Frysinger wrote:
>> am i missing something or does the .data.shared_align section lack
>> definition in vmlinux.lds.h and all arch vmlinux.lds.S files ?
>>
>> with the recent change "net: remove time limit in process_backlog()",
>> the softnet_data variable changed from "DEFINE_PER_CPU()" to
>> "DEFINE_PER_CPU_ALIGNED()" which moved it from the .data section to
>> the .data.shared_align section.  i'm not saying this patch is wrong,
>> just that is what caused me to notice this larger problem.  no one
>> else in the kernel is using this aligned macro variant, so i imagine
>> that's why no one has noticed yet.
>>
>> since .data.shared_align isnt declared in any vmlinux files that i can
>> see, the linker just places it last.  this "just works" for most
>> people, but when building a ROM kernel on Blackfin systems, it causes
>> section overlap errors:
>> bfin-uclinux-ld.real: section .init.data [00000000202e06b8 ->
>> 00000000202e48b7] overlaps section .data.shared_aligned
>> [00000000202e06b8 -> 00000000202e0723]
>> i imagine other arches which support the ROM config option and thus do
>> funky placement would see similar issues ...
>>
>> on x86, it is stuck in a dedicated section:
>>   [ 8] .data             PROGBITS        ffffffff810ec000 2ec000
>> 0303a8 00  WA  0   0 4096
>>   [ 9] .data.shared_alig PROGBITS        ffffffff8111c3c0 31c3c0
>> 0000c8 00  WA  0   0 64
>>
>> the ifdef forest in asm-generic/percpu.h is beyond a quick glance &
>> fix, so i leave it up to someone else ;)
>
> as there any resolution on this?
> I briefly looked at it some time ago.
> And it looks like a plain oversight.

no, it's still broken in 2.6.35 :(
$ make ARCH=blackfin BF537-STAMP_defconfig
$ make ARCH=blackfin -s -j4
$ readelf -WS vmlinux | grep '\<data\>'
[10] .data PROGBITS 0018369c 17269c 012964 00 WA 0 0 4
[11] .data..shared_aligned PROGBITS 00196000 185000 00006c 00
WA 0 0 4
-mike

2010-08-04 20:28:16

by Sam Ravnborg

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

On Wed, Aug 04, 2010 at 02:12:03PM -0400, Mike Frysinger wrote:
> On Wed, Aug 4, 2010 at 07:56, Sam Ravnborg wrote:
> > On Fri, May 21, 2010 at 04:58:53PM -0400, Mike Frysinger wrote:
> >> am i missing something or does the .data.shared_align section lack
> >> definition in vmlinux.lds.h and all arch vmlinux.lds.S files ?
> >>
> >> with the recent change "net: remove time limit in process_backlog()",
> >> the softnet_data variable changed from "DEFINE_PER_CPU()" to
> >> "DEFINE_PER_CPU_ALIGNED()" which moved it from the .data section to
> >> the .data.shared_align section. ?i'm not saying this patch is wrong,
> >> just that is what caused me to notice this larger problem. ?no one
> >> else in the kernel is using this aligned macro variant, so i imagine
> >> that's why no one has noticed yet.
> >>
> >> since .data.shared_align isnt declared in any vmlinux files that i can
> >> see, the linker just places it last. ?this "just works" for most
> >> people, but when building a ROM kernel on Blackfin systems, it causes
> >> section overlap errors:
> >> bfin-uclinux-ld.real: section .init.data [00000000202e06b8 ->
> >> 00000000202e48b7] overlaps section .data.shared_aligned
> >> [00000000202e06b8 -> 00000000202e0723]
> >> i imagine other arches which support the ROM config option and thus do
> >> funky placement would see similar issues ...
> >>
> >> on x86, it is stuck in a dedicated section:
> >> ? [ 8] .data ? ? ? ? ? ? PROGBITS ? ? ? ?ffffffff810ec000 2ec000
> >> 0303a8 00 ?WA ?0 ? 0 4096
> >> ? [ 9] .data.shared_alig PROGBITS ? ? ? ?ffffffff8111c3c0 31c3c0
> >> 0000c8 00 ?WA ?0 ? 0 64
> >>
> >> the ifdef forest in asm-generic/percpu.h is beyond a quick glance &
> >> fix, so i leave it up to someone else ;)
> >
> > as there any resolution on this?
> > I briefly looked at it some time ago.
> > And it looks like a plain oversight.
>
> no, it's still broken in 2.6.35 :(
> $ make ARCH=blackfin BF537-STAMP_defconfig
> $ make ARCH=blackfin -s -j4
> $ readelf -WS vmlinux | grep '\<data\>'
> [10] .data PROGBITS 0018369c 17269c 012964 00 WA 0 0 4
> [11] .data..shared_aligned PROGBITS 00196000 185000 00006c 00
> WA 0 0 4

Following simple patch should deal with it.
Jeremy - you introduced this in ("x86/i386: Put aligned
stack-canary in percpu shared_aligned section")
53f824520b6d84ca5b4a8fd71addc91dbf64357e

Does the following simple fix look correct to you?

Sam

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 030a954..e87260f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -150,6 +150,7 @@
#define DATA_DATA \
*(.data) \
*(.ref.data) \
+ *(.data..shared_aligned) /* percpu related */ \
DEV_KEEP(init.data) \
DEV_KEEP(exit.data) \
CPU_KEEP(init.data) \

2010-08-04 20:40:12

by Sam Ravnborg

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

[Resend - this time with Jeremy included in To: ]

Jeremy - you introduced this in ("x86/i386: Put aligned
stack-canary in percpu shared_aligned section")
53f824520b6d84ca5b4a8fd71addc91dbf64357e

Does the following simple fix look correct to you?
[See below]

Sam

>
On Wed, Aug 04, 2010 at 10:28:06PM +0200, Sam Ravnborg wrote:
> On Wed, Aug 04, 2010 at 02:12:03PM -0400, Mike Frysinger wrote:
> > On Wed, Aug 4, 2010 at 07:56, Sam Ravnborg wrote:
> > > On Fri, May 21, 2010 at 04:58:53PM -0400, Mike Frysinger wrote:
> > >> am i missing something or does the .data.shared_align section lack
> > >> definition in vmlinux.lds.h and all arch vmlinux.lds.S files ?
> > >>
> > >> with the recent change "net: remove time limit in process_backlog()",
> > >> the softnet_data variable changed from "DEFINE_PER_CPU()" to
> > >> "DEFINE_PER_CPU_ALIGNED()" which moved it from the .data section to
> > >> the .data.shared_align section. ?i'm not saying this patch is wrong,
> > >> just that is what caused me to notice this larger problem. ?no one
> > >> else in the kernel is using this aligned macro variant, so i imagine
> > >> that's why no one has noticed yet.
> > >>
> > >> since .data.shared_align isnt declared in any vmlinux files that i can
> > >> see, the linker just places it last. ?this "just works" for most
> > >> people, but when building a ROM kernel on Blackfin systems, it causes
> > >> section overlap errors:
> > >> bfin-uclinux-ld.real: section .init.data [00000000202e06b8 ->
> > >> 00000000202e48b7] overlaps section .data.shared_aligned
> > >> [00000000202e06b8 -> 00000000202e0723]
> > >> i imagine other arches which support the ROM config option and thus do
> > >> funky placement would see similar issues ...
> > >>
> > >> on x86, it is stuck in a dedicated section:
> > >> ? [ 8] .data ? ? ? ? ? ? PROGBITS ? ? ? ?ffffffff810ec000 2ec000
> > >> 0303a8 00 ?WA ?0 ? 0 4096
> > >> ? [ 9] .data.shared_alig PROGBITS ? ? ? ?ffffffff8111c3c0 31c3c0
> > >> 0000c8 00 ?WA ?0 ? 0 64
> > >>
> > >> the ifdef forest in asm-generic/percpu.h is beyond a quick glance &
> > >> fix, so i leave it up to someone else ;)
> > >
> > > as there any resolution on this?
> > > I briefly looked at it some time ago.
> > > And it looks like a plain oversight.
> >
> > no, it's still broken in 2.6.35 :(
> > $ make ARCH=blackfin BF537-STAMP_defconfig
> > $ make ARCH=blackfin -s -j4
> > $ readelf -WS vmlinux | grep '\<data\>'
> > [10] .data PROGBITS 0018369c 17269c 012964 00 WA 0 0 4
> > [11] .data..shared_aligned PROGBITS 00196000 185000 00006c 00
> > WA 0 0 4
>

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 030a954..e87260f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -150,6 +150,7 @@
#define DATA_DATA \
*(.data) \
*(.ref.data) \
+ *(.data..shared_aligned) /* percpu related */ \
DEV_KEEP(init.data) \
DEV_KEEP(exit.data) \
CPU_KEEP(init.data) \

2010-08-05 05:14:36

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

On 08/04/2010 01:40 PM, Sam Ravnborg wrote:
> [Resend - this time with Jeremy included in To: ]
>
> Jeremy - you introduced this in ("x86/i386: Put aligned
> stack-canary in percpu shared_aligned section")
> 53f824520b6d84ca5b4a8fd71addc91dbf64357e
>
> Does the following simple fix look correct to you?
> [See below]
>
> Sam
>
> On Wed, Aug 04, 2010 at 10:28:06PM +0200, Sam Ravnborg wrote:
>> On Wed, Aug 04, 2010 at 02:12:03PM -0400, Mike Frysinger wrote:
>>> On Wed, Aug 4, 2010 at 07:56, Sam Ravnborg wrote:
>>>> On Fri, May 21, 2010 at 04:58:53PM -0400, Mike Frysinger wrote:
>>>>> am i missing something or does the .data.shared_align section lack
>>>>> definition in vmlinux.lds.h and all arch vmlinux.lds.S files ?
>>>>>
>>>>> with the recent change "net: remove time limit in process_backlog()",
>>>>> the softnet_data variable changed from "DEFINE_PER_CPU()" to
>>>>> "DEFINE_PER_CPU_ALIGNED()" which moved it from the .data section to
>>>>> the .data.shared_align section. i'm not saying this patch is wrong,
>>>>> just that is what caused me to notice this larger problem. no one
>>>>> else in the kernel is using this aligned macro variant, so i imagine
>>>>> that's why no one has noticed yet.

That's a bit odd and embarrassing... I guess the stack_canary stuff is
only visible when using CONFIG_CC_STACKPROTECTOR.

>>>>> since .data.shared_align isnt declared in any vmlinux files that i can
>>>>> see, the linker just places it last. this "just works" for most
>>>>> people, but when building a ROM kernel on Blackfin systems, it causes
>>>>> section overlap errors:
>>>>> bfin-uclinux-ld.real: section .init.data [00000000202e06b8 ->
>>>>> 00000000202e48b7] overlaps section .data.shared_aligned
>>>>> [00000000202e06b8 -> 00000000202e0723]
>>>>> i imagine other arches which support the ROM config option and thus do
>>>>> funky placement would see similar issues ...
>>>>>
>>>>> on x86, it is stuck in a dedicated section:
>>>>> [ 8] .data PROGBITS ffffffff810ec000 2ec000
>>>>> 0303a8 00 WA 0 0 4096
>>>>> [ 9] .data.shared_alig PROGBITS ffffffff8111c3c0 31c3c0
>>>>> 0000c8 00 WA 0 0 64
>>>>>
>>>>> the ifdef forest in asm-generic/percpu.h is beyond a quick glance&
>>>>> fix, so i leave it up to someone else ;)
>>>> as there any resolution on this?
>>>> I briefly looked at it some time ago.
>>>> And it looks like a plain oversight.
>>> no, it's still broken in 2.6.35 :(
>>> $ make ARCH=blackfin BF537-STAMP_defconfig
>>> $ make ARCH=blackfin -s -j4
>>> $ readelf -WS vmlinux | grep '\<data\>'
>>> [10] .data PROGBITS 0018369c 17269c 012964 00 WA 0 0 4
>>> [11] .data..shared_aligned PROGBITS 00196000 185000 00006c 00
>>> WA 0 0 4
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 030a954..e87260f 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -150,6 +150,7 @@
> #define DATA_DATA \
> *(.data) \
> *(.ref.data) \
> + *(.data..shared_aligned) /* percpu related */ \

Typo? Shouldn't that be ".data.shared_aligned"? Or where does the '..'
come from?

J

2010-08-26 07:20:18

by Mike Frysinger

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

On Thu, Aug 5, 2010 at 01:14, Jeremy Fitzhardinge wrote:
>  On 08/04/2010 01:40 PM, Sam Ravnborg wrote:
>> --- a/include/asm-generic/vmlinux.lds.h
>> +++ b/include/asm-generic/vmlinux.lds.h
>> @@ -150,6 +150,7 @@
>>  #define DATA_DATA                                                     \
>>        *(.data)                                                        \
>>        *(.ref.data)                                                    \
>> +       *(.data..shared_aligned) /* percpu related */                   \
>
> Typo?  Shouldn't that be ".data.shared_aligned"?  Or where does the '..'
> come from?

include/asm-generic/percpu.h:#define PER_CPU_SHARED_ALIGNED_SECTION
"..shared_aligned"
include/asm-generic/percpu.h:#define PER_CPU_ALIGNED_SECTION "..shared_aligned"
include/asm-generic/percpu.h:#define PER_CPU_ALIGNED_SECTION "..shared_aligned"
-mike
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2010-08-28 06:00:32

by Mike Frysinger

[permalink] [raw]
Subject: Re: missing .data.shared_align placement in vmlinux

On Wed, Aug 4, 2010 at 16:40, Sam Ravnborg wrote:
> [Resend - this time with Jeremy included in To: ]
>
> Jeremy - you introduced this in ("x86/i386: Put aligned
> stack-canary in percpu shared_aligned section")
> 53f824520b6d84ca5b4a8fd71addc91dbf64357e

can we get this merged for 2.6.36 ?
-mike