ARM: Do not allow unaligned accesses when CONFIG_ALIGNMENT_TRAP

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Wouldn't it make more sense to disable strict alignment checking in kernel mode regardless of this option. I can't imagine why it would ever be desirable. IMO the usermode default should also be changed to allow unaligned accesses on ARMv6 and up.

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

This description doesn't make sense. If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? What's the panic/oops look like? And that information should be in the commit description _anyway_.

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? What's the panic/oops look like? And that information should be in the commit description _anyway_.

OK, not clear from my commit log. See below for details (I'll add them to the log):

http://www.codesourcery.com/archives/arm-gnu/msg04202.html

Basically the fault happens before we call alignment_init(). There are other ways to fix this issue (like initialising the alignment handler earlier).

Quoted 20 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. ?If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? ?What's the panic/oops look like? ?And that information should be in the commit description _anyway_.

OK, not clear from my commit log. See below for details (I'll add them to the log):

http://www.codesourcery.com/archives/arm-gnu/msg04202.html

Quoted 3 lines

Basically the fault happens before we call alignment_init(). There are other ways to fix this issue (like initialising the alignment handler earlier).

The reset value of SCTLR.A is 0 on ARMv7. ?Something must be setting it to 1 early, before the alignment trap handler is initialised. ?This something is what needs to be changed.

Quoted 20 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? What's the panic/oops look like? And that information should be in the commit description _anyway_.

OK, not clear from my commit log. See below for details (I'll add them to the log):

http://www.codesourcery.com/archives/arm-gnu/msg04202.html

Quoted 3 lines

Basically the fault happens before we call alignment_init(). There are other ways to fix this issue (like initialising the alignment handler earlier).

The reset value of SCTLR.A is 0 on ARMv7. Something must be setting it to 1 early, before the alignment trap handler is initialised. This something is what needs to be changed.

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

This description doesn't make sense. If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? What's the panic/oops look like? And that information should be in the commit description _anyway_.

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? What's the panic/oops look like? And that information should be in the commit description _anyway_.

Does the patch below look better?

We cannot move alignment_init() earlier as we don't know how early the compiler would generate unaligned accesses. An alternative is some #ifdef's in head.S. Please let me know which variant you prefer.

Quoted 17 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. ?If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? ?What's the panic/oops look like? ?And that information should be in the commit description _anyway_.

Does the patch below look better?

Quoted 3 lines

We cannot move alignment_init() earlier as we don't know how early the compiler would generate unaligned accesses. An alternative is some #ifdef's in head.S. Please let me know which variant you prefer.

ifdefs may be ugly, but I don't see a better solution here. ?Crippling the entire build to make a couple of lines slightly more aesthetically pleasing doesn't seem right to me.

2011/5/24 M?ns Rullg?rd <[email protected]>:

Quoted 23 lines

Quoted 20 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. ?If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? ?What's the panic/oops look like? ?And that information should be in the commit description _anyway_.

Does the patch below look better?

We cannot move alignment_init() earlier as we don't know how early the compiler would generate unaligned accesses. An alternative is some #ifdef's in head.S. Please let me know which variant you prefer.

Quoted 3 lines

ifdefs may be ugly, but I don't see a better solution here. ?Crippling the entire build to make a couple of lines slightly more aesthetically pleasing doesn't seem right to me.

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

Quoted 24 lines · Catalin Marinas

2011/5/24 M?ns Rullg?rd <[email protected]>:

Quoted 23 lines

Quoted 20 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. ?If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? ?What's the panic/oops look like? ?And that information should be in the commit description _anyway_.

Does the patch below look better?

We cannot move alignment_init() earlier as we don't know how early the compiler would generate unaligned accesses. An alternative is some #ifdef's in head.S. Please let me know which variant you prefer.

Quoted 7 lines

Quoted 3 lines

ifdefs may be ugly, but I don't see a better solution here. ?Crippling the entire build to make a couple of lines slightly more aesthetically pleasing doesn't seem right to me.

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

On Cortex-A8 unaligned loads not crossing a 128-bit boundary have no penalty. Crossing a 128-bit boundary stalls for 9 cycles (measured, 8 according to TRM). On Cortex-A9, unaligned loads crossing a 64-bit boundary delay 7 cycles (measured), other misalignments have no penalty. On average these timings are faster than doing 4 byte loads and oring them together, which uses 6 cycles on A9, 9 cycles on A8.

It may of course be the case that unaligned accesses are rare enough that this isn't worth worrying about at all.

2011/5/24 M?ns Rullg?rd <[email protected]>:

Quoted 23 lines

Quoted 20 lines · Catalin Marinas

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Catalin Marinas

Newer versions of gcc generate unaligned accesses by default, causing kernel panics when CONFIG_ALIGNMENT_TRAP is enabled. This patch adds the -mno-unaligned-access option to gcc.

Quoted 8 lines

This description doesn't make sense. ?If we have alignment traps enabled, then we _expect_ to fix up unaligned loads and stores.

Therefore there should be no panic if CONFIG_ALIGNMENT_TRAP is enabled.

So what's the _actual_ problem that this is trying to address? ?What's the panic/oops look like? ?And that information should be in the commit description _anyway_.

Does the patch below look better?

We cannot move alignment_init() earlier as we don't know how early the compiler would generate unaligned accesses. An alternative is some #ifdef's in head.S. Please let me know which variant you prefer.

Quoted 3 lines

ifdefs may be ugly, but I don't see a better solution here. ?Crippling the entire build to make a couple of lines slightly more aesthetically pleasing doesn't seem right to me.

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

	char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:

c0082ba0:   e92d4ff0    push    {r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4:   e3074118    movw    r4, #28952  ; 0x7118
c0082ba8:   e24dd04c    sub sp, sp, #76 ; 0x4c
c0082bac:   e34c402a    movt    r4, #49194  ; 0xc02a
c0082bb0:   e58d1014    str r1, [sp, #20]
c0082bb4:   e58d0020    str r0, [sp, #32]
c0082bb8:   e8b40003    ldm r4!, {r0, r1}
c0082bbc:   e58d003f    str r0, [sp, #63]   <----------- !!!!!
c0082bc0:   e59d0014    ldr r0, [sp, #20]
c0082bc4:   e5d43000    ldrb    r3, [r4]

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

Quoted 35 lines · Catalin Marinas

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

	char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:

c0082ba0:   e92d4ff0    push    {r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4:   e3074118    movw    r4, #28952  ; 0x7118
c0082ba8:   e24dd04c    sub sp, sp, #76 ; 0x4c
c0082bac:   e34c402a    movt    r4, #49194  ; 0xc02a
c0082bb0:   e58d1014    str r1, [sp, #20]
c0082bb4:   e58d0020    str r0, [sp, #32]
c0082bb8:   e8b40003    ldm r4!, {r0, r1}
c0082bbc:   e58d003f    str r0, [sp, #63]   <----------- !!!!!
c0082bc0:   e59d0014    ldr r0, [sp, #20]
c0082bc4:   e5d43000    ldrb    r3, [r4]

Quoted 2 lines

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

So, it looks like empty_str may be misaligned on the stack, and the compiler is doing a misaligned store when initialising it.

Since the unaligned access support stuff is new, I'm suspicious of a compiler bug here... Can you follow up with your friendly neighbourhood tools guys?

Quoted 36 lines · Dave Martin

Quoted 35 lines · Catalin Marinas

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

	char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:

c0082ba0:   e92d4ff0    push    {r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4:   e3074118    movw    r4, #28952  ; 0x7118
c0082ba8:   e24dd04c    sub sp, sp, #76 ; 0x4c
c0082bac:   e34c402a    movt    r4, #49194  ; 0xc02a
c0082bb0:   e58d1014    str r1, [sp, #20]
c0082bb4:   e58d0020    str r0, [sp, #32]
c0082bb8:   e8b40003    ldm r4!, {r0, r1}
c0082bbc:   e58d003f    str r0, [sp, #63]   <----------- !!!!!
c0082bc0:   e59d0014    ldr r0, [sp, #20]
c0082bc4:   e5d43000    ldrb    r3, [r4]

Quoted 5 lines

Quoted 2 lines

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

So, it looks like empty_str may be misaligned on the stack, and the compiler is doing a misaligned store when initialising it.

empty_str has type char[] so there are no alignment requirements.

Quoted 3 lines

Since the unaligned access support stuff is new, I'm suspicious of a compiler bug here... Can you follow up with your friendly neighbourhood tools guys?

The compiler might be doing something unintentional, but I see no violation of any rules here.

Quoted 44 lines · M?ns Rullg?rd

Quoted 36 lines · Dave Martin

Quoted 35 lines · Catalin Marinas

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

	char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:

c0082ba0:   e92d4ff0    push    {r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4:   e3074118    movw    r4, #28952  ; 0x7118
c0082ba8:   e24dd04c    sub sp, sp, #76 ; 0x4c
c0082bac:   e34c402a    movt    r4, #49194  ; 0xc02a
c0082bb0:   e58d1014    str r1, [sp, #20]
c0082bb4:   e58d0020    str r0, [sp, #32]
c0082bb8:   e8b40003    ldm r4!, {r0, r1}
c0082bbc:   e58d003f    str r0, [sp, #63]   <----------- !!!!!
c0082bc0:   e59d0014    ldr r0, [sp, #20]
c0082bc4:   e5d43000    ldrb    r3, [r4]

Quoted 5 lines

Quoted 2 lines

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

So, it looks like empty_str may be misaligned on the stack, and the compiler is doing a misaligned store when initialising it.

Quoted 8 lines

empty_str has type char[] so there are no alignment requirements.

Quoted 3 lines

Since the unaligned access support stuff is new, I'm suspicious of a compiler bug here... Can you follow up with your friendly neighbourhood tools guys?

The compiler might be doing something unintentional, but I see no violation of any rules here.

Indeed -- mainly I'm wondering if this specific problem (unaligned accesses generated by the compiler for normal C code) is one we will ever need to deal with for kernel code. I'd guess that the performance of unaligned accesses will always be at least slightly suboptimal in practice, so the compiler guys may well not intend to misalign data just for the sake of it.

Quoted 36 lines · Dave Martin

Quoted 35 lines · Catalin Marinas

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

? ? ?char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:
c0082ba0: ? e92d4ff0 ? ?push ? ?{r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4: ? e3074118 ? ?movw ? ?r4, #28952 ?; 0x7118
c0082ba8: ? e24dd04c ? ?sub sp, sp, #76 ; 0x4c
c0082bac: ? e34c402a ? ?movt ? ?r4, #49194 ?; 0xc02a
c0082bb0: ? e58d1014 ? ?str r1, [sp, #20]
c0082bb4: ? e58d0020 ? ?str r0, [sp, #32]
c0082bb8: ? e8b40003 ? ?ldm r4!, {r0, r1}
c0082bbc: ? e58d003f ? ?str r0, [sp, #63] ? <----------- !!!!!
c0082bc0: ? e59d0014 ? ?ldr r0, [sp, #20]
c0082bc4: ? e5d43000 ? ?ldrb ? ?r3, [r4]

Quoted 5 lines

Quoted 2 lines

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

So, it looks like empty_str may be misaligned on the stack, and the compiler is doing a misaligned store when initialising it.

empty_str has type char[] so there are no alignment requirements.

2011/5/25 M?ns Rullg?rd <[email protected]>:

Quoted 41 lines

Quoted 39 lines · Dave Martin

Quoted 37 lines · Catalin Marinas

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

? ? ?char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:
c0082ba0: ? e92d4ff0 ? ?push ? ?{r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4: ? e3074118 ? ?movw ? ?r4, #28952 ?; 0x7118
c0082ba8: ? e24dd04c ? ?sub sp, sp, #76 ; 0x4c
c0082bac: ? e34c402a ? ?movt ? ?r4, #49194 ?; 0xc02a
c0082bb0: ? e58d1014 ? ?str r1, [sp, #20]
c0082bb4: ? e58d0020 ? ?str r0, [sp, #32]
c0082bb8: ? e8b40003 ? ?ldm r4!, {r0, r1}
c0082bbc: ? e58d003f ? ?str r0, [sp, #63] ? <----------- !!!!!
c0082bc0: ? e59d0014 ? ?ldr r0, [sp, #20]
c0082bc4: ? e5d43000 ? ?ldrb ? ?r3, [r4]

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

Quoted 4 lines

Quoted 2 lines

So, it looks like empty_str may be misaligned on the stack, and the compiler is doing a misaligned store when initialising it.

empty_str has type char[] so there are no alignment requirements.

I think the local variables after char empty_str[] are unaligned (int alloc).

2011/5/25 M?ns Rullg?rd <[email protected]>:

Quoted 44 lines

Quoted 41 lines · Dave Martin

Quoted 35 lines · Catalin Marinas

Quoted 7 lines · Dave Martin

Quoted 3 lines · Catalin Marinas

BTW, are we sure that the code generated with unaligned accesses is better? AFAIK, while processors support unaligned accesses, they may not always be optimal.

The code gcc generates to synthesise an unaligned access using aligned accesses is pretty simplistic:

...

Quoted 6 lines

For code which natively needs to read unaligned fields from data structures, I sincerely doubt that the CPU will not beat the above code for efficiency...

So if there's code doing unaligned access to data structures for a good reason, building with unaligned access support turned on in the compiler seems a good idea, if that code might performance-critical for anything.

gcc generates unaligned accesses in the the pcpu_dump_alloc_info() function. We have a local variable like below (9 bytes):

     char empty_str[] = "--------";

and it looks like other stack accesses are unaligned:

c0082ba0 <pcpu_dump_alloc_info>:

c0082ba0:   e92d4ff0    push    {r4, r5, r6, r7, r8, r9, sl, fp, lr}
c0082ba4:   e3074118    movw    r4, #28952  ; 0x7118
c0082ba8:   e24dd04c    sub sp, sp, #76 ; 0x4c
c0082bac:   e34c402a    movt    r4, #49194  ; 0xc02a
c0082bb0:   e58d1014    str r1, [sp, #20]
c0082bb4:   e58d0020    str r0, [sp, #32]
c0082bb8:   e8b40003    ldm r4!, {r0, r1}
c0082bbc:   e58d003f    str r0, [sp, #63]   <----------- !!!!!
c0082bc0:   e59d0014    ldr r0, [sp, #20]
c0082bc4:   e5d43000    ldrb    r3, [r4]

Quoted 2 lines

I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.

So, it looks like empty_str may be misaligned on the stack, and the compiler is doing a misaligned store when initialising it.

Quoted 1 line

empty_str has type char[] so there are no alignment requirements.

I think the local variables after char empty_str[] are unaligned (int alloc). Changing the array size to 16 solves the issue.

The gcc guys here in ARM will have a look and I'll get back to you.

This issue seems to be caused by passing -fconserve-stack to GCC. This was added in 8f7f5c9f ("kbuild: set -fconserve-stack option for gcc 4.5") and as you can see from the archive:

http://lkml.org/lkml/2009/9/20/39

it was thought to only have an impact on inlining decisions. Looking at the documentation for GCC 4.6:

-fconserve-stack
Attempt to minimize stack usage. The compiler will attempt to
use less stack space, even if that makes the program slower. This option implies setting the ?large-stack-frame? parameter to 100 and the ?large-stack-frame-growth? parameter to 400.

So it sounds like we might not want to enable this blindly across all architectures. Indeed, on ARM, it encourages the compiler to pack variables on the stack which leads to the weird and wonderful alignment situation that has been encountered in this thread.

Can we remove -fconserve-stack from the top-level Makefile (or at least make it conditional by architecture)?

So it sounds like we might not want to enable this blindly across all architectures. Indeed, on ARM, it encourages the compiler to pack variables on the stack which leads to the weird and wonderful alignment situation that has been encountered in this thread.

This issue seems to be caused by passing -fconserve-stack to GCC. This was added in 8f7f5c9f ("kbuild: set -fconserve-stack option for gcc 4.5") and as you can see from the archive:

http://lkml.org/lkml/2009/9/20/39

it was thought to only have an impact on inlining decisions. Looking at the documentation for GCC 4.6:

-fconserve-stack
Attempt to minimize stack usage. The compiler will attempt to
use less stack space, even if that makes the program slower. This option implies setting the ?large-stack-frame? parameter to 100 and the ?large-stack-frame-growth? parameter to 400.

So it sounds like we might not want to enable this blindly across all architectures. Indeed, on ARM, it encourages the compiler to pack variables on the stack which leads to the weird and wonderful alignment situation that has been encountered in this thread.

Can we remove -fconserve-stack from the top-level Makefile (or at least make it conditional by architecture)?

It is possible that -fconserve-stack is still valuable on ARM given that it is also used with -mno-unaligned-access for other things than structure packing on the stack, and therefore its merits can be debated independently from the alignment issue at hand.

Quoted 4 lines

It is possible that -fconserve-stack is still valuable on ARM given that it is also used with -mno-unaligned-access for other things than structure packing on the stack, and therefore its merits can be debated independently from the alignment issue at hand.

The big advantage of -fconserve-stack is that it throttles the inliner if the inlining would cause too much stack growth. This is something you likely want on ARM too, especially as code gets more and more complex.

Quoted 5 lines

It is possible that -fconserve-stack is still valuable on ARM given that it is also used with -mno-unaligned-access for other things than structure packing on the stack, and therefore its merits can be debated independently from the alignment issue at hand.

The big advantage of -fconserve-stack is that it throttles the inliner if the inlining
would cause too much stack growth. This is something you likely want on ARM too, especially as code gets more and more complex.

From private conversation, the GCC guys don't seem to think this is a

Do you have any concrete examples of -fconserve-stack giving an overall win that isn't in the noise? The fact that the GCC documentation explicitly states that enabling the option can lead to `making the program slower' does make me question why we're enabling it in the first place.

It is possible that -fconserve-stack is still valuable on ARM given that it is also used with -mno-unaligned-access for other things than structure packing on the stack, and therefore its merits can be debated independently from the alignment issue at hand.

Catalin said in his mail "I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.". So I don't think we know enough to say whether -mno-unaligned-access avoids the stack packing.

Quoted 4 lines · Nicolas Pitre

It is possible that -fconserve-stack is still valuable on ARM given that it is also used with -mno-unaligned-access for other things than structure packing on the stack, and therefore its merits can be debated independently from the alignment issue at hand.

Catalin said in his mail "I haven't tried with -mno-unaligned-access, I suspect the variables on the stack would be aligned.". So I don't think we know enough to say whether -mno-unaligned-access avoids the stack packing.

OK, I tried this now:

-fconserve-stack: we get unaligned accesses on the stack because the newer versions of gcc turned unaligned accesses on by default.

-fconserve-stack -mno-unaligned-access: the stack variables are aligned. We probably get the benefit of -fconserve-stack as well.

So as per the initial post in this thread, we could have -mno-unaligned-access on ARM always on (when CONFIG_ALIGNMENT_TRAP). As Nicolas suggested, we could compile some files with -munaligned-access (and maybe -fno-conserve-stack).

I raised this with the gcc guys so they are looking into it. But it really doesn't look like a gcc bug as long as -mno-unaligned-access is taken into account.

Quoted 13 lines · Catalin Marinas

OK, I tried this now:

-fconserve-stack: we get unaligned accesses on the stack because the newer versions of gcc turned unaligned accesses on by default.

-fconserve-stack -mno-unaligned-access: the stack variables are aligned. We probably get the benefit of -fconserve-stack as well.

So as per the initial post in this thread, we could have -mno-unaligned-access on ARM always on (when CONFIG_ALIGNMENT_TRAP). As Nicolas suggested, we could compile some files with -munaligned-access (and maybe -fno-conserve-stack).

Quoted 3 lines

I raised this with the gcc guys so they are looking into it. But it really doesn't look like a gcc bug as long as -mno-unaligned-access is taken into account.

Ok, we need to check one last thing, and that's what the behaviour is with -mno-unaligned-access and packed structures (such as the ethernet header). If it makes no difference, then I suggest we always build with -mno-unaligned-access.

Quoted 14 lines · Russell King - ARM Linux

Quoted 13 lines · Catalin Marinas

OK, I tried this now:

-fconserve-stack: we get unaligned accesses on the stack because the newer versions of gcc turned unaligned accesses on by default.

-fconserve-stack -mno-unaligned-access: the stack variables are aligned. We probably get the benefit of -fconserve-stack as well.

So as per the initial post in this thread, we could have -mno-unaligned-access on ARM always on (when CONFIG_ALIGNMENT_TRAP). As Nicolas suggested, we could compile some files with -munaligned-access (and maybe -fno-conserve-stack).

Quoted 8 lines

Quoted 3 lines

I raised this with the gcc guys so they are looking into it. But it really doesn't look like a gcc bug as long as -mno-unaligned-access is taken into account.

Ok, we need to check one last thing, and that's what the behaviour is with -mno-unaligned-access and packed structures (such as the ethernet header). If it makes no difference, then I suggest we always build with -mno-unaligned-access.

I tried some simple code below:

struct test {
	unsigned char a[6];
	unsigned long b;
} __attribute__((packed));

void set(struct test *t, unsigned long v)

{
	t->b = v;
}

int main(void)

{
	struct test t;
	
	set(&t, 10);

	return 0;
}

With -mno-unaligned-access in newer toolchains, the set() function looks like this (compiled with -march=armv7):

00000000 <set>:

   0:   e7e7c451        ubfx    ip, r1, #8, #8
   4:   e7e72851        ubfx    r2, r1, #16, #8
   8:   e1a03c21        lsr     r3, r1, #24
   c:   e5c01006        strb    r1, [r0, #6]
  10:   e5c0c007        strb    ip, [r0, #7]
  14:   e5c02008        strb    r2, [r0, #8]
  18:   e5c03009        strb    r3, [r0, #9]
  1c:   e12fff1e        bx      lr

If I don't pass -mno-unaligned-access later toolchains use unaligned accesses by default and the set() function is more efficient:

00000000 <set>:

   0:   e5801006        str     r1, [r0, #6]
   4:   e12fff1e        bx      lr

Quoted 5 lines · Russell King - ARM Linux

Ok, we need to check one last thing, and that's what the behaviour is with -mno-unaligned-access and packed structures (such as the ethernet header). If it makes no difference, then I suggest we always build with -mno-unaligned-access.

I tried some simple code below:

struct test {
	unsigned char a[6];
	unsigned long b;
} __attribute__((packed));

void set(struct test *t, unsigned long v)

{
	t->b = v;
}

int main(void)

{
	struct test t;
	
	set(&t, 10);

	return 0;
}

With -mno-unaligned-access in newer toolchains, the set() function looks like this (compiled with -march=armv7):

00000000 <set>:

   0:   e7e7c451        ubfx    ip, r1, #8, #8
   4:   e7e72851        ubfx    r2, r1, #16, #8
   8:   e1a03c21        lsr     r3, r1, #24
   c:   e5c01006        strb    r1, [r0, #6]
  10:   e5c0c007        strb    ip, [r0, #7]
  14:   e5c02008        strb    r2, [r0, #8]
  18:   e5c03009        strb    r3, [r0, #9]
  1c:   e12fff1e        bx      lr

If I don't pass -mno-unaligned-access later toolchains use unaligned accesses by default and the set() function is more efficient:

00000000 <set>:

   0:   e5801006        str     r1, [r0, #6]
   4:   e12fff1e        bx      lr

The problem is that in addition to that we also get unaligned stack variables which are not really efficient. Either way we have a drawback somewhere. We could argue that -fconserve-stack is badly implemented on ARM.

Disable -fconserve-stack on ARM

There are reports that -fconserve-stack misaligns variables on the stack. Disable it for ARM to work around this gcc bug.

Signed-off-by Andi Kleen <[email protected]>

arch/arm/Makefile +0 -0

index 25750bc..0883cff 100644

Didn't work, on the gcc command line we get -fno-conserve-stack followed by -fconserve-stack from the top makefile and it seems that the latter takes priority.

Disable -fconserve-stack on ARM v2

There are reports that -fconserve-stack misaligns variables on the stack. Disable it for ARM to work around this gcc bug.

v2: Move top level flags definition up

Signed-off-by Andi Kleen <[email protected]>