Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 29 Sep 2020 10:54:16 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@kernel.org>, kim.phillips@amd.com
Subject: Re: [PATCH] rcu,ftrace: Fix ftrace recursion
Message-ID: <20200929105416.757c47f0@gandalf.local.home>
In-Reply-To: <20200929144105.GU29330@paulmck-ThinkPad-P72>
References: <20200929113340.GN2628@hirez.programming.kicks-ass.net>
        <20200929103620.06762622@gandalf.local.home>
        <20200929144105.GU29330@paulmck-ThinkPad-P72>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 29 Sep 2020 07:41:06 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> On Tue, Sep 29, 2020 at 10:36:20AM -0400, Steven Rostedt wrote:
> > On Tue, 29 Sep 2020 13:33:40 +0200
> > Peter Zijlstra <peterz@infradead.org> wrote:
> >   
> > > Kim reported that perf-ftrace made his box unhappy. It turns out that
> > > commit:
> > > 
> > >   ff5c4f5cad33 ("rcu/tree: Mark the idle relevant functions noinstr")
> > > 
> > > removed one too many notrace. Probably due to there not being a helpful
> > > comment.
> > > 
> > > Reinstate the notrace and add a comment to avoid loosing it again.
> > > 
> > > Fixes: ff5c4f5cad33 ("rcu/tree: Mark the idle relevant functions noinstr")
> > > Reported-by: Kim Phillips <kim.phillips@amd.com>
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > ---
> > >  kernel/rcu/tree.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index ee5e595501e8..33020d84ec6b 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -1098,8 +1098,11 @@ noinstr bool __rcu_is_watching(void)
> > >   * CPU can safely enter RCU read-side critical sections.  In other words,
> > >   * if the current CPU is not in its idle loop or is in an interrupt or
> > >   * NMI handler, return true.
> > > + *
> > > + * Must be notrace because __ftrace_ops_list_func() / ftrace_ops_assist_func()
> > > + * will call this (for every function) outside of recursion protection.
> > >   */
> > > -bool rcu_is_watching(void)
> > > +notrace bool rcu_is_watching(void)
> > >  {
> > >  	bool ret;
> > >    
> > 
> > I think the patch I suggested is more suitable.  
> 
> OK, I will let you guys fight it out.  ;-)
> 

Well, I think we should actually apply both, but the comment needs to be
updated, as it will no longer be outside recursion. And the comment is
wrong now as well, as its only outside recursion protection for the
assist_func(). 

But it does prevent it from being always called for perf.

 * Make notrace because it can be called by the internal functions of
 * ftrace, and making this notrace removes unnecessary recursion calls.


-- Steve