> From: Michael Hope <michael.h...@linaro.org>
> Date: Fri, 8 Jun 2012 05:50:52 +0200
> On 8 June 2012 15:20, Hans-Peter Nilsson <hans-peter.nils...@axis.com> wrote:
> > So the default for ALIGNMENT_TRAP changed in >3.1?
>
> ALIGNMENT_TRAP is on by default but the early boot time trap is now
> conditional on the architecture level:
>
> __enable_mmu:
> #if defined(CONFIG_ALIGNMENT_TRAP) && __LINUX_ARM_ARCH__ < 6
> orr r0, r0, #CR_A
> #else
> bic r0, r0, #CR_A
> #endif
Ah, interesting.
> >> I don't agree that unaligned access is a sign of wrong code. It's
> >> very common when poking about in protocol buffers. Using the hardware
> >> support is better than the multi-byte read plus OR that GCC used to
> >> do.
> >
> > Totally disagree. Code that e.g. casts unaligned char * to int *
> > and dereferences that, is crappy and unportable and fails
> > without OS-configury choice on some platforms, and is also
> > avoidable. But hopefully that wasn't what you meant.
>
> I mean direct access via helpers:
>
> inline u16 get_u16(const char *p) {
> #ifdef __ARM_FEATURE_UNALIGNED
> return ntohs(*(const u16 *)p);
> #else
> ...
> }
> ...
> u16 checksum = get_u16(buffer + CHECKSUM_OFFSET);
*shrug* I see no relation to the "certain classes of bad codes"
I allude to; i.e. brute unaligned accesses without portability
checks. The above code IIUC is in Linux, that also has control
over, and enabled the unaligned access mode in the code you
quoted, so it certainly knows it's safe!
> >> Where else have you seen this?
> >
> > I don't get the "else". I've seen the unaligned accesses from
> > userland code as quoted above. There were other (userland)
> > places in that build-tree that I'm not going to check, as this
> > was (again) GCC-generated code. There were no other misaligned
> > accesses on that system; not from the kernel, not from userspace.
>
> You're suggesting we disable an optimisation.
Tongue-in-cheek: I'm not really sure it is an "optimization";
for that claim we still have to see numbers. ;)
> The combination of
> older Linux ARM kernels and GCC 4.7 gives a faulty kernel.
We're in agreement!
> In all
> other cases it should be an improvement. Have you seen other
> combinations where there's a fault when this feature is turned on?
Nope, I think this one is bad enough.
It doesn't seem far-fetched that other non-Linux-kernels have
the trap on (maybe even knowingly and deliberately, for debug
reasons) and whose users are surprised by the new behavior.
Maybe they want to change their kernels, maybe not. With the
updated changes.html they can at least see that this is expected
behavior without digging in too deeply. The decision to
document it (as per the patch) or revert it (my humble
preference) might well be helped by performance numbers.
You're implying this change is an optimization that helps more
than it hurts; have you seen any such numbers? 1/2 :-)
brgds, H-P
