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
