Date: Tue, 16 Jun 2015 12:20:55 -0700
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Alexei Starovoitov <ast@plumgrid.com>,
        Daniel Wagner <daniel.wagner@bmw-carit.de>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: call_rcu from trace_preempt
Message-ID: <20150616192054.GQ3913@linux.vnet.ibm.com>
Reply-To: paulmck@linux.vnet.ibm.com
References: <20150615230702.GB3913@linux.vnet.ibm.com>
 <557F7764.5060707@plumgrid.com>
 <20150616021458.GE3913@linux.vnet.ibm.com>
 <557FB7E1.6080004@plumgrid.com>
 <20150616122733.GG3913@linux.vnet.ibm.com>
 <20150616113738.3ab2667b@gandalf.local.home>
 <20150616160546.GL3913@linux.vnet.ibm.com>
 <55805960.2030008@plumgrid.com>
 <20150616173942.GP3913@linux.vnet.ibm.com>
 <20150616145744.457ab89d@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150616145744.457ab89d@gandalf.local.home>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2853
Lines: 83

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/