On Tue, May 17, 2016 at 20:13:24 +0300, Sergey Fedorov wrote:
> On 14/05/16 06:34, Emilio G. Cota wrote:
(snip)
> > +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
http://thread.gmane.org/gmane.comp.emulators.qemu/405812/focus=405965
> > + 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:
qemu_spin_locked(foo)
is a little too similar to
qemu_spin_lock(foo).
Thanks,
Emilio
