Re: [Qemu-devel] [PATCH] rcu: actually register threads that have RCU read-side critical sections

[Wen Congyang <wency@cn.fujitsu.com>]

On 07/24/2015 12:58 AM, Paolo Bonzini wrote:
> 
> 
> On 23/07/2015 14:59, Wen Congyang wrote:
>>>>
>>>> If the thread doesn't use RCU, rcu_register_thread() is harmless, is
>>>> it right?
>>>
>>> Every rcu_register_thread() makes synchronize_rcu() a little slower.
>>
>> Yes, but synchronize_rcu() is very slow...
> 
> Hmm, worse, rcu_register_thread() if called together with
> synchronize_rcu() it waits for the synchronize_rcu() to finish. :/

What about this modification:
diff --git a/util/rcu.c b/util/rcu.c
index 7270151..ccf8cfa 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -48,6 +48,7 @@ unsigned long rcu_gp_ctr = RCU_GP_LOCKED;
 
 QemuEvent rcu_gp_event;
 static QemuMutex rcu_gp_lock;
+static QemuMutex rcu_sync_lock;
 
 /*
  * Check whether a quiescent state was crossed between the beginning of
@@ -115,9 +116,12 @@ static void wait_for_readers(void)
         }
 
         /* Wait for one thread to report a quiescent state and
-         * try again.
+         * try again. Release rcu_gp_lock, so rcu_(un)register_thread()
+         * doesn't wait too much time.
          */
+        qemu_mutex_unlock(&rcu_gp_lock);
         qemu_event_wait(&rcu_gp_event);
+        qemu_mutex_lock(&rcu_gp_lock);
     }
 
     /* put back the reader list in the registry */
@@ -126,6 +130,7 @@ static void wait_for_readers(void)
 
 void synchronize_rcu(void)
 {
+    qemu_mutex_lock(&rcu_sync_lock);
     qemu_mutex_lock(&rcu_gp_lock);
 
     if (!QLIST_EMPTY(&registry)) {
@@ -150,6 +155,7 @@ void synchronize_rcu(void)
     }
 
     qemu_mutex_unlock(&rcu_gp_lock);
+    qemu_mutex_unlock(&rcu_sync_lock);
 }
 
 
@@ -288,6 +294,7 @@ static void rcu_init_complete(void)
     QemuThread thread;
 
     qemu_mutex_init(&rcu_gp_lock);
+    qemu_mutex_init(&rcu_sync_lock);
     qemu_event_init(&rcu_gp_event, true);
 
     qemu_event_init(&rcu_call_ready_event, false);
@@ -304,12 +311,14 @@ static void rcu_init_complete(void)
 #ifdef CONFIG_POSIX
 static void rcu_init_lock(void)
 {
+    qemu_mutex_lock(&rcu_sync_lock);
     qemu_mutex_lock(&rcu_gp_lock);
 }
 
 static void rcu_init_unlock(void)
 {
     qemu_mutex_unlock(&rcu_gp_lock);
+    qemu_mutex_unlock(&rcu_sync_lock);
 }
 #endif
 
rcu_register_thread() will be a littl slower when it is
called together with synchronize_rcu().

Thanks
Wen Congyang

> 
> Paolo
> 
>>>
>>>>>> be simpler to add an assertion in rcu_register_thread.  I'm just a bit
>>>>>> wary of doing little more than the bare minimum in 2.4, because of the
>>>>>> OS X failure that I didn't quite understand.
>>>> Which problem? I don't find it in the maillist.
>>>
>>> http://article.gmane.org/gmane.comp.emulators.qemu/351548
>>
>> Hmm, I guess rcu_reader is invalid when pthread key is destroyed.
>> pthread key and __thread
>> variable, which is destroyed first? I don't find any document to
>> describe it.
>