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

[Emilio G. Cota]

On Wed, Apr 20, 2016 at 08:18:56 -0700, Richard Henderson wrote:
> On 04/19/2016 04:07 PM, Emilio G. Cota wrote:
> >From: Guillaume Delbergue <address@hidden>
> >
> >Signed-off-by: Guillaume Delbergue <address@hidden>
> >[Rewritten. - Paolo]
> >Signed-off-by: Paolo Bonzini <address@hidden>
> >[Emilio's additions: call cpu_relax() while spinning; optimize for
> >  uncontended locks by acquiring the lock with xchg+test instead of
> >  test+xchg+test.]
> >Signed-off-by: Emilio G. Cota <address@hidden>
> >---
> 
> It probably doesn't matter for any real hosts, but do note that there are
> compiler primitives for test-and-set that (can be) simpler for a cpu to
> implement than xchg.  This likely affects only ancient hosts like sparcv7,
> or tiny hosts like SH.
> 
> We don't have to change anything here, but it does seem more natural to use
> a test-and-set primitive.

I've tried to find a GCC intrinsic for test-and-set, and I've only found
lock_test_and_set, which is what we use for atomic_xchg (except on ppc)
because it really is an atomic exchange:
 "This builtin, as described by Intel, is not a traditional test-and-set
  operation, but rather an atomic exchange operation."
 https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html

> >+static inline int qemu_spin_trylock(QemuSpin *spin)
> >+{
> >+    if (atomic_read(&spin->value) || atomic_xchg(&spin->value, true)) {
> >+        return -EBUSY;
> 
> I think there's no point in the extra read here.

Yep that's a remnant of the original "TATAS" implementation. Will
send a revised patch in a few minutes.

Thanks,

                Emilio