qemu-thread-posix: cleanup, fix, document QemuEvent
QemuEvent is currently broken on ARM due to missing memory barriers after qatomic_*(). Apart from adding the memory barrier, a closer look reveals some unpaired memory barriers too. Document more clearly what is going on. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: David Hildenbrand <david@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
parent
ff00bed189
commit
9586a1329f
|
@ -384,13 +384,21 @@ void qemu_event_destroy(QemuEvent *ev)
|
||||||
|
|
||||||
void qemu_event_set(QemuEvent *ev)
|
void qemu_event_set(QemuEvent *ev)
|
||||||
{
|
{
|
||||||
/* qemu_event_set has release semantics, but because it *loads*
|
assert(ev->initialized);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Pairs with both qemu_event_reset() and qemu_event_wait().
|
||||||
|
*
|
||||||
|
* qemu_event_set has release semantics, but because it *loads*
|
||||||
* ev->value we need a full memory barrier here.
|
* ev->value we need a full memory barrier here.
|
||||||
*/
|
*/
|
||||||
assert(ev->initialized);
|
|
||||||
smp_mb();
|
smp_mb();
|
||||||
if (qatomic_read(&ev->value) != EV_SET) {
|
if (qatomic_read(&ev->value) != EV_SET) {
|
||||||
if (qatomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
|
int old = qatomic_xchg(&ev->value, EV_SET);
|
||||||
|
|
||||||
|
/* Pairs with memory barrier in kernel futex_wait system call. */
|
||||||
|
smp_mb__after_rmw();
|
||||||
|
if (old == EV_BUSY) {
|
||||||
/* There were waiters, wake them up. */
|
/* There were waiters, wake them up. */
|
||||||
qemu_futex_wake(ev, INT_MAX);
|
qemu_futex_wake(ev, INT_MAX);
|
||||||
}
|
}
|
||||||
|
@ -399,18 +407,19 @@ void qemu_event_set(QemuEvent *ev)
|
||||||
|
|
||||||
void qemu_event_reset(QemuEvent *ev)
|
void qemu_event_reset(QemuEvent *ev)
|
||||||
{
|
{
|
||||||
unsigned value;
|
|
||||||
|
|
||||||
assert(ev->initialized);
|
assert(ev->initialized);
|
||||||
value = qatomic_read(&ev->value);
|
|
||||||
smp_mb_acquire();
|
/*
|
||||||
if (value == EV_SET) {
|
* If there was a concurrent reset (or even reset+wait),
|
||||||
/*
|
* do nothing. Otherwise change EV_SET->EV_FREE.
|
||||||
* If there was a concurrent reset (or even reset+wait),
|
*/
|
||||||
* do nothing. Otherwise change EV_SET->EV_FREE.
|
qatomic_or(&ev->value, EV_FREE);
|
||||||
*/
|
|
||||||
qatomic_or(&ev->value, EV_FREE);
|
/*
|
||||||
}
|
* Order reset before checking the condition in the caller.
|
||||||
|
* Pairs with the first memory barrier in qemu_event_set().
|
||||||
|
*/
|
||||||
|
smp_mb__after_rmw();
|
||||||
}
|
}
|
||||||
|
|
||||||
void qemu_event_wait(QemuEvent *ev)
|
void qemu_event_wait(QemuEvent *ev)
|
||||||
|
@ -418,20 +427,40 @@ void qemu_event_wait(QemuEvent *ev)
|
||||||
unsigned value;
|
unsigned value;
|
||||||
|
|
||||||
assert(ev->initialized);
|
assert(ev->initialized);
|
||||||
value = qatomic_read(&ev->value);
|
|
||||||
smp_mb_acquire();
|
/*
|
||||||
|
* qemu_event_wait must synchronize with qemu_event_set even if it does
|
||||||
|
* not go down the slow path, so this load-acquire is needed that
|
||||||
|
* synchronizes with the first memory barrier in qemu_event_set().
|
||||||
|
*
|
||||||
|
* If we do go down the slow path, there is no requirement at all: we
|
||||||
|
* might miss a qemu_event_set() here but ultimately the memory barrier in
|
||||||
|
* qemu_futex_wait() will ensure the check is done correctly.
|
||||||
|
*/
|
||||||
|
value = qatomic_load_acquire(&ev->value);
|
||||||
if (value != EV_SET) {
|
if (value != EV_SET) {
|
||||||
if (value == EV_FREE) {
|
if (value == EV_FREE) {
|
||||||
/*
|
/*
|
||||||
* Leave the event reset and tell qemu_event_set that there
|
* Leave the event reset and tell qemu_event_set that there are
|
||||||
* are waiters. No need to retry, because there cannot be
|
* waiters. No need to retry, because there cannot be a concurrent
|
||||||
* a concurrent busy->free transition. After the CAS, the
|
* busy->free transition. After the CAS, the event will be either
|
||||||
* event will be either set or busy.
|
* set or busy.
|
||||||
|
*
|
||||||
|
* This cmpxchg doesn't have particular ordering requirements if it
|
||||||
|
* succeeds (moving the store earlier can only cause qemu_event_set()
|
||||||
|
* to issue _more_ wakeups), the failing case needs acquire semantics
|
||||||
|
* like the load above.
|
||||||
*/
|
*/
|
||||||
if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
|
if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* This is the final check for a concurrent set, so it does need
|
||||||
|
* a smp_mb() pairing with the second barrier of qemu_event_set().
|
||||||
|
* The barrier is inside the FUTEX_WAIT system call.
|
||||||
|
*/
|
||||||
qemu_futex_wait(ev, EV_BUSY);
|
qemu_futex_wait(ev, EV_BUSY);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue