[ltt-dev] [PATCH 6/7] urcu, call_rcu: Cleanup call_rcu_data pointers before used in child

[Paul E. McKenney]

On Thu, Sep 29, 2011 at 02:19:47PM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > On Thu, Sep 29, 2011 at 01:59:12PM -0400, Mathieu Desnoyers wrote:
> > > * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > > > On Wed, Sep 28, 2011 at 04:34:32PM +0800, Lai Jiangshan wrote:
> > > > > Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> > > > > ---
> > > > >  urcu-call-rcu-impl.h |    6 ++++++
> > > > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> > > > > index 65c1c7a..3c68ae7 100644
> > > > > --- a/urcu-call-rcu-impl.h
> > > > > +++ b/urcu-call-rcu-impl.h
> > > > > @@ -686,6 +686,12 @@ void call_rcu_after_fork_child(void)
> > > > >  	default_call_rcu_data = NULL;
> > > > >  	(void)get_default_call_rcu_data();
> > > > > 
> > > > > +	/* Cleanup call_rcu_data pointers before used */
> > > > > +	maxcpus = 0;
> > > > > +	free(per_cpu_call_rcu_data);
> > > > > +	per_cpu_call_rcu_data = NULL;
> > > > 
> > > > Good catch!  I would expect that the number of CPUs would be the
> > > > same for the child as it was for the parent, but doesn't hurt to
> > > > make the child start over.
> > > > 
> > > > > +	thread_call_rcu_data = NULL;
> > > > 
> > > > Isn't thread_call_rcu_data already NULL due to the child being a new
> > > > thread and the C initialization-to-zero rules?
> > > 
> > > #include <unistd.h>
> > > #include <stdio.h>
> > > 
> > > int __thread a;
> > > 
> > > int main()
> > > {
> > > 	 int pid;
> > > 
> > > 	 a = 1;
> > > 	 pid = fork();
> > > 	 if (pid > 0) {
> > > 		  printf("parent val %d\n", a);
> > > 		  return 0;
> > > 	 } else {
> > > 		  printf("child val %d\n", a);
> > > 		  return 0;
> > > 	 }
> > > }
> > > 
> > > compudj at thinkos:/tmp$ gcc -o test test.c
> > > compudj at thinkos:/tmp$ ./test
> > > parent val 1
> > > child val 1
> > > 
> > > AFAIK, the c initialization to zero rules apply to exec(), not fork.
> > > Here we are taking care of after-fork in the child, in cases where the
> > > child is not doing an exec.
> > > 
> > > Does that make more sense ?
> > 
> > I would think that the forked child would be a new thread, but obviously
> > not!  I stand corrected.
> 
> It all makes more sense when you consider the following paragraph of
> pthread_atfork(3):
> 
>        To understand the purpose of pthread_atfork, recall that fork(2) dupli‐
>        cates the whole memory space, including mutexes in their current  lock‐
>        ing  state,  but only the calling thread: other threads are not running
>        in the child process.  The mutexes are not usable after  the  fork  and
>        must be initialized with pthread_mutex_init in the child process.  This
>        is a limitation of the current implementation and might or might not be
>        present in future versions.

I clearly need to get out of the kernel more often.  ;-)

							Thanx, Paul