tracing: Add a no-rcu-check version of trace_##event##_enabled()
Steven Rostedt
maintainerPATCH 1/1 tracing: Add a no-rcu-check version of trace_##event##_enabled()
From: Steven Rostedt <[email protected]>
Tracepoints require that RCU is watching. To prevent them from being used in places that RCU is not watching, the trace_##event() macro always calls rcu_is_watching() even when the event is not enabled and warns if RCU is not watching. This is to make sure a warning is triggered even if the tracepoint is never enabled (as it is only a bug when it is).
It was noticed that tracepoints could be hidden within trace_#event#_enabled() calls, which are used to do extra work for the tracepoint only if the tracepoint is enabled. But this also can hide the fact that a tracepoint is placed in a location that can be called when RCU is not watching.
Commit 9764e731ef6ab ("tracepoint: Add lockdep rcu_is_watching() check to trace_##name##_enabled()") added a check to the trace_##event##_enabled() macro to make sure RCU is watching when it is called to make sure not to hide the bug of a tracepoint being called when RCU is not watching.
There is one case in the irq_disable tracepoint where it is within a trace_irq_disable_enabled() block, but it checks if RCU is watching, and if it isn't, it makes a call to ct_irq_enter() that makes RCU watch again. But because trace_irq_disable_enabled() now checks if RCU is watching and will trigger if it isn't. This is a false warning as the code within the block handles this case.
Add a new internal macro __trace_##event##_enabled() that doesn't check if RCU is watching, and convert the irq_enable/disable tracepoints over to it.
--- include/linux/tracepoint.h | 12 +++++++++++- kernel/trace/trace_preemptirq.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)
include/linux/tracepoint.h +11 -1
@@ -292,13 +292,18 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) { \ } \ static inline bool \ + __trace_##name##_enabled(void) \ + { \ + return static_branch_unlikely(&__tracepoint_##name.key);\ + } \ + static inline bool \ trace_##name##_enabled(void) \ { \ if (IS_ENABLED(CONFIG_LOCKDEP)) { \ WARN_ONCE(!rcu_is_watching(), \ "RCU not watching for tracepoint"); \ } \ - return static_branch_unlikely(&__tracepoint_##name.key);\ + return __trace_##name##_enabled(); \ }#define __DECLARE_TRACE (name, proto, args, cond, data_proto) \
@@ -457,6 +462,11 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) { \ } \ static inline bool \ + __trace_##name##_enabled(void) \ + { \ + return false; \ + } \ + static inline bool \ trace_##name##_enabled(void) \ { \ return false; \
kernel/trace/trace_preemptirq.c +1 -1
@@ -30,7 +30,7 @@#else #define trace (point, args) \ do { \ - if (trace_##point##_enabled()) { \ + if (__trace_##point##_enabled()) { \ bool exit_rcu = false; \ if (in_nmi()) \ break; \
Signature · 1 line
2.53.0
Quoted 26 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Tracepoints require that RCU is watching. To prevent them from being used in places that RCU is not watching, the trace_##event() macro always calls
rcu_is_watching()even when the event is not enabled and warns if RCU is not watching. This is to make sure a warning is triggered even if the tracepoint is never enabled (as it is only a bug when it is).It was noticed that tracepoints could be hidden within trace_#event#_enabled() calls, which are used to do extra work for the tracepoint only if the tracepoint is enabled. But this also can hide the fact that a tracepoint is placed in a location that can be called when RCU is not watching.
Commit
9764e731ef6ab("tracepoint: Add lockdeprcu_is_watching()check to trace_##name##_enabled()") added a check to the trace_##event##_enabled() macro to make sure RCU is watching when it is called to make sure not to hide the bug of a tracepoint being called when RCU is not watching.There is one case in the
irq_disabletracepoint where it is within atrace_irq_disable_enabled()block, but it checks if RCU is watching, and if it isn't, it makes a call toct_irq_enter()that makes RCU watch again. But becausetrace_irq_disable_enabled()now checks if RCU is watching and will trigger if it isn't. This is a false warning as the code within the block handles this case.
Quoted 8 lines
Add a new internal macro __trace_##event##_enabled() that doesn't check if RCU is watching, and convert the irq_enable/disable tracepoints over to it.
Reported-by Geert Uytterhoeven <[email protected]>Closes https://lore.kernel.org/all/CAMuHMdXud_RpWag_hFqa2ByBGRxg6KnxGL1ObCWZrpTsk3TfAw@mail.gmail.com/ open hereFixes 9764e731ef6ab ("tracepoint: Add lockdep rcu_is_watching() check to trace_##name##_enabled()")Signed-off-by Steven Rostedt <[email protected]>
Thanks, warning gone on all affected platforms.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds