From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50376)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1b5aOP-0004jL-61
	for qemu-devel@nongnu.org; Wed, 25 May 2016 11:06:42 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1b5aOJ-0006jT-AO
	for qemu-devel@nongnu.org; Wed, 25 May 2016 11:06:41 -0400
Received: from out2-smtp.messagingengine.com ([66.111.4.26]:44157)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1b5aOI-0006jI-Vm
	for qemu-devel@nongnu.org; Wed, 25 May 2016 11:06:35 -0400
Date: Wed, 25 May 2016 11:06:33 -0400
From: "Emilio G. Cota" <cota@braap.org>
Message-ID: <20160525150633.GA4546@flamenco>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1ed20476-934e-4ed7-89ea-69ec3d2f6680@redhat.com>
	<2668174b-abe5-5301-690f-8f199426a02e@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/3] atomics: emit an
 smp_read_barrier_depends() barrier only for Sparc and Thread Sanitizer
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>, MTTCG Devel <mttcg@greensocs.com>, Alex =?iso-8859-1?Q?Benn=E9e?= <alex.bennee@linaro.org>, Richard Henderson <rth@twiddle.net>, Sergey Fedorov <serge.fdrv@gmail.com>

On Wed, May 25, 2016 at 14:16:56 +0200, Paolo Bonzini wrote:
> On 24/05/2016 22:06, Emilio G. Cota wrote:
> > For correctness, smp_read_barrier_depends() is only required to
> > emit a barrier on Sparc hosts. However, we are currently emitting
> > a consume fence unconditionally.
> 
> Let's say why this is suboptimal:
> 
> ... and most compilers currently treat consume and acquire fences as 
> equivalent.
> 
> Likewise, let's add a comment like this:
> 
> +/* Most compilers currently treat consume and acquire the same, but really
> + * no processors except Alpha need a barrier here.  Leave it in if
> + * using Thread Sanitizer to avoid warnings, otherwise optimize it away.
> + */
> 
> If okay I can do the change myself.

On Wed, May 25, 2016 at 14:20:02 +0200, Paolo Bonzini wrote:
> On 24/05/2016 22:06, Emilio G. Cota wrote:
> > Currently we emit a consume-load in atomic_rcu_read. This is
> > overkill for non-Sparc hosts, and is only useful to make
> > things easier for Thread Sanitizer, which as far as I understand
> > works best without explicit fences.
> 
> Likewise:
> 
> Currently we emit a consume-load in atomic_rcu_read.  Because of
> limitations in current compilers, this is overkill for non-Alpha hosts
> and it is only useful to make Thread Sanitizer work.
> 
> and
> 
> +/* See above: most compilers currently treat consume and acquire the
> + * same, but this slows down atomic_rcu_read unnecessarily.
> + */

Please go ahead with these changes. Don't forget to s/Sparc/Alpha/ on the
commit messages! There are 3 bogus Sparc's in the commit log of
(my) patch 2/3, including the commit title.

Thanks,

		Emilio