Re: call_rcu from trace_preempt

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Tue, Jun 16, 2015 at 02:57:44PM -0400, Steven Rostedt wrote:
> On Tue, 16 Jun 2015 10:39:42 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > > >>If rcu_is_watching() and __rcu_is_watching() are both marked as
> > > >>notrace, it makes sense to use preempt_disable/enable_notrace() as it
> > > >>otherwise defeats the purpose of the notrace markers on rcu_is_watching.
> > 
> > And __rcu_is_watching() is marked notrace as well.
> 
> Isn't that what I said?

You did say "if", so I checked.  ;-)

> > > >>That is regardless of what the rest of this thread is about.
> > > >
> > > >Good enough!  Alexei, are you OK with my adding your Signed-off-by
> > > >to the above patch?
> > > 
> > > sure.
> > > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > > 
> > > >(Still not sold on reentrant call_rcu() and
> > > >kfree_rcu(), but getting notrace set up correctly is worthwhile.)
> > > 
> > > I'm not sold on it either. So far trying to understand
> > > all consequences.
> > 
> > Here is the updated patch.  Steven, I added your "Acked-by" based
> > on your positive comments above, please let me know if you would
> > like me to remove it.
> 
> I'm fine with it. But doesn't Acked-by go above Signed-off-by?

I have done it both ways, usually in time order.

							Thanx, Paul

> -- Steve
> 
> > 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > commit 9611f225d383a2edbdf74ca7f00c8d0b1e56dc45
> > Author: Alexei Starovoitov <ast@plumgrid.com>
> > Date:   Tue Jun 16 10:35:18 2015 -0700
> > 
> >     rcu: Make rcu_is_watching() really notrace
> >     
> >     Although rcu_is_watching() is marked notrace, it invokes preempt_disable()
> >     and preempt_enable(), both of which can be traced.  This defeats the
> >     purpose of the notrace on rcu_is_watching(), so this commit substitutes
> >     preempt_disable_notrace() and preempt_enable_notrace().
> >     
> >     Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >     Acked-by: Steven Rostedt <rostedt@goodmis.org>
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index fc0385380e97..c844ef3c2fae 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -973,9 +973,9 @@ bool notrace rcu_is_watching(void)
> >  {
> >  	bool ret;
> >  
> > -	preempt_disable();
> > +	preempt_disable_notrace();
> >  	ret = __rcu_is_watching();
> > -	preempt_enable();
> > +	preempt_enable_notrace();
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(rcu_is_watching);
>