[PATCH v5 11/13] qemu-thread: Remove qatomic_read() in qemu_event_set()

[Akihiko Odaki <akihiko.odaki@daynix.com>]

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.odaki@daynix.com>
---
 util/event.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/util/event.c b/util/event.c
index e937804a92a8..df6d60836041 100644
--- a/util/event.c
+++ b/util/event.c
@@ -55,18 +55,9 @@ void qemu_event_set(QemuEvent *ev)
     assert(ev->initialized);
 
 #ifdef HAVE_FUTEX
-    /*
-     * 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);
     }
 #else
     pthread_mutex_lock(&ev->lock);
@@ -88,7 +79,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();
 }
@@ -102,7 +93,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().
          */
         unsigned value = qatomic_load_acquire(&ev->value);
         if (value == EV_SET) {

-- 
2.49.0