On 2025/05/16 23:25, 'Paolo Bonzini' via devel wrote:
Il ven 16 mag 2025, 08:58 Akihiko Odaki <akihiko.od...@daynix.com
<mailto:akihiko.od...@daynix.com>> ha scritto:
I also found smp_mb__after_rmw() before qemu_futex_wake_all() is no
longer necessary. Summing up, I think the code should look like as
follows:
#ifdef HAVE_FUTEX
You would still need smp_mb__before_rmw() here.
docs/devel/atomics.rst says:
> however, according to the C11 memory model that QEMU uses, this order
> does not propagate to other memory accesses on either side of the
> read-modify-write operation. As far as those are concerned, the
> operation consist of just a load-acquire followed by a store-release.
> Stores that precede the RMW operation, and loads that follow it, can
> still be reordered and will happen *in the middle* of the
> read-modify-write operation!
I think we only need a store-release, which is ensured even by the C11
read-modify-write operation; we only need to ensure that ev->value is
set to EV_SET after all stores specified earlier appear.
if (qatomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
Removing the qatomic_read() works, but it's more expensive in the case
that the event is already set.
Removing the qatomic_read() (while keeping smp_mb()) is more expensive
in that case indeed, but I'm not so sure if the case is common enough
that it depreciates the overhead added in the other cases.
I don't know whether the qatomic_read() has improved the performance or
not, but perhaps it is still a premature optimization.
> > The barrier before qemu_futex_wake_all(ev) could be unnecessary
because
there is probably one in FUTEX_WAKE. But not being able to audit Windows
makes me a bit uneasy about it.
With "[PATCH v3 01/10] futex: Check value after qemu_futex_wait()",
qemu_event_wait() always calls qatomic_load_acquire(&ev->value) or
qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) before returning, so a
store-release of ev->value is sufficient to ensure ordering and we don't
need to rely on qemu_futex_wait() for that.
Regards,
Akihiko Odaki