Messages in this thread | | | From | Joel Fernandes <> | Date | Fri, 10 Aug 2018 06:01:34 -0700 | Subject | Re: [PATCH ftrace/core] tracing: irqsoff: Account for additional preempt_disable |
| |
On Fri, Aug 10, 2018 at 5:55 AM, Masami Hiramatsu <mhiramat@kernel.org> wrote: [..] >> >> >> The other way to fix this is to just use SRCU for all tracepoints. >> >> However we can't do that because we can't use NMIs from RCU context. >> >> >> >> Fixes: c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints >> >> and unify their usage") >> >> Fixes: e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use SRCU") >> >> Reported-by: Masami Hiramatsu <mhiramat@kernel.org> >> >> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> >> >> --- >> >> kernel/trace/trace_irqsoff.c | 26 ++++++++++++++++++++++++++ >> >> 1 file changed, 26 insertions(+) >> >> >> >> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c >> >> index 770cd30cda40..ffbf1505d5bc 100644 >> >> --- a/kernel/trace/trace_irqsoff.c >> >> +++ b/kernel/trace/trace_irqsoff.c >> >> @@ -603,14 +603,40 @@ static void irqsoff_tracer_stop(struct trace_array *tr) >> >> */ >> >> static void tracer_hardirqs_on(void *none, unsigned long a0, unsigned long a1) >> >> { >> > >> > To ensure this function must not be preempted even if we increment preempt >> > count, I think you should check irq_disabled() whole this process, put below >> > here. >> > >> > if (unlikely(!irq_disabled())) >> > return; >> > >> > Since irq_disabled() will be checked in irq_trace() anyway, so no problem >> > to return here when !irq_disabled(). >> >> IRQs can never be enabled here. The trace hooks are called only after >> disabling interrupts, or before enabling them. Right? >> > > Even though, it should be verified or atleast commented on the function header.
Ok. ftrace/core has been fixed since since so this patch is outdated and isn't needed any more, but I agree a separate patch doc comment would help make it clear about this fact. Both IRQ and preempt tracepoints work this way, they fire only within their critical section.
| |