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

Reply via email to