The pair of smp_mb() and qatomic_read() sometimes allows skipping the
following qatomic_xchg() call, but it is unclear if it improves
performance so remove it.
Commit 374293ca6fb0 ("qemu-thread: use acquire/release to clarify
semantics of QemuEvent") replaced atomic_mb_read() in qemu_event_set()
with a pair of smp_mb() and atomic_read(). atomic_mb_read() was actually
cheaper than atomic_xchg(). include/qemu/atomic.h at that time had the
following comment:
/* atomic_mb_read/set semantics map Java volatile variables. They are
* less expensive on some platforms (notably POWER & ARMv7) than fully
* sequentially consistent operations.
*
* As long as they are used as paired operations they are safe to
* use. See docs/atomic.txt for more discussion.
*/
However, smp_mb() enforces full sequential consistency, so we cannot
use the same reasoning to claim that the pair of it and qatomic_read()
is cheaper than qatomic_xchg(). Therefore remove the pair and simplify
the code instead.
Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
util/qemu-thread-posix.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 4d6f24d705c7..a1b592d358c3 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -386,18 +386,9 @@ void qemu_event_set(QemuEvent *ev)
{
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.
- */
- smp_mb();
- if (qatomic_read(&ev->value) != EV_SET) {
- if (qatomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
- /* There were waiters, wake them up. */
- qemu_futex_wake_all(ev);
- }
+ if (qatomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
+ /* There were waiters, wake them up. */
+ qemu_futex_wake_all(ev);
}
}
@@ -413,7 +404,7 @@ void qemu_event_reset(QemuEvent *ev)
/*
* Order reset before checking the condition in the caller.
- * Pairs with the first memory barrier in qemu_event_set().
+ * Pairs with the store-release in qemu_event_set().
*/
smp_mb__after_rmw();
}
@@ -428,7 +419,7 @@ void qemu_event_wait(QemuEvent *ev)
/*
* 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().
+ * synchronizes with the store-release in qemu_event_set().
*/
value = qatomic_load_acquire(&ev->value);
if (value == EV_SET) {
--
2.49.0
