[RFC PATCH] arm: asm/cmpxchg.h: Add support half-word xchg()

[Arnd Bergmann]

On Wednesday 25 February 2015 11:11:28 Pranith Kumar wrote:
> On Wed, Feb 25, 2015 at 10:58 AM, Arnd Bergmann <arnd at arndb.de> wrote:
> > On Wednesday 25 February 2015 10:36:20 Pranith Kumar wrote:
> >> This patch adds support for a half-word xchg() for ARM using ldrexh/strexh
> >> instructions. It also fixes an asm comment  for __cmpxchg2.
> >>
> >> Currently using a half-word xchg() results in the following splat on an ARMv7
> >> machine.
> >>
> >> [   45.833303] xchg: bad data size: pc 0xbe806020, ptr 0xeb18deee, size 2
> >> [   45.833324] ------------[ cut here ]------------
> >> [   45.837939] kernel BUG at /dvs/git/dirty/git-master_linux/kernel/arch/arm/kernel/traps.c:727!
> >>
> >> Signed-off-by: Pranith Kumar <bobby.prani at gmail.com>
> >
> > Unfortunately, the BUG message seems incomplete, can you reproduce this
> > with CONFIG_DEBUG_BUGVERBOSE enabled?
> 
> The bug here is in a module caused when xchg() was used on uint16_t
> variable. It is caused by the __bad_xchg() for 2 byte swap.
> 
> More information:
> [   45.833303] xchg: bad data size: pc 0xbe806020, ptr 0xeb18deee, size 2
> [   45.833324] ------------[ cut here ]------------
> [   45.837939] kernel BUG at
> /dvs/git/dirty/git-master_linux/kernel/arch/arm/kernel/traps.c:727!
> [   45.846450] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> [   45.852275] Modules linked in: test(O+) nvhost_vi
> [   45.857012] CPU: 0 PID: 1848 Comm: insmod Tainted: G           O
> 3.10.24-g6a2d13a #1
> [   45.864744] task: ee406580 ti: eb18c000 task.ti: eb18c000
> [   45.870141] PC is at __bad_xchg+0x24/0x28
> [   45.874146] LR is at __bad_xchg+0x24/0x28

I'm more interested in the backtrace here, it's possible we should fix the
driver instead.

> >>  arch/arm/include/asm/cmpxchg.h | 18 +++++++++++++++++-
> >>  1 file changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
> >> index abb2c37..9505cca 100644
> >> --- a/arch/arm/include/asm/cmpxchg.h
> >> +++ b/arch/arm/include/asm/cmpxchg.h
> >> @@ -50,6 +50,16 @@ static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int size
> >>                       : "r" (x), "r" (ptr)
> >>                       : "memory", "cc");
> >>               break;
> >> +     case 2:
> >> +             asm volatile("@ __xchg2\n"
> >> +             "1:     ldrexh  %0, [%3]\n"
> >> +             "       strexh  %1, %2, [%3]\n"
> >> +             "       teq     %1, #0\n"
> >> +             "       bne     1b"
> >> +                     : "=&r" (ret), "=&r" (tmp)
> >> +                     : "r" (x), "r" (ptr)
> >> +                     : "memory", "cc");
> >> +             break;
> >>       case 4:
> >>               asm volatile("@ __xchg4\n"
> >>               "1:     ldrex   %0, [%3]\n"
> >
> > Does this work on all ARMv6 or just ARMv6k?
> >
> 
> ldrexh/strexh is being used in cmpxchg() in the same file in a similar
> manner, and the comment there says that it works for all ARCH >=
> ARMv6k, so not ARMv6 I guess.

Ok, then you need to put the same check in __xchg too. I'm not sure
about the 1-byte case here, because that is already used in ARMv6
__xchg.

	Arnd