On 19/04/21 11:36, Peter Zijlstra wrote:
On Mon, Apr 19, 2021 at 11:02:12AM +0200, Paolo Bonzini wrote:
void writer(void)
{
atomic_store_explicit(&seq, seq+1, memory_order_relaxed);
atomic_thread_fence(memory_order_acquire);
This needs to be memory_order_release. The only change in the resulting
assembly is that "dmb ishld" becomes "dmb ish", which is not as good as the
"dmb ishst" you get from smp_wmb() but not buggy either.
Yuck! And that is what requires the insides to be
atomic_store_explicit(), otherwise this fence doesn't have to affect
them.
Not just that, even the write needs to be atomic_store_explicit in order
to avoid a data race.atomic_store_explicit
I also don't see how this is better than seq_cst.
It is better than seq_cst on TSO architectures. Another possibility is
to use release stores for everything (both increments and the stores
between them).
But yes, not broken, but also very much not optimal.
Agreed on that, just like RCU/memory_order_consume.
Paolo