Re: [RFC] ftrace / perf 'recursion'

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Aug 17, 2016 at 09:49:32AM -0400, Steven Rostedt wrote:
> On Wed, 17 Aug 2016 12:57:16 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 

> > +static inline notrace void __smp_irq_work_interrupt(void)
> 
> FYI, anything marked "inline" is also marked "notrace", because it only
> gets traced if gcc decides not to inline it, and because that
> "randomness" caused issues in the past, we define all "inline"s to
> include "notrace" so a function marked inline will never be traced
> regardless if gcc decides not to inline it.

Ah, missed that.

> > +static inline notrace void exiting_irq_work(void)
> > +{
> > +#ifdef CONFIG_TRACING
> > +	if (unlikely(1 /* function_tracing_enabled() */)) {
> > +		unsigned long trace_recursion = current->trace_recursion;
> > +
> > +		current->trace_recursion |= 1 << 10; /* TRACE_INTERNAL_IRQ_BIT */
> > +		barrier();
> > +		exiting_irq();
> > +		barrier();
> > +		current->trace_recursion = trace_recursion;
> > +		return;
> > +	}
> > +#endif
> 
> yuck. 

Well, yes ;-)

> This looks very fragile. What happens if perf gets hooked to
> function graph tracing, then this wont help that on function exit.

Not sure what you mean, all callers of this are also notrace. There
should not be any return trampoline pending.

> Also, it will prevent any tracing of NMIs that occur in there.

It should not, see how I only mark the IRQ bit, not the NMI bit.

Could be I misunderstand your recursion bits though....

> I would really like to keep this fix within perf if possible. If
> anything, the flag should just tell the perf function handler not to
> trace, this shouldn't stop all function handlers.

Well, my thinking was that there's a reason most of irq_work is already
notrace. kernel/irq_work.c has CC_FLAGS_FTRACE removed. That seems to
suggest that tracing irq_work is a problem.

tracing also seems to use irq_work..