[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Emilio G. Cota
Subject: Re: [Qemu-devel] [PATCH v5 07/18] qemu-thread: add simple test-and-set spinlock
Date: Tue, 17 May 2016 15:38:42 -0400
User-agent: Mutt/1.5.23 (2014-03-12)

On Tue, May 17, 2016 at 20:13:24 +0300, Sergey Fedorov wrote:
> On 14/05/16 06:34, Emilio G. Cota wrote:
> > +static inline void qemu_spin_lock(QemuSpin *spin)
> > +{
> > +    while (atomic_test_and_set_acquire(&spin->value)) {
> From gcc-4.8 info page, node "__atomic Builtins", description of
> __atomic_test_and_set():
>     It should be only used for operands of type 'bool' or 'char'.

Yes I'm aware of that. The way I interpret it is that if you're
storing something other than something ~= bool, you might
be in trouble, since it might get cleared.
We use 'int' but effectively store here a bool, so we're safe.

As to why we're using int, see

> > +        while (atomic_read(&spin->value)) {
> > +            cpu_relax();
> > +        }
> > +    }
> Looks like relaxed atomic access can be a subject to various
> optimisations according to
> https://gcc.gnu.org/wiki/Atomic/GCCMM/AtomicSync#Relaxed.

The important thing here is that the read actually happens
on every iteration; this is achieved with atomic_read().
Barriers etc. do not matter here because once we exit
the loop, the try to acquire the lock -- and if we succeed,
we then emit the right barrier.

> > +static inline bool qemu_spin_locked(QemuSpin *spin)
> > +{
> > +    return atomic_read_acquire(&spin->value);
> Why not just atomic_read()?

I think atomic_read() is better, yes. I'll change it. I went
with the fence because I wanted to have at least a caller
of atomic_read_acquire :P

I also hesitated between calling it _locked or _is_locked;
I used _locked for consistency with qemu_mutex_iothread_locked,
although I think _is_locked is a bit clearer:
   is a little too similar to



reply via email to

[Prev in Thread] Current Thread [Next in Thread]