Re: [Qemu-devel] [PATCH v3 05/11] qemu-thread: add simple test-and-set spinlock

[Emilio G. Cota]

On Wed, Apr 20, 2016 at 12:39:45 -0700, Richard Henderson wrote:
> On 04/20/2016 11:11 AM, Emilio G. Cota wrote:
> >On Wed, Apr 20, 2016 at 10:55:45 -0700, Richard Henderson wrote:
> >>On 04/20/2016 10:17 AM, Emilio G. Cota wrote:
(snip)
> >My comment was related to this:
> >
> >>[...] do note that there are compiler primitives for test-and-set that
> >>(can be) simpler for a cpu to implement than xchg.
> >
> >What compiler (I assume gcc) primitives are these? I couldn't find them.
> 
> __sync_lock_test_and_set and __atomic_test_and_set.
> 
> Both expand to ldstub on sparcv7, tas on coldfire, tas.b on sh.
> None of these are xchg operations.
> 
> I had forgotten that there wasn't a __sync_exchange builtin, so
> __sync_lock_test_and_set plays double-duty as both xchg and test-and-set.
> 
> But when __atomic builtins are available, __atomic_exchange does not fall
> back; you must use __atomic_test_and_set for less capable hosts.
> 
> But of course none of this is really relevant for normal hosts.

I see. Maybe we should do something like the appended, then?

                Emilio

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 5bc4d6c..6132dad 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -134,6 +134,7 @@
     })
 
 /* Provide shorter names for GCC atomic builtins, return old value */
+#define atomic_test_and_set(ptr) __atomic_test_and_set(ptr, __ATOMIC_SEQ_CST)
 #define atomic_fetch_inc(ptr)  __atomic_fetch_add(ptr, 1, __ATOMIC_SEQ_CST)
 #define atomic_fetch_dec(ptr)  __atomic_fetch_sub(ptr, 1, __ATOMIC_SEQ_CST)
 #define atomic_fetch_add(ptr, n) __atomic_fetch_add(ptr, n, __ATOMIC_SEQ_CST)
@@ -328,6 +329,7 @@
 #endif
 
 /* Provide shorter names for GCC atomic builtins.  */
+#define atomic_test_and_set(ptr) atomic_xchg(ptr, true)
 #define atomic_fetch_inc(ptr)  __sync_fetch_and_add(ptr, 1)
 #define atomic_fetch_dec(ptr)  __sync_fetch_and_add(ptr, -1)
 #define atomic_fetch_add       __sync_fetch_and_add
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index a216941..39ff1ac 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -74,7 +74,7 @@ static inline void qemu_spin_init(QemuSpin *spin)
 
 static inline void qemu_spin_lock(QemuSpin *spin)
 {
-    while (atomic_xchg(&spin->value, true)) {
+    while (atomic_test_and_set(&spin->value)) {
         while (atomic_read(&spin->value)) {
             cpu_relax();
         }
@@ -83,7 +83,7 @@ static inline void qemu_spin_lock(QemuSpin *spin)
 
 static inline int qemu_spin_trylock(QemuSpin *spin)
 {
-    if (atomic_xchg(&spin->value, true)) {
+    if (atomic_test_and_set(&spin->value)) {
         return -EBUSY;
     }
     return 0;