From a77f7d8228bcf1259f8c4121ce02d4763424d45a Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Sun, 13 Sep 2015 10:48:03 -0400 Subject: [PATCH] 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 --- Makefile.am | 2 +- urcu-defer-impl.h | 6 ------ urcu-qsbr.c | 4 ++++ urcu.c | 4 ++++ urcu/debug.h | 30 ++++++++++++++++++++++++++++++ urcu/static/urcu-bp.h | 7 +------ urcu/static/urcu-qsbr.h | 16 ++++++++-------- urcu/static/urcu.h | 11 +++++------ 8 files changed, 53 insertions(+), 27 deletions(-) create mode 100644 urcu/debug.h diff --git a/Makefile.am b/Makefile.am index 752510d..ce70dc9 100644 --- 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 f1dca8f..f965533 100644 --- 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 619df60..af82fb7 100644 --- 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, ®istry); 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 5ffeb79..4702ba9 100644 --- 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, ®istry); 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 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 + * + * 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 + +#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 b6d5f13..0fcaa3a 100644 --- a/urcu/static/urcu-bp.h +++ b/urcu/static/urcu-bp.h @@ -39,6 +39,7 @@ #include #include #include +#include /* * 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 143d75a..8e46820 100644 --- a/urcu/static/urcu-qsbr.h +++ b/urcu/static/urcu-qsbr.h @@ -31,7 +31,6 @@ #include #include -#include #include #include #include @@ -43,6 +42,7 @@ #include #include #include +#include #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 af8eee4..fbba46c 100644 --- a/urcu/static/urcu.h +++ b/urcu/static/urcu.h @@ -42,6 +42,7 @@ #include #include #include +#include #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 */ -- 2.34.1