Introduce urcu_assert and registration check

Add a "registered" flag to urcu.c and urcu-qsbr.c, set/cleared when a
thread is registered and unregistered. Add corresponding asserts in
those functions checking if a thread is registered or unregistered more
than once (which would be a bug in the way the application uses urcu).

Move the checks enabled on RCU_DEBUG to a single header: urcu/debug.h.
Add checks for the registered flag in RCU read-side lock functions (new
urcu_assert() checks, which are only built-in if RCU_DEBUG is defined at
compile-time).

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

diff --git a/Makefile.am b/Makefile.am
index 752510d717783c9d0868c6701767613b617ba357..ce70dc9f7c9bc6224469e6022e9421a9f27fec9b 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,7 +24,7 @@ nobase_dist_include_HEADERS = urcu/compiler.h urcu/hlist.h urcu/list.h \
                $(top_srcdir)/urcu/map/*.h \
                $(top_srcdir)/urcu/static/*.h \
                urcu/rand-compat.h \
-               urcu/tls-compat.h
+               urcu/tls-compat.h urcu/debug.h
 nobase_nodist_include_HEADERS = urcu/arch.h urcu/uatomic.h urcu/config.h
 
 dist_noinst_HEADERS = urcu-die.h urcu-wait.h

diff --git a/urcu-defer-impl.h b/urcu-defer-impl.h
index f1dca8ff85c2289d300cc92a77e9d623cbb39216..f96553365ba4f51035f427798edd2ac4eb981f13 100644 (file)
--- a/urcu-defer-impl.h
+++ b/urcu-defer-impl.h
@@ -84,12 +84,6 @@
  * This is required to permit relinking with newer versions of the library.
  */
 
-#ifdef DEBUG_RCU
-#define rcu_assert(args...)    assert(args)
-#else
-#define rcu_assert(args...)
-#endif
-
 /*
  * defer queue.
  * Contains pointers. Encoded to save space when same callback is often used.

diff --git a/urcu-qsbr.c b/urcu-qsbr.c
index 619df60511b67ce9f5408d13e5ef33f7326ee7f5..af82fb7e6d3d6bbe3d6ef199c5c9d23ce9253163 100644 (file)
--- a/urcu-qsbr.c
+++ b/urcu-qsbr.c
@@ -468,6 +468,8 @@ void rcu_register_thread(void)
        assert(URCU_TLS(rcu_reader).ctr == 0);
 
        mutex_lock(&rcu_registry_lock);
+       assert(!URCU_TLS(rcu_reader).registered);
+       URCU_TLS(rcu_reader).registered = 1;
        cds_list_add(&URCU_TLS(rcu_reader).node, &registry);
        mutex_unlock(&rcu_registry_lock);
        _rcu_thread_online();
@@ -480,6 +482,8 @@ void rcu_unregister_thread(void)
         * with a waiting writer.
         */
        _rcu_thread_offline();
+       assert(URCU_TLS(rcu_reader).registered);
+       URCU_TLS(rcu_reader).registered = 0;
        mutex_lock(&rcu_registry_lock);
        cds_list_del(&URCU_TLS(rcu_reader).node);
        mutex_unlock(&rcu_registry_lock);

diff --git a/urcu.c b/urcu.c
index 5ffeb79553b6374f0fb1a607b3862ed78dda63e2..4702ba9aea4c1bc55f180b658c28bc56c03dec96 100644 (file)
--- a/urcu.c
+++ b/urcu.c
@@ -500,6 +500,8 @@ void rcu_register_thread(void)
        assert(!(URCU_TLS(rcu_reader).ctr & RCU_GP_CTR_NEST_MASK));
 
        mutex_lock(&rcu_registry_lock);
+       assert(!URCU_TLS(rcu_reader).registered);
+       URCU_TLS(rcu_reader).registered = 1;
        rcu_init();     /* In case gcc does not support constructor attribute */
        cds_list_add(&URCU_TLS(rcu_reader).node, &registry);
        mutex_unlock(&rcu_registry_lock);
