From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: Ying Fang <fangying1@huawei.com>, stefanha@redhat.com
Subject: [PATCH 4/4] async: use explicit memory barriers and relaxed accesses
Date: Mon, 6 Apr 2020 15:13:20 -0400 [thread overview]
Message-ID: <20200406191320.13371-5-pbonzini@redhat.com> (raw)
In-Reply-To: <20200406191320.13371-1-pbonzini@redhat.com>
When using C11 atomics, non-seqcst reads and writes do not participate
in the total order of seqcst operations. In util/async.c and util/aio-posix.c,
in particular, the pattern that we use
write ctx->notify_me write bh->scheduled
read bh->scheduled read ctx->notify_me
if !bh->scheduled, sleep if ctx->notify_me, notify
needs to use seqcst operations for both the write and the read. In
general this is something that we do not want, because there can be
many sources that are polled in addition to bottom halves. The
alternative is to place a seqcst memory barrier between the write
and the read. This also comes with a disadvantage, in that the
memory barrier is implicit on strongly-ordered architectures and
it wastes a few dozen clock cycles.
Fortunately, ctx->notify_me is never written concurrently by two
threads, so we can instead relax the writes to ctx->notify_me.
[This part of the commit message is still to be written more
in detail and is what I am not sure about.]
Analyzed-by: Ying Fang <fangying1@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/aio-posix.c | 9 ++++++++-
util/aio-win32.c | 8 +++++++-
util/async.c | 12 ++++++++++--
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/util/aio-posix.c b/util/aio-posix.c
index cd6cf0a4a9..37afec726f 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -569,7 +569,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
* so disable the optimization now.
*/
if (blocking) {
- atomic_add(&ctx->notify_me, 2);
+ atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
+ /*
+ * Write ctx->notify_me before computing the timeout
+ * (reading bottom half flags, etc.). Pairs with
+ * atomic_xchg in aio_notify().
+ */
+ smp_mb();
}
qemu_lockcnt_inc(&ctx->list_lock);
@@ -590,6 +596,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
}
if (blocking) {
+ /* Finish the poll before clearing the flag. */
atomic_sub(&ctx->notify_me, 2);
aio_notify_accept(ctx);
}
diff --git a/util/aio-win32.c b/util/aio-win32.c
index a23b9c364d..2cccdb35c1 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -331,7 +331,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
* so disable the optimization now.
*/
if (blocking) {
- atomic_add(&ctx->notify_me, 2);
+ atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
+ /*
+ * Write ctx->notify_me before computing the timeout
+ * (reading bottom half flags, etc.). Pairs with
+ * atomic_xchg in aio_notify().
+ */
+ smp_mb();
}
qemu_lockcnt_inc(&ctx->list_lock);
diff --git a/util/async.c b/util/async.c
index b94518b948..ee1bc87d2b 100644
--- a/util/async.c
+++ b/util/async.c
@@ -249,7 +249,14 @@ aio_ctx_prepare(GSource *source, gint *timeout)
{
AioContext *ctx = (AioContext *) source;
- atomic_or(&ctx->notify_me, 1);
+ atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) | 1);
+
+ /*
+ * Write ctx->notify_me before computing the timeout
+ * (reading bottom half flags, etc.). Pairs with
+ * atomic_xchg in aio_notify().
+ */
+ smp_mb();
/* We assume there is no timeout already supplied */
*timeout = qemu_timeout_ns_to_ms(aio_compute_timeout(ctx));
@@ -414,7 +422,7 @@ void aio_notify(AioContext *ctx)
* with atomic_or in aio_ctx_prepare or atomic_add in aio_poll.
*/
smp_mb();
- if (ctx->notify_me) {
+ if (atomic_read(&ctx->notify_me)) {
event_notifier_set(&ctx->notifier);
atomic_mb_set(&ctx->notified, true);
}
--
2.18.2
next prev parent reply other threads:[~2020-04-06 19:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-06 19:13 [RFC PATCH 0/4] async: fix hangs on weakly-ordered architectures Paolo Bonzini
2020-04-06 19:13 ` [PATCH 1/4] atomics: convert to reStructuredText Paolo Bonzini
2020-04-06 19:58 ` Eric Blake
2020-04-07 10:29 ` Paolo Bonzini
2020-04-06 19:13 ` [PATCH 2/4] atomics: update documentation for C11 Paolo Bonzini
2020-04-06 20:03 ` Eric Blake
2020-04-07 9:06 ` Stefan Hajnoczi
2020-04-07 9:13 ` Paolo Bonzini
2020-04-06 19:13 ` [PATCH 3/4] rcu: do not mention atomic_mb_read/set in documentation Paolo Bonzini
2020-04-06 19:13 ` Paolo Bonzini [this message]
2020-04-07 9:09 ` [PATCH 4/4] async: use explicit memory barriers and relaxed accesses Stefan Hajnoczi
2020-04-07 9:18 ` Paolo Bonzini
2020-04-07 9:10 ` [RFC PATCH 0/4] async: fix hangs on weakly-ordered architectures Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200406191320.13371-5-pbonzini@redhat.com \
--to=pbonzini@redhat.com \
--cc=fangying1@huawei.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.