On Fri, Aug 10, 2018 at 5:55 AM, Masami Hiramatsu <mhira...@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 <mhira...@kernel.org>
>> >> Signed-off-by: Joel Fernandes (Google) <j...@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.

Reply via email to