* Mathieu Desnoyers ([email protected]) wrote: > Fix call_rcu fork handling by putting all call_rcu threads in a > quiescent state before fork (paused state), and unpausing them when the > parent returns from fork. > > On the child, everything will run fine as long as we don't issue fork() > from a call_rcu callback. > > Side-note: pthread_atfork is not appropriate when playing with > multithread and malloc/free. The glibc malloc implementation sadly > expects that all malloc/free are executed from the context of a single > thread while pthread atfork handlers are running, which leads to > interesting hang in glibc.
I'll rework the patch so the modification to tests/test_urcu_fork.c is moved to a separate patch, and merge the fix into the master branch, since it solves a reproduceable problem. Thanks, Mathieu > > Signed-off-by: Mathieu Desnoyers <[email protected]> > --- > diff --git a/tests/test_urcu_fork.c b/tests/test_urcu_fork.c > index 07c521a..6e454b5 100644 > --- a/tests/test_urcu_fork.c > +++ b/tests/test_urcu_fork.c > @@ -85,6 +85,8 @@ int main(int argc, char **argv) > pid_t pid; > int ret; > > +#if 0 > + /* pthread_atfork does not work with malloc/free in callbacks */ > ret = pthread_atfork(call_rcu_before_fork, > call_rcu_after_fork_parent, > call_rcu_after_fork_child); > @@ -93,6 +95,7 @@ int main(int argc, char **argv) > perror("pthread_atfork"); > exit(EXIT_FAILURE); > } > +#endif > > test_rcu(); > > @@ -101,10 +104,12 @@ int main(int argc, char **argv) > fprintf(stderr, "%s parent pid: %d, before fork\n", > argv[0], (int) getpid()); > > + call_rcu_before_fork(); > pid = fork(); > > if (pid == 0) { > /* child */ > + call_rcu_after_fork_child(); > fprintf(stderr, "%s child pid: %d, after fork\n", > argv[0], (int) getpid()); > test_rcu(); > @@ -114,6 +119,7 @@ int main(int argc, char **argv) > int status; > > /* parent */ > + call_rcu_after_fork_parent(); > fprintf(stderr, "%s parent pid: %d, after fork\n", > argv[0], (int) getpid()); > test_rcu(); > diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h > index 6580397..f6625ba 100644 > --- a/urcu-call-rcu-impl.h > +++ b/urcu-call-rcu-impl.h > @@ -75,8 +75,9 @@ static CDS_LIST_HEAD(call_rcu_data_list); > > static DEFINE_URCU_TLS(struct call_rcu_data *, thread_call_rcu_data); > > -/* Guard call_rcu thread creation. */ > - > +/* > + * Guard call_rcu thread creation and atfork handlers. > + */ > static pthread_mutex_t call_rcu_mutex = PTHREAD_MUTEX_INITIALIZER; > > /* If a given thread does not have its own call_rcu thread, this is default. > */ > @@ -254,6 +255,21 @@ static void *call_rcu_thread(void *arg) > struct cds_wfcq_node *cbs, *cbs_tmp_n; > enum cds_wfcq_ret splice_ret; > > + if (uatomic_read(&crdp->flags) & URCU_CALL_RCU_PAUSE) { > + /* > + * Pause requested. Become quiescent: remove > + * ourself from all global lists, and don't > + * process any callback. The callback lists may > + * still be non-empty though. > + */ > + rcu_unregister_thread(); > + cmm_smp_mb__before_uatomic_or(); > + uatomic_or(&crdp->flags, URCU_CALL_RCU_PAUSED); > + while ((uatomic_read(&crdp->flags) & > URCU_CALL_RCU_PAUSE) != 0) > + poll(NULL, 0, 1); > + rcu_register_thread(); > + } > + > cds_wfcq_init(&cbs_tmp_head, &cbs_tmp_tail); > splice_ret = __cds_wfcq_splice_blocking(&cbs_tmp_head, > &cbs_tmp_tail, &crdp->cbs_head, &crdp->cbs_tail); > @@ -705,12 +721,25 @@ void free_all_cpu_call_rcu_data(void) > > /* > * Acquire the call_rcu_mutex in order to ensure that the child sees > - * all of the call_rcu() data structures in a consistent state. > + * all of the call_rcu() data structures in a consistent state. Ensure > + * that all call_rcu threads are in a quiescent state across fork. > * Suitable for pthread_atfork() and friends. > */ > void call_rcu_before_fork(void) > { > + struct call_rcu_data *crdp; > + > call_rcu_lock(&call_rcu_mutex); > + > + cds_list_for_each_entry(crdp, &call_rcu_data_list, list) { > + uatomic_or(&crdp->flags, URCU_CALL_RCU_PAUSE); > + cmm_smp_mb__after_uatomic_or(); > + wake_call_rcu_thread(crdp); > + } > + cds_list_for_each_entry(crdp, &call_rcu_data_list, list) { > + while ((uatomic_read(&crdp->flags) & URCU_CALL_RCU_PAUSED) == 0) > + poll(NULL, 0, 1); > + } > } > > /* > @@ -720,6 +749,10 @@ void call_rcu_before_fork(void) > */ > void call_rcu_after_fork_parent(void) > { > + struct call_rcu_data *crdp; > + > + cds_list_for_each_entry(crdp, &call_rcu_data_list, list) > + uatomic_and(&crdp->flags, ~URCU_CALL_RCU_PAUSE); > call_rcu_unlock(&call_rcu_mutex); > } > > @@ -752,7 +785,11 @@ void call_rcu_after_fork_child(void) > rcu_set_pointer(&per_cpu_call_rcu_data, NULL); > URCU_TLS(thread_call_rcu_data) = NULL; > > - /* Dispose of all of the rest of the call_rcu_data structures. */ > + /* > + * Dispose of all of the rest of the call_rcu_data structures. > + * Leftover call_rcu callbacks will be merged into the new > + * default call_rcu thread queue. > + */ > cds_list_for_each_entry_safe(crdp, next, &call_rcu_data_list, list) { > if (crdp == default_call_rcu_data) > continue; > diff --git a/urcu-call-rcu.h b/urcu-call-rcu.h > index 1dad0e2..997bb2f 100644 > --- a/urcu-call-rcu.h > +++ b/urcu-call-rcu.h > @@ -44,10 +44,12 @@ struct call_rcu_data; > > /* Flag values. */ > > -#define URCU_CALL_RCU_RT 0x1 > -#define URCU_CALL_RCU_RUNNING 0x2 > -#define URCU_CALL_RCU_STOP 0x4 > -#define URCU_CALL_RCU_STOPPED 0x8 > +#define URCU_CALL_RCU_RT (1U << 0) > +#define URCU_CALL_RCU_RUNNING (1U << 1) > +#define URCU_CALL_RCU_STOP (1U << 2) > +#define URCU_CALL_RCU_STOPPED (1U << 3) > +#define URCU_CALL_RCU_PAUSE (1U << 4) > +#define URCU_CALL_RCU_PAUSED (1U << 5) > > /* > * The rcu_head data structure is placed in the structure to be freed > -- > Mathieu Desnoyers > Operating System Efficiency R&D Consultant > EfficiOS Inc. > http://www.efficios.com > > _______________________________________________ > lttng-dev mailing list > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
