[Qemu-devel] [PULL 08/11] rcu: detect missing rcu_register_thread()

[Paolo Bonzini]

Use an "impossible" value for the .depth field in order to quickly
detect threads that have not registered themselves with the RCU
subsystem.

Avoid a false positive around forking by unregistering and registering
the forking thread explicitly.  Previously, it was enough to re-register
the thread.

Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
 include/qemu/rcu.h |  4 +++-
 util/rcu.c         | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 7df1e86..4facb35 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -82,7 +82,9 @@ static inline void rcu_read_lock(void)
     struct rcu_reader_data *p_rcu_reader = &rcu_reader;
     unsigned ctr;
 
-    if (p_rcu_reader->depth++ > 0) {
+    p_rcu_reader->depth++;
+    assert(p_rcu_reader->depth >= 1);
+    if (p_rcu_reader->depth > 1) {
         return;
     }
 
diff --git a/util/rcu.c b/util/rcu.c
index e21bb46..2490273 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -63,8 +63,11 @@ static inline int rcu_gp_ongoing(unsigned long *ctr)
 
 /* Written to only by each individual reader. Read by both the reader and the
  * writers.
+ *
+ * Initializing the depth to -1 causes an assertion failure on the first
+ * call to rcu_read_lock() if the thread does not call rcu_register_thread().
  */
-__thread struct rcu_reader_data rcu_reader;
+__thread struct rcu_reader_data rcu_reader = { .depth = -1 };
 
 /* Protected by rcu_gp_lock.  */
 typedef QLIST_HEAD(, rcu_reader_data) ThreadList;
@@ -279,7 +282,12 @@ static void rcu_unregister_thread_notify(Notifier *n, void 
*data)
 
 void rcu_register_thread(void)
 {
-    assert(rcu_reader.ctr == 0);
+    /* rcu_reader.depth is also used to detect whether the thread is
+     * registered.
+     */
+    assert(rcu_reader.depth == -1);
+    rcu_reader.depth = 0;
+
     qemu_mutex_lock(&rcu_gp_lock);
     QLIST_INSERT_HEAD(&registry, &rcu_reader, node);
     qemu_mutex_unlock(&rcu_gp_lock);
@@ -290,6 +298,12 @@ void rcu_register_thread(void)
 
 void rcu_unregister_thread(void)
 {
+    /* Resetting the depth to -1 causes an assertion failure on the next
+     * call to rcu_read_lock().
+     */
+    assert(rcu_reader.depth == 0);
+    rcu_reader.depth = -1;
+
     qemu_mutex_lock(&rcu_gp_lock);
     QLIST_REMOVE(&rcu_reader, node);
     qemu_mutex_unlock(&rcu_gp_lock);
@@ -301,7 +315,6 @@ static void rcu_init_complete(void)
 {
     QemuThread thread;
 
-    qemu_mutex_init(&rcu_gp_lock);
     qemu_event_init(&rcu_gp_event, true);
 
     qemu_event_init(&rcu_call_ready_event, false);
@@ -311,8 +324,6 @@ static void rcu_init_complete(void)
      */
     qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
                        NULL, QEMU_THREAD_DETACHED);
-
-    rcu_register_thread();
 }
 
 #ifdef CONFIG_POSIX
@@ -329,14 +340,28 @@ static void rcu_init_unlock(void)
 
 void rcu_after_fork(void)
 {
+    int save_depth = rcu_reader.depth;
+    if (save_depth != -1) {
+        rcu_unregister_thread();
+    }
+
     memset(&registry, 0, sizeof(registry));
+
     rcu_init_complete();
+
+    if (save_depth != -1) {
+        rcu_register_thread();
+        rcu_reader.depth = save_depth;
+    }
 }
 
 static void __attribute__((__constructor__)) rcu_init(void)
 {
+    qemu_mutex_init(&rcu_gp_lock);
 #ifdef CONFIG_POSIX
     pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_unlock);
 #endif
     rcu_init_complete();
+
+    rcu_register_thread();
 }
-- 
2.4.3