Re: [Qemu-devel] [PATCH v5 07/18] qemu-thread: add simple test-and-set spinlock

[Paolo Bonzini]

On 18/05/2016 18:59, Emilio G. Cota wrote:
> On Wed, May 18, 2016 at 17:09:48 +0200, Paolo Bonzini wrote:
>> But honestly I think it would be even better to just use
>> __sync_lock_test_and_set in the spinlock implementation and not add this
>> to atomics.h.  There's already enough issues with the current subset of
>> atomics, I am not really happy to add non-SC read-modify-write
>> operations to the mix.
> 
> I can drop the two patches that touch atomic.h, and have the
> spinlock patch as appended.

Load acquire and store release are fine by me, I wanted to add them too.
 But the patch you attached is fine, I will salvage the
load-acquire/store-release patch later.

Thanks,

Paolo

> Thanks,
> 
>               Emilio
> 
> commit 99c8f1049a1508edc9de30e3cf27898888aa7f68
> Author: Guillaume Delbergue <address@hidden>
> Date:   Sun Oct 18 09:44:02 2015 +0200
> 
>     qemu-thread: add simple test-and-set spinlock
>     
>     Signed-off-by: Guillaume Delbergue <address@hidden>
>     [Rewritten. - Paolo]
>     Signed-off-by: Paolo Bonzini <address@hidden>
>     [Emilio's additions: use TAS instead of atomic_xchg; emit acquire/release
>      barriers; call cpu_relax() while spinning; optimize for uncontended 
> locks by
>      acquiring the lock with TAS instead of TATAS; add qemu_spin_locked().]
>     Signed-off-by: Emilio G. Cota <address@hidden>
> 
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index bdae6df..2d225ff 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -1,6 +1,9 @@
>  #ifndef __QEMU_THREAD_H
>  #define __QEMU_THREAD_H 1
>  
> +#include <errno.h>
> +#include "qemu/processor.h"
> +#include "qemu/atomic.h"
>  
>  typedef struct QemuMutex QemuMutex;
>  typedef struct QemuCond QemuCond;
> @@ -60,4 +63,40 @@ struct Notifier;
>  void qemu_thread_atexit_add(struct Notifier *notifier);
>  void qemu_thread_atexit_remove(struct Notifier *notifier);
>  
> +typedef struct QemuSpin {
> +    int value;
> +} QemuSpin;
> +
> +static inline void qemu_spin_init(QemuSpin *spin)
> +{
> +    __sync_lock_release(&spin->value);
> +}
> +
> +static inline void qemu_spin_lock(QemuSpin *spin)
> +{
> +    while (__sync_lock_test_and_set(&spin->value, true)) {
> +        while (atomic_read(&spin->value)) {
> +            cpu_relax();
> +        }
> +    }
> +}
> +
> +static inline int qemu_spin_trylock(QemuSpin *spin)
> +{
> +    if (__sync_lock_test_and_set(&spin->value, true)) {
> +        return -EBUSY;
> +    }
> +    return 0;
> +}
> +
> +static inline bool qemu_spin_locked(QemuSpin *spin)
> +{
> +    return atomic_read(&spin->value);
> +}
> +
> +static inline void qemu_spin_unlock(QemuSpin *spin)
> +{
> +    __sync_lock_release(&spin->value);
> +}
> +
>  #endif
>