[PATCH] ARM: Do not allow unaligned accesses when CONFIG_ALIGNMENT_TRAP

From: dave.martin@linaro.org (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: Do not allow unaligned accesses when CONFIG_ALIGNMENT_TRAP
Date: Wed, 25 May 2011 15:05:30 +0100	[thread overview]
Message-ID: <20110525140530.GB2340@arm.com> (raw)
In-Reply-To: <yw1xaaeanbxe.fsf@unicorn.mansr.com>

On Wed, May 25, 2011 at 02:32:13PM +0100, M?ns Rullg?rd wrote:
> Dave Martin <dave.martin@linaro.org> writes:
> 
> > On Wed, May 25, 2011 at 12:14:08PM +0100, Catalin Marinas wrote:
> >> On Tue, May 24, 2011 at 06:13:31PM +0100, Dave Martin wrote:
> >> > On Tue, May 24, 2011 at 04:26:35PM +0100, Catalin Marinas wrote:
> >> > > 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.
> >> 
> >> 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.
> >
> > 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.
> 
> > 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.

The answer might be yes or no; it depends on the compiler guys.

(By "normal C code" I exclude things like the __packed__ attribute,
inline asm, and and type-punning activities forbidden by the C language
standards.  By nature, these will affect specific parts of the kernel
rather than being universal.)

---Dave