From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byfbQ-0003uQ-Ma for qemu-devel@nongnu.org; Mon, 24 Oct 2016 09:47:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byfbP-0006Gy-MW for qemu-devel@nongnu.org; Mon, 24 Oct 2016 09:47:48 -0400 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:35337) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1byfbP-0006GR-FR for qemu-devel@nongnu.org; Mon, 24 Oct 2016 09:47:47 -0400 Received: by mail-wm0-x244.google.com with SMTP id o81so9896071wma.2 for ; Mon, 24 Oct 2016 06:47:47 -0700 (PDT) Received: from 640k.lan (dynamic-adsl-78-12-246-178.clienti.tiscali.it. [78.12.246.178]) by smtp.gmail.com with ESMTPSA id us3sm19434867wjb.32.2016.10.24.06.47.45 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 06:47:45 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Mon, 24 Oct 2016 15:46:52 +0200 Message-Id: <1477316855-42218-8-git-send-email-pbonzini@redhat.com> In-Reply-To: <1477316855-42218-1-git-send-email-pbonzini@redhat.com> References: <1477316855-42218-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PULL 07/50] qemu-thread: use acquire/release to clarify semantics of QemuEvent List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 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 --- util/qemu-thread-posix.c | 15 ++++++++++++--- util/qemu-thread-win32.c | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 74a3023..ce51b37 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -360,7 +360,11 @@ void qemu_event_destroy(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) { /* There were waiters, wake them up. */ futex_wake(ev, INT_MAX); @@ -370,7 +374,11 @@ void qemu_event_set(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), * do nothing. Otherwise change EV_SET->EV_FREE. @@ -383,7 +391,8 @@ void qemu_event_wait(QemuEvent *ev) { unsigned value; - value = atomic_mb_read(&ev->value); + value = atomic_read(&ev->value); + smp_mb_acquire(); if (value != EV_SET) { if (value == EV_FREE) { /* diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 98a5ddf..072806f 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -274,7 +274,11 @@ void qemu_event_destroy(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) { /* There were waiters, wake them up. */ SetEvent(ev->event); @@ -284,7 +288,11 @@ void qemu_event_set(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), * do nothing. Otherwise change EV_SET->EV_FREE. */ @@ -296,7 +304,8 @@ void qemu_event_wait(QemuEvent *ev) { unsigned value; - value = atomic_mb_read(&ev->value); + value = atomic_read(&ev->value); + smp_mb_acquire(); if (value != EV_SET) { if (value == EV_FREE) { /* qemu_event_set is not yet going to call SetEvent, but we are -- 1.8.3.1