urcu-qsbr: use rcu_thread_offline/rcu_thread_online instead of inlining them

* Mathieu Desnoyers wrote:

> Just to let you know that I pushed two updates into urcu: one fixes a
> grace period hang caused by a missing wakeup in the synchronize_rcu
> QSBR code. This appears to hit us due to the more fine-grained wakeup
> code brought by Paolo. The wakeup was really missing from the
> synchronize_rcu code (so Paolo's code just triggered an existing
> problem). I thought it would be good to let you know the effect: grace
> periods are delayed forever. This problem never appeared in a release
> (I caught it before).

Good catch.  Why not use rcu_thread_offline/online in synchronize_rcu,
instead of touching rcu_reader.ctr directly?  I had this in my QEMU
branch but hadn't posted yet because it was meant as a cleanup only.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/urcu-qsbr.c b/urcu-qsbr.c
index 1dc99792d7a49f2f6f3614693e34cde239312bfb..1adaa9409926edb602302584347edf3febd7c7fd 100644 (file)
--- a/urcu-qsbr.c
+++ b/urcu-qsbr.c
@@ -208,21 +208,17 @@ void synchronize_rcu(void)
        was_online = rcu_reader.ctr;
 
        /* All threads should read qparity before accessing data structure
-        * where new ptr points to.
-        */
-       /* Write new ptr before changing the qparity */
-       cmm_smp_mb();
-
-       /*
+        * where new ptr points to.  In the "then" case, rcu_thread_offline
+        * includes a memory barrier.
+        *
         * Mark the writer thread offline to make sure we don't wait for
         * our own quiescent state. This allows using synchronize_rcu()
         * in threads registered as readers.
         */
-       if (was_online) {
-               CMM_STORE_SHARED(rcu_reader.ctr, 0);
-               cmm_smp_mb();   /* write rcu_reader.ctr before read futex */
-               wake_up_gp();
-       }
+       if (was_online)
+               rcu_thread_offline();
+       else
+               cmm_smp_mb();
 
        mutex_lock(&rcu_gp_lock);
 
@@ -263,9 +259,9 @@ out:
         * freed.
         */
        if (was_online)
-               _CMM_STORE_SHARED(rcu_reader.ctr,
-                                 CMM_LOAD_SHARED(rcu_gp_ctr));
-       cmm_smp_mb();
+               rcu_thread_online();
+       else
+               cmm_smp_mb();
 }
 #else /* !(CAA_BITS_PER_LONG < 64) */
 void synchronize_rcu(void)
@@ -279,12 +275,10 @@ void synchronize_rcu(void)
         * our own quiescent state. This allows using synchronize_rcu()
         * in threads registered as readers.
         */
-       cmm_smp_mb();
-       if (was_online) {
-               CMM_STORE_SHARED(rcu_reader.ctr, 0);
-               cmm_smp_mb();   /* write rcu_reader.ctr before read futex */
-               wake_up_gp();
-       }
+       if (was_online)
+               rcu_thread_offline();
+       else
+               cmm_smp_mb();
 
        mutex_lock(&rcu_gp_lock);
        if (cds_list_empty(&registry))
@@ -294,9 +288,9 @@ out:
        mutex_unlock(&rcu_gp_lock);
 
        if (was_online)
-               _CMM_STORE_SHARED(rcu_reader.ctr,
-                                 CMM_LOAD_SHARED(rcu_gp_ctr));
-       cmm_smp_mb();
+               rcu_thread_online();
+       else
+               cmm_smp_mb();
 }
 #endif  /* !(CAA_BITS_PER_LONG < 64) */