xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Campbell <ian.campbell@citrix.com>
To: xen-devel@lists.xen.org
Cc: julien.grall@citrix.com, tim@xen.org,
	Ian Campbell <ian.campbell@citrix.com>,
	stefano.stabellini@eu.citrix.com
Subject: [PATCH 3/3] xen: arm: retry trylock if strex fails on free lock.
Date: Fri, 19 Jul 2013 16:20:10 +0100	[thread overview]
Message-ID: <1374247210-20994-3-git-send-email-ian.campbell@citrix.com> (raw)
In-Reply-To: <1374247170.13645.100.camel@kazak.uk.xensource.com>

This comes from the Linux patches 15e7e5c1ebf5 for arm32 and 4ecf7ccb1973 for
arm64 by Will Deacon and Catalin Marinas respectively. The Linux commit message
says:

    An exclusive store instruction may fail for reasons other than lock
    contention (e.g. a cache eviction during the critical section) so, in
    line with other architectures using similar exclusive instructions
    (alpha, mips, powerpc), retry the trylock operation if the lock appears
    to be free but the strex reported failure.

I have observed this due to register_cpu_notifier containing:
    if ( !spin_trylock(&cpu_add_remove_lock) )
        BUG(); /* Should never fail as we are called only during boot. */
which was spuriously failing.

The ARMv8 variant is taken directly from the Linux patch. For v7 I had to
reimplement since we don't currently use ticket locks.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/include/asm-arm/arm32/spinlock.h |   25 ++++++++++++++-----------
 xen/include/asm-arm/arm64/spinlock.h |    3 ++-
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/xen/include/asm-arm/arm32/spinlock.h b/xen/include/asm-arm/arm32/spinlock.h
index 4a11a97..ba11ad6 100644
--- a/xen/include/asm-arm/arm32/spinlock.h
+++ b/xen/include/asm-arm/arm32/spinlock.h
@@ -34,17 +34,20 @@ static always_inline void _raw_spin_unlock(raw_spinlock_t *lock)
 
 static always_inline int _raw_spin_trylock(raw_spinlock_t *lock)
 {
-    unsigned long tmp;
-
-    __asm__ __volatile__(
-"   ldrex   %0, [%1]\n"
-"   teq     %0, #0\n"
-"   strexeq %0, %2, [%1]"
-    : "=&r" (tmp)
-    : "r" (&lock->lock), "r" (1)
-    : "cc");
-
-    if (tmp == 0) {
+    unsigned long contended, res;
+
+    do {
+        __asm__ __volatile__(
+    "   ldrex   %0, [%2]\n"
+    "   teq     %0, #0\n"
+    "   strexeq %1, %3, [%2]\n"
+    "   movne   %1, #0\n"
+        : "=&r" (contended), "=r" (res)
+        : "r" (&lock->lock), "r" (1)
+        : "cc");
+    } while (res);
+
+    if (!contended) {
         smp_mb();
         return 1;
     } else {
diff --git a/xen/include/asm-arm/arm64/spinlock.h b/xen/include/asm-arm/arm64/spinlock.h
index 717f2fe..3a36cfd 100644
--- a/xen/include/asm-arm/arm64/spinlock.h
+++ b/xen/include/asm-arm/arm64/spinlock.h
@@ -40,9 +40,10 @@ static always_inline int _raw_spin_trylock(raw_spinlock_t *lock)
     unsigned int tmp;
 
     asm volatile(
-        "       ldaxr   %w0, %1\n"
+        "2:     ldaxr   %w0, %1\n"
         "       cbnz    %w0, 1f\n"
         "       stxr    %w0, %w2, %1\n"
+        "       cbnz    %w0, 2b\n"
         "1:\n"
         : "=&r" (tmp), "+Q" (lock->lock)
         : "r" (1)
-- 
1.7.2.5

  parent reply	other threads:[~2013-07-19 15:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-19 15:19 [PATCH 0/3] xen: arm: update asm primitives (bitops, spinlocks, atomics) Ian Campbell
2013-07-19 15:20 ` [PATCH 1/3] xen/arm64: Assembly optimized bitops from Linux Ian Campbell
2013-07-19 15:20 ` [PATCH 2/3] xen/arm64: resync atomics and spinlock asm with Linux Ian Campbell
2013-07-29 16:02   ` Tim Deegan
2013-07-29 16:13     ` Ian Campbell
2013-07-29 18:05       ` Will Deacon
2013-07-30  9:34         ` Ian Campbell
2013-07-30  9:45           ` Will Deacon
2013-07-30  9:55             ` Ian Campbell
2013-07-30  9:59               ` Will Deacon
2013-07-30 10:12                 ` Ian Campbell
2013-07-19 15:20 ` Ian Campbell [this message]
2013-07-29 15:52   ` [PATCH 3/3] xen: arm: retry trylock if strex fails on free lock Tim Deegan
2013-07-29 16:20     ` Ian Campbell
2013-07-29 16:25       ` Tim Deegan
2013-08-22 14:56 ` [PATCH 0/3] xen: arm: update asm primitives (bitops, spinlocks, atomics) Ian Campbell

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=1374247210-20994-3-git-send-email-ian.campbell@citrix.com \
    --to=ian.campbell@citrix.com \
    --cc=julien.grall@citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.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).