On 21/09/20 15:44, Stefan Hajnoczi wrote: > On Mon, Sep 21, 2020 at 01:15:30PM +0200, Paolo Bonzini wrote: >> On 21/09/20 12:41, Stefan Hajnoczi wrote: >>> The upshot is that all atomic variables in QEMU need to use C11 Atomic >>> atomic_* types. This is a big change! >> >> The main issue with this is that C11 atomic types do too much magic, >> including defaulting to seq-cst operations for loads and stores. As >> documented in docs/devel/atomics.rst, seq-cst loads and stores are >> almost never what you want (they're only a little below volatile :)): > > I can't find where atomics.rst says seq-cst operations are almost never > what you want?
Note that I'm talking only about loads and stores. They are so much
never what you want that we don't even provide a wrapper for them in
qemu/atomic.h.
``qemu/atomic.h`` also provides loads and stores that cannot be
reordered with each other::
typeof(*ptr) atomic_mb_read(ptr)
void atomic_mb_set(ptr, val)
However these do not provide sequential consistency and, in
particular, they do not participate in the total ordering enforced by
sequentially-consistent operations. For this reason they are
deprecated. They should instead be replaced with any of the following
(ordered from easiest to hardest):
- accesses inside a mutex or spinlock
- lightweight synchronization primitives such as ``QemuEvent``
- RCU operations (``atomic_rcu_read``, ``atomic_rcu_set``) when
publishing or accessing a new version of a data structure
- other atomic accesses: ``atomic_read`` and ``atomic_load_acquire``
for loads, ``atomic_set`` and ``atomic_store_release`` for stores,
``smp_mb`` to forbid reordering subsequent loads before a store.
where seq-cst loads and stores are again completely missing. smp_mb is
there to replace them, as we did in commit 5710a3e0 ("async: use
explicit memory barriers").
>> we can use store-release/load-acquire
>
> They don't provide atomic arithmetic/logic operations. The only
> non-seq-cst ALU operation I see in atomic.h is
> atomic_fetch_inc_nonzero(), and it's a cmpxchg() loop (ugly compared to
> an atomic ALU instruction).
Seq-cst is fine for RMW operations (arithmetic/logic, xchg, cmpxchg),
also because they're usually less performance critical than loads and
stores. It's only loads and stores that give a false sense of
correctness as in the above commit.
Paolo
signature.asc
Description: OpenPGP digital signature
