david.venhoff/QEMU-Nyx-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

qemu-thread: use acquire/release to clarify semantics of QemuEvent

Browse Source 
Do not use the somewhat mysterious atomic_mb_read/atomic_mb_set,
instead make sure that the operations on QemuEvent are annotated
with the desired acquire and release semantics.

In particular, qemu_event_set wakes up the waiting thread, so it must
be a release from the POV of the waker (compare with qemu_mutex_unlock).
And it actually needs a full barrier, because that's the only thing that
provides something like a "load-release".

Use smp_mb_acquire until we have atomic_load_acquire and
atomic_store_release in atomic.h.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2016-09-19 11:10:57 +02:00
parent f1ee86963b
commit 374293ca6f
2 changed files with 24 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
util/qemu-thread-posix.c
View File

@ -360,7 +360,11 @@ void qemu_event_destroy(QemuEvent *ev)
void qemu_event_set(QemuEvent *ev) void qemu_event_set(QemuEvent *ev)
{ {
if (atomic_mb_read(&ev->value) != EV_SET) { /* qemu_event_set has release semantics, but because it *loads*
* ev->value we need a full memory barrier here.
*/
smp_mb();
if (atomic_read(&ev->value) != EV_SET) {
if (atomic_xchg(&ev->value, EV_SET) == EV_BUSY) { if (atomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
/* There were waiters, wake them up. */ /* There were waiters, wake them up. */
futex_wake(ev, INT_MAX); futex_wake(ev, INT_MAX);
@ -370,7 +374,11 @@ void qemu_event_set(QemuEvent *ev)
void qemu_event_reset(QemuEvent *ev) void qemu_event_reset(QemuEvent *ev)
{ {
if (atomic_mb_read(&ev->value) == EV_SET) { unsigned value;
value = atomic_read(&ev->value);
smp_mb_acquire();
if (value == EV_SET) {
/* /*
* If there was a concurrent reset (or even reset+wait), * If there was a concurrent reset (or even reset+wait),
* do nothing. Otherwise change EV_SET->EV_FREE. * do nothing. Otherwise change EV_SET->EV_FREE.
@ -383,7 +391,8 @@ void qemu_event_wait(QemuEvent *ev)
{ {
unsigned value; unsigned value;
value = atomic_mb_read(&ev->value); value = atomic_read(&ev->value);
smp_mb_acquire();
if (value != EV_SET) { if (value != EV_SET) {
if (value == EV_FREE) { if (value == EV_FREE) {
/* /*

15
util/qemu-thread-win32.c
View File

@ -274,7 +274,11 @@ void qemu_event_destroy(QemuEvent *ev)
void qemu_event_set(QemuEvent *ev) void qemu_event_set(QemuEvent *ev)
{ {
if (atomic_mb_read(&ev->value) != EV_SET) { /* qemu_event_set has release semantics, but because it *loads*
* ev->value we need a full memory barrier here.
*/
smp_mb();
if (atomic_read(&ev->value) != EV_SET) {
if (atomic_xchg(&ev->value, EV_SET) == EV_BUSY) { if (atomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
/* There were waiters, wake them up. */ /* There were waiters, wake them up. */
SetEvent(ev->event); SetEvent(ev->event);
@ -284,7 +288,11 @@ void qemu_event_set(QemuEvent *ev)
void qemu_event_reset(QemuEvent *ev) void qemu_event_reset(QemuEvent *ev)
{ {
if (atomic_mb_read(&ev->value) == EV_SET) { unsigned value;
value = atomic_read(&ev->value);
smp_mb_acquire();
if (value == EV_SET) {
/* If there was a concurrent reset (or even reset+wait), /* If there was a concurrent reset (or even reset+wait),
* do nothing. Otherwise change EV_SET->EV_FREE. * do nothing. Otherwise change EV_SET->EV_FREE.
*/ */
@ -296,7 +304,8 @@ void qemu_event_wait(QemuEvent *ev)
{ {
unsigned value; unsigned value;
value = atomic_mb_read(&ev->value); value = atomic_read(&ev->value);
smp_mb_acquire();
if (value != EV_SET) { if (value != EV_SET) {
if (value == EV_FREE) { if (value == EV_FREE) {
/* qemu_event_set is not yet going to call SetEvent, but we are /* qemu_event_set is not yet going to call SetEvent, but we are