Follow-up to remaining issue with alignment of __log_buf in printk.c

Follow-up to remaining issue with alignment of __log_buf in printk.c

[Andrew Lunn]

Hi Robert

GregKH has accepted my patch:

http://comments.gmane.org/gmane.linux.ports.arm.kernel/170442

So i hope it will be fixed in the next -rc kernel.

   Andrew

Follow-up to remaining issue with alignment of __log_buf in printk.c

[Robert Jarzmik]

Hi Stephen and others,

I have a XScale PXA based board with has the alignement issue which makes the
kernel trap during its early stage.

I wonder what is the status now, is there a fix available ?

I have tracked what happens on PXA. The pxa is an ARM v5TE chip. The new printk
version you submitted is translated to the following assembly on the line :
        msg->ts_nsec = local_clock();
Into:
=> 0xc001bbe0 <log_store+496>:	strd	r0, [r4, r5]

In ARMv5, the "strd" assembly opcode expects the address to be 64bits aligned,
hence the bug.

Now the solutions I have seen so far in the mailing lists :
 - #define LOG_ALIGN (__alignof__(u64))
   Does always work.
 - #define LOG_ALIGN (__alignof__(struct log))
   Doesn't work with my toolchain, as __alignof__(struct log) is 4, not 8

What are you intending to do to solve the ARMv5 issue ? Are you waiting for
someone to submit a patch ?

Cheers.

-- 
Robert

Follow-up to remaining issue with alignment of __log_buf in printk.c

[Stephen Warren]

On 06/14/2012 01:19 PM, Robert Jarzmik wrote:
> 
> Hi Stephen and others,
> 
> I have a XScale PXA based board with has the alignement issue which makes the
> kernel trap during its early stage.
> 
> I wonder what is the status now, is there a fix available ?
> 
> I have tracked what happens on PXA. The pxa is an ARM v5TE chip. The new printk
> version you submitted is translated to the following assembly on the line :
>         msg->ts_nsec = local_clock();
> Into:
> => 0xc001bbe0 <log_store+496>:	strd	r0, [r4, r5]
> 
> In ARMv5, the "strd" assembly opcode expects the address to be 64bits aligned,
> hence the bug.
> 
> Now the solutions I have seen so far in the mailing lists :
>  - #define LOG_ALIGN (__alignof__(u64))
>    Does always work.
>  - #define LOG_ALIGN (__alignof__(struct log))
>    Doesn't work with my toolchain, as __alignof__(struct log) is 4, not 8

Isn't that a bug in the toolchain; isn't the alignment of a struct
required to be the greatest alignment of any of its members? Otherwise,
this problem could arise with any usage of that struct.

I suppose this could be worked around with something like:

#define LOG_ALIGN max(__alignof__(struct log), __alignof__(u64))

Follow-up to remaining issue with alignment of __log_buf in printk.c

[Robert Jarzmik]

Stephen Warren <swarren@wwwdotorg.org> writes:

> Isn't that a bug in the toolchain; isn't the alignment of a struct
> required to be the greatest alignment of any of its members? Otherwise,
> this problem could arise with any usage of that struct.

I'm not a compiler expert, but I think EABI requires 64-bit data types to be
8-bytes aligned. If that assertion is true, then clearly my eabi compiler is a
little liar with its __alignof__(struct log) result.
And I think also that ISO C standard requires a structure to be aligned by
default to the <quote>least perfect multiple of the lowest common multiple of
the alignments of all the members of the struct</quote>. Which here means 8
bytes ...

> I suppose this could be worked around with something like:
> #define LOG_ALIGN max(__alignof__(struct log), __alignof__(u64))

Well, I wonder what is the correct approach here. In the end, the compiler
should be fixed ... But meanwhile, your proposition would solve the PXA
problem. The real deal is to know whether we want a compiler work-around in
printk.c ... and that's rather a policy issue than a technical one.

I wonder what others think about it ... let's wait for another pair of point of
views.

Cheers.

-- 
Robert

PS: For Andrew, I saw your patch. It won't solve my problem as my compiler
believes __alignof__(struct log) == 4 ... bad luck for me.

PPS: My toolchain is arm-2011.09

Follow-up to remaining issue with alignment of __log_buf in printk.c

[Andrew Lunn]

> PS: For Andrew, I saw your patch. It won't solve my problem as my compiler
> believes __alignof__(struct log) == 4 ... bad luck for me.
> 
> PPS: My toolchain is arm-2011.09

Hi Robert

I have gcc version 4.4.5 (Debian 4.4.5-8), both native and cross.

  Andrew