Hi All,
My stargate 2 board refused to start and after bisection I ended
up at the same patch that Stephen found an alignment issue in.
Unfortunately Stephen's patch doesn't seem to have fixed the
issue for me.
https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
Increasing the alignement for 32 bit systems to 8 seems to do the
job but I can't immediately think why...
System is a pxa27x strong arm.
Tree was effectively today's mainline.
Saddly I have too many other bits to fit in today for futher
investigation (hence this information free email!)
Jonathan
On 05/27/2012 06:39 AM, Jonathan Cameron wrote:
> Hi All,
>
> My stargate 2 board refused to start and after bisection I ended
> up at the same patch that Stephen found an alignment issue in.
> Unfortunately Stephen's patch doesn't seem to have fixed the
> issue for me.
> https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
> Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
>
> Increasing the alignement for 32 bit systems to 8 seems to do the
> job but I can't immediately think why...
>
> System is a pxa27x strong arm.
The first element in the structure type that's actually stored in the
__log_buf array is a u64; see struct log in kernel/printk.c. Depending
on alignment rules, a u64 and a struct containing it might require a
4-byte or 8-byte alignment. The following link implies this might have
changed over time:
http://wiki.debian.org/ArmEabiPort#Struct_packing_and_alignment
(see "64-bit data type alignment" a little below that anchor). I'm not
sure what ABI the kernel expects to use internally, or your compiler;
perhaps you need the new EABI 8-byte alignment requirement for a u64 and
hence the struct as a whole, but Tegra (or my toolchain?) is OK with the
older 4-byte alignment for a u64 or struct?
Further, I'm not sure if the following alignment selection logic:
> #if !defined(CONFIG_64BIT) || defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> #define LOG_ALIGN 4
> #else
> #define LOG_ALIGN 8
> #endif
... uses the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS for its intended
purpose?
Russell, can you please comment here. Thanks.
On Sun, May 27, 2012 at 10:03:20AM -0600, Stephen Warren wrote:
> On 05/27/2012 06:39 AM, Jonathan Cameron wrote:
> > Hi All,
> >
> > My stargate 2 board refused to start and after bisection I ended
> > up at the same patch that Stephen found an alignment issue in.
> > Unfortunately Stephen's patch doesn't seem to have fixed the
> > issue for me.
> > https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
> > Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
> >
> > Increasing the alignement for 32 bit systems to 8 seems to do the
> > job but I can't immediately think why...
> >
> > System is a pxa27x strong arm.
First thing to point out is that PXA27x is Xscale, not StrongARM.
> The first element in the structure type that's actually stored in the
> __log_buf array is a u64; see struct log in kernel/printk.c. Depending
> on alignment rules, a u64 and a struct containing it might require a
> 4-byte or 8-byte alignment. The following link implies this might have
> changed over time:
>
> http://wiki.debian.org/ArmEabiPort#Struct_packing_and_alignment
>
> (see "64-bit data type alignment" a little below that anchor). I'm not
> sure what ABI the kernel expects to use internally, or your compiler;
> perhaps you need the new EABI 8-byte alignment requirement for a u64 and
> hence the struct as a whole, but Tegra (or my toolchain?) is OK with the
> older 4-byte alignment for a u64 or struct?
>
> Further, I'm not sure if the following alignment selection logic:
>
> > #if !defined(CONFIG_64BIT) || defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> > #define LOG_ALIGN 4
> > #else
> > #define LOG_ALIGN 8
> > #endif
>
> ... uses the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS for its intended
> purpose?
>
> Russell, can you please comment here. Thanks.
And most likely it's using EABI which does want 8 byte alignment. So this
should probably be fixed for EABI builds.
Russell King - ARM Linux <[email protected]> wrote:
>On Sun, May 27, 2012 at 10:03:20AM -0600, Stephen Warren wrote:
>> On 05/27/2012 06:39 AM, Jonathan Cameron wrote:
>> > Hi All,
>> >
>> > My stargate 2 board refused to start and after bisection I ended
>> > up at the same patch that Stephen found an alignment issue in.
>> > Unfortunately Stephen's patch doesn't seem to have fixed the
>> > issue for me.
>> > https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
>> > Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
>> >
>> > Increasing the alignement for 32 bit systems to 8 seems to do the
>> > job but I can't immediately think why...
>> >
>> > System is a pxa27x strong arm.
>
>First thing to point out is that PXA27x is Xscale, not StrongARM.
Sorry my mistake!
>
>> The first element in the structure type that's actually stored in the
>> __log_buf array is a u64; see struct log in kernel/printk.c.
>Depending
>> on alignment rules, a u64 and a struct containing it might require a
>> 4-byte or 8-byte alignment. The following link implies this might
>have
>> changed over time:
>>
>> http://wiki.debian.org/ArmEabiPort#Struct_packing_and_alignment
>>
>> (see "64-bit data type alignment" a little below that anchor). I'm
>not
>> sure what ABI the kernel expects to use internally, or your compiler;
>> perhaps you need the new EABI 8-byte alignment requirement for a u64
>and
>> hence the struct as a whole, but Tegra (or my toolchain?) is OK with
>the
>> older 4-byte alignment for a u64 or struct?
>>
>> Further, I'm not sure if the following alignment selection logic:
>>
>> > #if !defined(CONFIG_64BIT) ||
>defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>> > #define LOG_ALIGN 4
>> > #else
>> > #define LOG_ALIGN 8
>> > #endif
>>
>> ... uses the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS for its intended
>> purpose?
>>
>> Russell, can you please comment here. Thanks.
>
>And most likely it's using EABI which does want 8 byte alignment.
Indeed I am using eabi.
So
>this
>should probably be fixed for EABI builds.
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
On 05/27/2012 10:03 AM, Stephen Warren wrote:
> On 05/27/2012 06:39 AM, Jonathan Cameron wrote:
>> Hi All,
>>
>> My stargate 2 board refused to start and after bisection I ended
>> up at the same patch that Stephen found an alignment issue in.
>> Unfortunately Stephen's patch doesn't seem to have fixed the
>> issue for me.
>> https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
>> Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
>>
>> Increasing the alignement for 32 bit systems to 8 seems to do the
>> job but I can't immediately think why...
>>
>> System is a pxa27x strong arm.
...
> #if !defined(CONFIG_64BIT) || defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> #define LOG_ALIGN 4
> #else
> #define LOG_ALIGN 8
> #endif
Actually, why not replace that with:
#define LOG_ALIGN (__alignof__(struct log_buf))
That way, the compiler will calculate the arch-/ABI-appropriate
alignment value automatically and correctly in all cases, so we won't
have to fix that ifdef above.
On Tue, May 29, 2012 at 6:14 PM, Stephen Warren <[email protected]> wrote:
> On 05/27/2012 10:03 AM, Stephen Warren wrote:
>> On 05/27/2012 06:39 AM, Jonathan Cameron wrote:
>>> Hi All,
>>>
>>> My stargate 2 board refused to start and after bisection I ended
>>> up at the same patch that Stephen found an alignment issue in.
>>> Unfortunately Stephen's patch doesn't seem to have fixed the
>>> issue for me.
>>> https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
>>> Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
>>>
>>> Increasing the alignement for 32 bit systems to 8 seems to do the
>>> job but I can't immediately think why...
>>>
>>> System is a pxa27x strong arm.
> ...
>> #if !defined(CONFIG_64BIT) || defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>> #define LOG_ALIGN 4
>> #else
>> #define LOG_ALIGN 8
>> #endif
>
> Actually, why not replace that with:
>
> #define LOG_ALIGN (__alignof__(struct log_buf))
>
> That way, the compiler will calculate the arch-/ABI-appropriate
> alignment value automatically and correctly in all cases, so we won't
> have to fix that ifdef above.
__alignof__(u64) will be 8 on x86_64, while the current logic results
in 4. Not sure if x86_64 would somehow benefit from that, or if it's
just a waste of bytes.
Are you sure it results in 4 on some architectures?
Kay
On 05/29/2012 10:32 AM, Kay Sievers wrote:
> On Tue, May 29, 2012 at 6:14 PM, Stephen Warren <[email protected]> wrote:
>> On 05/27/2012 10:03 AM, Stephen Warren wrote:
>>> On 05/27/2012 06:39 AM, Jonathan Cameron wrote:
>>>> Hi All,
>>>>
>>>> My stargate 2 board refused to start and after bisection I ended
>>>> up at the same patch that Stephen found an alignment issue in.
>>>> Unfortunately Stephen's patch doesn't seem to have fixed the
>>>> issue for me.
>>>> https://lkml.org/lkml/2012/5/10/510 is the thread. Patch from
>>>> Stephen is : f8450fca6ecdea38b5a882fdf6cd097e3ec8651c
>>>>
>>>> Increasing the alignement for 32 bit systems to 8 seems to do the
>>>> job but I can't immediately think why...
>>>>
>>>> System is a pxa27x strong arm.
>> ...
>>> #if !defined(CONFIG_64BIT) || defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>>> #define LOG_ALIGN 4
>>> #else
>>> #define LOG_ALIGN 8
>>> #endif
>>
>> Actually, why not replace that with:
>>
>> #define LOG_ALIGN (__alignof__(struct log_buf))
>>
>> That way, the compiler will calculate the arch-/ABI-appropriate
>> alignment value automatically and correctly in all cases, so we won't
>> have to fix that ifdef above.
>
> __alignof__(u64) will be 8 on x86_64, while the current logic results
> in 4. Not sure if x86_64 would somehow benefit from that, or if it's
> just a waste of bytes.
>
> Are you sure it results in 4 on some architectures?
I have no idea to be honest, but I'd tend towards making this reliable
first, and optimizing a few bytes later.
Perhaps something like the following would work though:
> #if (__alignof__(struct log_buf) == 4) || defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
> #define LOG_ALIGN 4
> #else
> #define LOG_ALIGN 8
> #endif