@@ -508,6 +510,8 @@ void rcu_register_thread(void)
 void rcu_unregister_thread(void)
 {
        mutex_lock(&rcu_registry_lock);
+       assert(URCU_TLS(rcu_reader).registered);
+       URCU_TLS(rcu_reader).registered = 0;
        cds_list_del(&URCU_TLS(rcu_reader).node);
        mutex_unlock(&rcu_registry_lock);
 }

diff --git a/urcu/debug.h b/urcu/debug.h
new file mode 100644 (file)
index 0000000..327bd92
--- /dev/null
+++ b/urcu/debug.h
@@ -0,0 +1,30 @@
+#ifndef _URCU_DEBUG_H
+#define _URCU_DEBUG_H
+
+/*
+ * urcu/debug.h
+ *
+ * Userspace RCU debugging facilities.
+ *
+ * Copyright (c) 2015 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ */
+
+#include <assert.h>
+
+#ifdef DEBUG_RCU
+#define urcu_assert(...)       assert(__VA_ARGS__)
+#else
+#define urcu_assert(...)
+#endif
+
+#endif /* _URCU_DEBUG_H */

diff --git a/urcu/static/urcu-bp.h b/urcu/static/urcu-bp.h
index b6d5f132778380c57af501307cf25e017c92c8ec..0fcaa3aa272806aa9c8786c96b7675de4fcfddd3 100644 (file)
--- a/urcu/static/urcu-bp.h
+++ b/urcu/static/urcu-bp.h
@@ -39,6 +39,7 @@
 #include <urcu/uatomic.h>
 #include <urcu/list.h>
 #include <urcu/tls-compat.h>
+#include <urcu/debug.h>
 
 /*
  * This code section can only be included in LGPL 2.1 compatible source code.
@@ -52,12 +53,6 @@
 extern "C" {
 #endif
 
-#ifdef DEBUG_RCU
-#define rcu_assert(args...)    assert(args)
-#else
-#define rcu_assert(args...)
-#endif
-
 enum rcu_state {
        RCU_READER_ACTIVE_CURRENT,
        RCU_READER_ACTIVE_OLD,

diff --git a/urcu/static/urcu-qsbr.h b/urcu/static/urcu-qsbr.h
index 143d75a7e1e047f0ec2f81709f3ca39175debdab..8e46820d7ae7436812f5ce8c9d1b1cc7d57f6900 100644 (file)
--- a/urcu/static/urcu-qsbr.h
+++ b/urcu/static/urcu-qsbr.h
@@ -31,7 +31,6 @@
 
 #include <stdlib.h>
 #include <pthread.h>
-#include <assert.h>
 #include <limits.h>
 #include <unistd.h>
 #include <stdint.h>
@@ -43,6 +42,7 @@
 #include <urcu/list.h>
 #include <urcu/futex.h>
 #include <urcu/tls-compat.h>
+#include <urcu/debug.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -56,12 +56,6 @@ extern "C" {
  * This is required to permit relinking with newer versions of the library.
  */
 
