Re: [PATCH v3 35/51] trace,hardirq: No moar _rcuidle() tracing
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2023-01-17 04:25:25
Also in:
linux-acpi, linux-alpha, linux-arch, linux-arm-msm, linux-clk, linux-m68k, linux-mips, linux-mm, linux-omap, linux-perf-users, linux-pm, linux-riscv, linux-s390, linux-samsung-soc, linux-sh, linux-tegra, linux-um, linuxppc-dev, lkml, loongarch, sparclinux
Hi Peter, On Thu, 12 Jan 2023 20:43:49 +0100 Peter Zijlstra [off-list ref] wrote:
Robot reported that trace_hardirqs_{on,off}() tickle the forbidden
_rcuidle() tracepoint through local_irq_{en,dis}able().
For 'sane' configs, these calls will only happen with RCU enabled and
as such can use the regular tracepoint. This also means it's possible
to trace them from NMI context again.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>The code looks good to me. I just have a question about comment.
quoted hunk ↗ jump to hunk
--- kernel/trace/trace_preemptirq.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)--- a/kernel/trace/trace_preemptirq.c +++ b/kernel/trace/trace_preemptirq.c@@ -20,6 +20,15 @@ static DEFINE_PER_CPU(int, tracing_irq_cpu); /* + * ...
Is this intended? Wouldn't you leave any comment here? Thank you,
quoted hunk ↗ jump to hunk
+ */ +#ifdef CONFIG_ARCH_WANTS_NO_INSTR +#define trace(point) trace_##point +#else +#define trace(point) if (!in_nmi()) trace_##point##_rcuidle +#endif + +/* * Like trace_hardirqs_on() but without the lockdep invocation. This is * used in the low level entry code where the ordering vs. RCU is important * and lockdep uses a staged approach which splits the lockdep hardirq@@ -28,8 +37,7 @@ static DEFINE_PER_CPU(int, tracing_irq_c void trace_hardirqs_on_prepare(void) { if (this_cpu_read(tracing_irq_cpu)) { - if (!in_nmi()) - trace_irq_enable(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1); tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); this_cpu_write(tracing_irq_cpu, 0); }@@ -40,8 +48,7 @@ NOKPROBE_SYMBOL(trace_hardirqs_on_prepar void trace_hardirqs_on(void) { if (this_cpu_read(tracing_irq_cpu)) { - if (!in_nmi()) - trace_irq_enable_rcuidle(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_enable)(CALLER_ADDR0, CALLER_ADDR1); tracer_hardirqs_on(CALLER_ADDR0, CALLER_ADDR1); this_cpu_write(tracing_irq_cpu, 0); }@@ -63,8 +70,7 @@ void trace_hardirqs_off_finish(void) if (!this_cpu_read(tracing_irq_cpu)) { this_cpu_write(tracing_irq_cpu, 1); tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); - if (!in_nmi()) - trace_irq_disable(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1); } }@@ -78,8 +84,7 @@ void trace_hardirqs_off(void) if (!this_cpu_read(tracing_irq_cpu)) { this_cpu_write(tracing_irq_cpu, 1); tracer_hardirqs_off(CALLER_ADDR0, CALLER_ADDR1); - if (!in_nmi()) - trace_irq_disable_rcuidle(CALLER_ADDR0, CALLER_ADDR1); + trace(irq_disable)(CALLER_ADDR0, CALLER_ADDR1); } } EXPORT_SYMBOL(trace_hardirqs_off);
-- Masami Hiramatsu (Google) [off-list ref]