arm: Remaining issue with alignment of __log_buf in printk.c
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
Quoted 9 lines · Jonathan Cameron
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
Quoted 4 lines
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:
Quoted 5 lines
#if !defined(CONFIG_64BIT) ||
defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)#defineLOG_ALIGN4
#else
#defineLOG_ALIGN8
#endif
... uses the CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS for its intended purpose?
Russell, can you please comment here. Thanks.
Quoted 10 lines · Stephen Warren
Quoted 9 lines · Jonathan Cameron
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
Quoted 4 lines
Quoted 4 lines
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.
Quoted 22 lines
The first element in the structure type that's actually stored in the
__log_bufarray is a u64; see struct log inkernel/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:
Quoted 5 lines
#if !defined(CONFIG_64BIT) ||
defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)#defineLOG_ALIGN4
#else
#defineLOG_ALIGN8
#endif
Quoted 4 lines
... uses the
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESSfor 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.
Quoted 11 lines · Russell King - ARM Linux
Quoted 10 lines · Stephen Warren
Quoted 9 lines · Jonathan Cameron
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
Quoted 6 lines
Quoted 4 lines
Quoted 4 lines
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!
Quoted 29 lines
Quoted 3 lines
The first element in the structure type that's actually stored in the
__log_bufarray is a u64; see struct log inkernel/printk.c.Depending
Quoted 2 lines
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
Quoted 5 lines
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
Quoted 2 lines
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
Quoted 1 line
hence the struct as a whole, but Tegra (or my toolchain?) is OK with
the
Quoted 5 lines
older 4-byte alignment for a u64 or struct?
Further, I'm not sure if the following alignment selection logic:
Quoted 1 line
#if !defined(CONFIG_64BIT) ||
defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)Quoted 5 lines
Quoted 4 lines
#define
LOG_ALIGN4
#else
#defineLOG_ALIGN8
#endif
Quoted 6 lines
Quoted 4 lines
... uses the
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESSfor 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
Quoted 2 lines
this
should probably be fixed for EABI builds.
Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Quoted 14 lines · Stephen Warren
Quoted 13 lines · Jonathan Cameron
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 :f8450fca6ecdea38b5a882fdf6cd097e3ec8651cIncreasing 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.
...
Quoted 5 lines
#if !defined(CONFIG_64BIT) ||
defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)#defineLOG_ALIGN4
#else
#defineLOG_ALIGN8
#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.
Quoted 22 lines · Stephen Warren
Quoted 14 lines · Stephen Warren
Quoted 13 lines · Jonathan Cameron
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 :f8450fca6ecdea38b5a882fdf6cd097e3ec8651cIncreasing 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.
...
Quoted 5 lines
#if !defined(CONFIG_64BIT) ||
defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)#defineLOG_ALIGN4
#else
#defineLOG_ALIGN8
#endif
Quoted 7 lines
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
Quoted 31 lines · Kay Sievers
Quoted 22 lines · Stephen Warren
Quoted 14 lines · Stephen Warren
Quoted 13 lines · Jonathan Cameron
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 :f8450fca6ecdea38b5a882fdf6cd097e3ec8651cIncreasing 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.
...
Quoted 5 lines
#if !defined(CONFIG_64BIT) ||
defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)#defineLOG_ALIGN4
#else
#defineLOG_ALIGN8
#endifQuoted 7 lines
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.
Quoted 5 lines
__alignof__(u64)will be 8 onx86_64, while the current logic results in 4. Not sure ifx86_64would 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:
Quoted 5 lines
#if (
__alignof__(struct log_buf)== 4) ||defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)#defineLOG_ALIGN4
#else
#defineLOG_ALIGN8
#endif