Re: [PATCH] tracing/perf: Move rcu_irq_enter/exit_irqson() to perf trace point hook

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Feb 10, 2020, at 9:22 PM, rostedt rostedt@goodmis.org wrote:

> On Mon, 10 Feb 2020 19:30:32 -0500 (EST)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> ----- On Feb 10, 2020, at 5:06 PM, rostedt rostedt@goodmis.org wrote:
>> 
>> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>> 
>> Hi Steven,
> 
> Hi Mathieu!
[...]
>> 
>> Which brings a question about handling of NMIs: in the proposed patch, if
>> a NMI nests over rcuidle context, AFAIU it will be in a state
>> !rcu_is_watching() && in_nmi(), which is handled by this patch with a simple
>> "return", meaning important NMIs doing hardware event sampling can be
>> completely lost.
>> 
>> Considering that we cannot use rcu_irq_enter/exit_irqson() from NMI context,
>> is it at all valid to use rcu_read_lock/unlock() as perf does from NMI handlers,
>> considering that those can be nested on top of rcuidle context ?
>> 
> 
> Note, in the __DO_TRACE macro, we've had this for a long time:
> 
>		/* srcu can't be used from NMI */			\
>		WARN_ON_ONCE(rcuidle && in_nmi());			\
> 
> With nothing triggering.

The "rcuidle" argument is only true for tracepoints which are declared to be used
within the rcuidle code. AFAIK, it does not cover tracepoints which can be placed
in NMI handlers. The state I am concerned about is really:

WARN_ON_ONCE(!rcu_is_watching() && in_nmi())

As pointed out by Peter further down in this thread.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com