-#ifdef DEBUG_RCU
-#define rcu_assert(args...)    assert(args)
-#else
-#define rcu_assert(args...)
-#endif
-
 enum rcu_state {
        RCU_READER_ACTIVE_CURRENT,
        RCU_READER_ACTIVE_OLD,
@@ -91,6 +85,8 @@ struct rcu_reader {
        struct cds_list_head node __attribute__((aligned(CAA_CACHE_LINE_SIZE)));
        int waiting;
        pthread_t tid;
+       /* Reader registered flag, for internal checks. */
+       unsigned int registered:1;
 };
 
 extern DECLARE_URCU_TLS(struct rcu_reader, rcu_reader);
@@ -137,7 +133,7 @@ static inline enum rcu_state rcu_reader_state(unsigned long *ctr)
  */
 static inline void _rcu_read_lock(void)
 {
-       rcu_assert(URCU_TLS(rcu_reader).ctr);
+       urcu_assert(URCU_TLS(rcu_reader).ctr);
 }
 
 /*
@@ -149,6 +145,7 @@ static inline void _rcu_read_lock(void)
  */
 static inline void _rcu_read_unlock(void)
 {
+       urcu_assert(URCU_TLS(rcu_reader).ctr);
 }
 
 /*
@@ -197,6 +194,7 @@ static inline void _rcu_quiescent_state(void)
 {
        unsigned long gp_ctr;
 
+       urcu_assert(URCU_TLS(rcu_reader).registered);
        if ((gp_ctr = CMM_LOAD_SHARED(rcu_gp.ctr)) == URCU_TLS(rcu_reader).ctr)
                return;
        _rcu_quiescent_state_update_and_wakeup(gp_ctr);
@@ -212,6 +210,7 @@ static inline void _rcu_quiescent_state(void)
  */
 static inline void _rcu_thread_offline(void)
 {
+       urcu_assert(URCU_TLS(rcu_reader).registered);
        cmm_smp_mb();
        CMM_STORE_SHARED(URCU_TLS(rcu_reader).ctr, 0);
        cmm_smp_mb();   /* write URCU_TLS(rcu_reader).ctr before read futex */
@@ -229,6 +228,7 @@ static inline void _rcu_thread_offline(void)
  */
 static inline void _rcu_thread_online(void)
 {
+       urcu_assert(URCU_TLS(rcu_reader).registered);
        cmm_barrier();  /* Ensure the compiler does not reorder us with mutex */
        _CMM_STORE_SHARED(URCU_TLS(rcu_reader).ctr, CMM_LOAD_SHARED(rcu_gp.ctr));
        cmm_smp_mb();

diff --git a/urcu/static/urcu.h b/urcu/static/urcu.h
index af8eee442d3849f5756423d7337d16c93f712f34..fbba46cde9afdc6bff0c4b337a22afc78d40fea7 100644 (file)
--- a/urcu/static/urcu.h
+++ b/urcu/static/urcu.h
@@ -42,6 +42,7 @@
 #include <urcu/futex.h>
 #include <urcu/tls-compat.h>
 #include <urcu/rand-compat.h>
+#include <urcu/debug.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -79,12 +80,6 @@ enum rcu_state {
        RCU_READER_INACTIVE,
 };
 
-#ifdef DEBUG_RCU
-#define rcu_assert(args...)    assert(args)
-#else
-#define rcu_assert(args...)
-#endif
-
 /*
  * RCU memory barrier broadcast group. Currently, only broadcast to all process
  * threads is supported (group 0).
@@ -157,6 +152,8 @@ struct rcu_reader {
        /* Data used for registry */
        struct cds_list_head node __attribute__((aligned(CAA_CACHE_LINE_SIZE)));
        pthread_t tid;
+       /* Reader registered flag, for internal checks. */
+       unsigned int registered:1;
 };
 
 extern DECLARE_URCU_TLS(struct rcu_reader, rcu_reader);
@@ -224,6 +221,7 @@ static inline void _rcu_read_lock(void)
 {
        unsigned long tmp;
 
+       urcu_assert(URCU_TLS(rcu_reader).registered);
        cmm_barrier();
        tmp = URCU_TLS(rcu_reader).ctr;
        _rcu_read_lock_update(tmp);
@@ -257,6 +255,7 @@ static inline void _rcu_read_unlock(void)
 {
        unsigned long tmp;
 
+       urcu_assert(URCU_TLS(rcu_reader).registered);
        tmp = URCU_TLS(rcu_reader).ctr;
        _rcu_read_unlock_update_and_wakeup(tmp);
        cmm_barrier();  /* Ensure the compiler does not reorder us with mutex */