From mboxrd@z Thu Jan  1 00:00:00 1970
From: paulmck@linux.vnet.ibm.com (Paul E. McKenney)
Date: Thu, 29 Sep 2011 11:08:25 -0700
Subject: [ltt-dev] [PATCH 6/7] urcu,
	call_rcu: Cleanup call_rcu_data pointers	before used in child
In-Reply-To: <20110929175912.GB7066@Krystal>
References: <cover.1317198301.git.laijs@cn.fujitsu.com>
	<0f645da2f35f5c974a7059655e7b2e199cf98f72.1317198301.git.laijs@cn.fujitsu.com>
	<20110929173757.GD2362@linux.vnet.ibm.com>
	<20110929175912.GB7066@Krystal>
Message-ID: <20110929180825.GH2362@linux.vnet.ibm.com>

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.

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > 							Thanx, Paul
> > 
> > > +
> > >  	/* Dispose of all of the rest of the call_rcu_data structures. */
> > >  	cds_list_for_each_entry_safe(crdp, next, &call_rcu_data_list, list) {
> > >  		if (crdp == default_call_rcu_data)
> > > -- 
> > > 1.7.4.4
> > > 
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
>