On 07/17/2015 12:55 AM, Paolo Bonzini wrote: > 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(®istry, &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) {
If we allow call fork() in RCU read-side critical section, rcu_reader.depth should be set to 0 before calling rcu_unregister_thread(). Thanks Wen Congyang > + rcu_unregister_thread(); > + } > + > memset(®istry, 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(); > } >