linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: catalin.marinas@arm.com (Catalin Marinas)
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:50:05 +0100	[thread overview]
Message-ID: <BANLkTim2TOZgB-3Vkv5RcK572Qr07s3=CA@mail.gmail.com> (raw)
In-Reply-To: <yw1xaaeanbxe.fsf@unicorn.mansr.com>

2011/5/25 M?ns Rullg?rd <mans@mansr.com>:
> 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.

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.

-- 
Catalin

  parent reply	other threads:[~2011-05-25 14:50 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-23 11:16 [PATCH] ARM: Do not allow unaligned accesses when CONFIG_ALIGNMENT_TRAP Catalin Marinas
2011-05-23 12:30 ` Måns Rullgård
2011-05-23 13:25   ` Russell King - ARM Linux
2011-05-23 13:21 ` Russell King - ARM Linux
2011-05-23 13:51   ` Catalin Marinas
2011-05-23 14:37     ` Måns Rullgård
2011-05-23 14:41       ` Catalin Marinas
2011-05-23 14:52       ` Russell King - ARM Linux
2011-05-24  9:39   ` Catalin Marinas
2011-05-24 14:17     ` Måns Rullgård
2011-05-24 15:26       ` Catalin Marinas
2011-05-24 16:23         ` Måns Rullgård
2011-05-24 17:26           ` Nicolas Pitre
2011-05-24 17:13         ` Dave Martin
2011-05-25 11:14           ` Catalin Marinas
2011-05-25 12:43             ` Dave Martin
2011-05-25 13:32               ` Måns Rullgård
2011-05-25 14:05                 ` Dave Martin
2011-05-25 14:48                   ` Måns Rullgård
2011-05-25 14:50                 ` Catalin Marinas [this message]
2011-05-25 14:53                   ` Måns Rullgård
2011-05-26 17:10                   ` Will Deacon
2011-05-26 18:14                     ` Måns Rullgård
2011-05-26 19:58                     ` Andi Kleen
2011-05-26 21:03                     ` Nicolas Pitre
2011-05-26 21:10                       ` Andi Kleen
2011-05-26 21:26                         ` Måns Rullgård
2011-05-27 10:05                         ` Will Deacon
2011-05-27 16:53                           ` Andi Kleen
2011-05-26 21:51                       ` Russell King - ARM Linux
2011-05-26 22:29                         ` Andi Kleen
2011-05-27  8:38                         ` Catalin Marinas
2011-05-27  8:54                           ` Russell King - ARM Linux
2011-05-27  9:51                             ` Catalin Marinas
2011-05-27  9:56                               ` Catalin Marinas
2011-05-27 12:46                               ` Måns Rullgård
2011-05-28 15:34                                 ` [PATCH] Disable -fconserve-stack on ARM Andi Kleen
2011-05-31 16:30                                   ` Catalin Marinas
2011-05-31 18:01                                     ` Andi Kleen
2011-06-02 13:08                                       ` Catalin Marinas
     [not found] <mailman.254.1306496353.1533.linux-arm-kernel@lists.infradead.org>
2011-05-27 12:14 ` [PATCH] ARM: Do not allow unaligned accesses when CONFIG_ALIGNMENT_TRAP Frank Hofmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='BANLkTim2TOZgB-3Vkv5RcK572Qr07s3=CA@mail.gmail.com' \
    --to=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).