Re: [PATCH v12 0/3] tracing: Centralize preemptirq tracepoints and unify their usage

[Joel Fernandes]

Hi Masami,

On Thu, Aug 2, 2018 at 7:55 AM, Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> Hi Joel,
>
> I found this caused several issues when testing ftrace.
>
> #1) ftrace boottest (FTRACE_STARTUP_TEST) fails

This sadly appears to be a real issue. The startup test for
"preemptirqsoff" tracer fails, however it passes for only preemptoff
or only irqsoff. I tested only the last 2 tracers, not the first one,
that's why I didn't catch it. I need to debug this more.

> #2) mmiotrace reports "IRQs not enabled as expected" error
> #3) lock subsystem event boottest causes "IRQs not disabled as expected" error (sometimes)

Could you try the below patch and let me know if you still see the
issue? In the v11 I removed the lockdep_recursing() check since it
seemed unnecessary. But I'd like to rule out that there's still some
issue lurking there. Thanks and I appreciate your help, diff is
attached to this email.

> #4) ftracetest test.d/event/toplevel-enable.tc causes "suspicious RCU usage" warning

I sent a patch fixing this with you on CC. I tested that it fixes the
issue you're reporting.
diff --git a/kernel/trace/trace_preemptirq.c b/kernel/trace/trace_preemptirq.c
index e76b78bf258e..13e2c6e99465 100644
--- a/kernel/trace/trace_preemptirq.c
+++ b/kernel/trace/trace_preemptirq.c
@@ -19,7 +19,7 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu);
 
 void trace_hardirqs_on(void)
 {
-	if (!this_cpu_read(tracing_irq_cpu))
+	if (lockdep_recursing(current) || !this_cpu_read(tracing_irq_cpu))
 		return;
 
 	trace_irq_enable_rcuidle(CALLER_ADDR0, CALLER_ADDR1);
@@ -29,7 +29,7 @@ EXPORT_SYMBOL(trace_hardirqs_on);
 
 void trace_hardirqs_off(void)
 {
-	if (this_cpu_read(tracing_irq_cpu))
+	if (lockdep_recursing(current) || this_cpu_read(tracing_irq_cpu))
 		return;
 
 	this_cpu_write(tracing_irq_cpu, 1);
@@ -39,7 +39,7 @@ EXPORT_SYMBOL(trace_hardirqs_off);
 
 __visible void trace_hardirqs_on_caller(unsigned long caller_addr)
 {
-	if (!this_cpu_read(tracing_irq_cpu))
+	if (lockdep_recursing(current) || !this_cpu_read(tracing_irq_cpu))
 		return;
 
 	trace_irq_enable_rcuidle(CALLER_ADDR0, caller_addr);
@@ -49,7 +49,7 @@ EXPORT_SYMBOL(trace_hardirqs_on_caller);
 
 __visible void trace_hardirqs_off_caller(unsigned long caller_addr)
 {
-	if (this_cpu_read(tracing_irq_cpu))
+	if (lockdep_recursing(current) || this_cpu_read(tracing_irq_cpu))
 		return;
 
 	this_cpu_write(tracing_irq_cpu, 1);