qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Emilio G. Cota" <cota@braap.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "QEMU Developers" <qemu-devel@nongnu.org>,
	"MTTCG Devel" <mttcg@greensocs.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Richard Henderson" <rth@twiddle.net>,
	"Sergey Fedorov" <serge.fdrv@gmail.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] atomics: do not use __atomic primitives for RCU atomics
Date: Mon, 23 May 2016 11:55:10 -0400	[thread overview]
Message-ID: <20160523155510.GC1768@flamenco> (raw)
In-Reply-To: <955e8307-01a5-b2f9-48df-8309bd30c443@redhat.com>

On Mon, May 23, 2016 at 16:21:36 +0200, Paolo Bonzini wrote:
> On 21/05/2016 22:42, Emilio G. Cota wrote:
> > Commit a0aa44b4 ("include/qemu/atomic.h: default to __atomic functions")
> > set all atomics to default (on recent GCC versions) to __atomic primitives.
> > 
> > In the process, the atomic_rcu_read/set were converted to implement
> > consume/release semantics, respectively. This is inefficient; for
> > correctness and maximum performance we only need an smp_barrier_depends
> > for reads, and an smp_wmb for writes. Fix it by using the original
> > definition of these two primitives for all compilers.
> 
> Indeed most compilers implement consume the same as acquire, which is
> inefficient.
> However, isn't in practice atomic_thread_fence(release) +
> atomic_store(relaxed) the same as atomic_store(release)?

Yes. However this is not the issue I'm addressing with the patch.

The performance regression I measured is due to using load-acquire vs.
load+smp_read_barrier_depends(). In the latter case only Alpha will
emit a fence; in the former we always emit store-release, which
is "stronger" (i.e. more constraining.)

A similar thing applies to atomic_rcu_write, although I haven't
measured its impact. We only need smp_wmb+store, yet we emit a
store-release, which is again "stronger".

		E.

  reply	other threads:[~2016-05-23 15:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-21 20:42 [Qemu-devel] [PATCH 0/2] atomics: fix small RCU perf. regression + update documentation Emilio G. Cota
2016-05-21 20:42 ` [Qemu-devel] [PATCH 1/2] atomics: do not use __atomic primitives for RCU atomics Emilio G. Cota
2016-05-22  7:58   ` Alex Bennée
2016-05-24 18:42     ` Emilio G. Cota
2016-05-23 14:21   ` Paolo Bonzini
2016-05-23 15:55     ` Emilio G. Cota [this message]
2016-05-23 16:53   ` Richard Henderson
2016-05-23 17:09     ` Emilio G. Cota
2016-05-24  7:08       ` Paolo Bonzini
2016-05-24 19:56         ` Emilio G. Cota
2016-05-24 19:59           ` Sergey Fedorov
2016-05-25  8:52             ` Alex Bennée
2016-05-25 11:02               ` Sergey Fedorov
2016-05-21 20:42 ` [Qemu-devel] [PATCH 2/2] docs/atomics: update atomic_read/set comparison with Linux Emilio G. Cota

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=20160523155510.GC1768@flamenco \
    --to=cota@braap.org \
    --cc=alex.bennee@linaro.org \
    --cc=mttcg@greensocs.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=serge.fdrv@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).