[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC] Move to C11 Atomics
From: |
Paolo Bonzini |
Subject: |
Re: [RFC] Move to C11 Atomics |
Date: |
Mon, 21 Sep 2020 16:16:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
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