Re: call_rcu from trace_preempt

Paul E. McKenney
2015-06-16T19:21:11Z

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);
>