Re: [Qemu-devel] [PATCH 0/5] More thread sanitizer fixes and atomic.h improvements

["Emilio G. Cota" <cota@braap.org>]

On Thu, Oct 13, 2016 at 03:39:55 -0400, Paolo Bonzini wrote:
> > On Mon, Oct 10, 2016 at 15:59:02 +0200, Paolo Bonzini wrote:
> > > See each patch.  My attempt at fixing whatever I did when I obviously
> > > didn't know enough^W about the C11 memory model, and at setting a
> > > better example for future generations...
> > 
> > Just for context. Building on this patchset, is it now time to
> > phase out smp_(rw)mb in favour or C11's acq/rel, as you laid
> > out in your KVM Forum talk [*]?
> 
> Yes, this would be the start of it.  However I'm a bit undecided
> because ARMv8 doesn't have acq/rel memory barriers, and its STLR
> opcode is stronger than a store release.
> 
> > What is the plan with smp_mb_(sg)et? It's not clear to me from
> > the slides, but given patch 5 I don't see a reason to keep them.
> 
> No plan for now.  It makes sense to phase out at least atomic_mb_read.
> atomic_mb_set is more efficient on x86 than store+mfence, so there's
> that too.

I see, thanks.

On a related note: can we squeeze the appended in this patch set? If
we keep the atomic_mb's, at least there should be a good reason for their
use--this is not the case below.

		Emilio

commit cffdc51df4a6346f2b38425f1f1251aa12866fa8
Author: Emilio G. Cota <cota@braap.org>
Date:   Thu Oct 13 15:06:07 2016 -0400

    qht-bench: relax test_start/stop atomic accesses
    
    test_start/stop are used only as flags to loop on. Barriers are unnecessary,
    since no dependent data is transferred among threads apart from the flags
    themselves.
    
    This commit relaxes the three accesses to test_start/stop that were
    not yet relaxed.
    
    Signed-off-by: Emilio G. Cota <cota@braap.org>

diff --git a/tests/qht-bench.c b/tests/qht-bench.c
index 76360a0..2afa09d 100644
--- a/tests/qht-bench.c
+++ b/tests/qht-bench.c
@@ -193,7 +193,7 @@ static void *thread_func(void *p)
     rcu_register_thread();
 
     atomic_inc(&n_ready_threads);
-    while (!atomic_mb_read(&test_start)) {
+    while (!atomic_read(&test_start)) {
         cpu_relax();
     }
 
@@ -393,11 +393,11 @@ static void run_test(void)
     while (atomic_read(&n_ready_threads) != n_rw_threads + n_rz_threads) {
         cpu_relax();
     }
-    atomic_mb_set(&test_start, true);
+    atomic_set(&test_start, true);
     do {
         remaining = sleep(duration);
     } while (remaining);
-    atomic_mb_set(&test_stop, true);
+    atomic_set(&test_stop, true);
 
     for (i = 0; i < n_rw_threads; i++) {
         qemu_thread_join(&rw_threads[i]);