[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/4] async: use explicit memory barriers and relaxed accesses
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH 4/4] async: use explicit memory barriers and relaxed accesses |
Date: |
Tue, 7 Apr 2020 10:09:54 +0100 |
On Mon, Apr 06, 2020 at 03:13:20PM -0400, Paolo Bonzini wrote:
> 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 <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> 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);
Non-atomic "atomic" code looks suspicious and warrants a comment
mentioning that this is only executed from one thread. This applies to
the other instances in this patch too.
signature.asc
Description: PGP signature
- [RFC PATCH 0/4] async: fix hangs on weakly-ordered architectures, Paolo Bonzini, 2020/04/06
- [PATCH 3/4] rcu: do not mention atomic_mb_read/set in documentation, Paolo Bonzini, 2020/04/06
- [PATCH 4/4] async: use explicit memory barriers and relaxed accesses, Paolo Bonzini, 2020/04/06
- Re: [PATCH 4/4] async: use explicit memory barriers and relaxed accesses,
Stefan Hajnoczi <=
- [PATCH 2/4] atomics: update documentation for C11, Paolo Bonzini, 2020/04/06
- [PATCH 1/4] atomics: convert to reStructuredText, Paolo Bonzini, 2020/04/06
- Re: [RFC PATCH 0/4] async: fix hangs on weakly-ordered architectures, Stefan Hajnoczi, 2020/04/07