linux-kernel - Re: call_rcu from trace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150616192054.GQ3913@linux.vnet.ibm.com>
Date:	Tue, 16 Jun 2015 12:20:55 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Alexei Starovoitov <ast@...mgrid.com>,
	Daniel Wagner <daniel.wagner@...-carit.de>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: call_rcu from trace_preempt

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@...ux.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@...nel.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@...mgrid.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@...mgrid.com>
> >     Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> >     Acked-by: Steven Rostedt <rostedt@...dmis.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/