Re: [PATCH v3 04/10] qemu-thread: Avoid futex abstraction for non-Linux

[Akihiko Odaki]

On 2025/05/16 23:25, 'Paolo Bonzini' via devel wrote:
> Il ven 16 mag 2025, 08:58 Akihiko Odaki <akihiko.odaki@daynix.com 
> <mailto:akihiko.odaki@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