| From c2679254b9c9980d9045f0f722cf093a2b1f7590 Mon Sep 17 00:00:00 2001 |
| From: "Steven Rostedt (Google)" <rostedt@goodmis.org> |
| Date: Fri, 10 Mar 2023 17:28:56 -0500 |
| Subject: tracing: Make tracepoint lockdep check actually test something |
| |
| From: Steven Rostedt (Google) <rostedt@goodmis.org> |
| |
| commit c2679254b9c9980d9045f0f722cf093a2b1f7590 upstream. |
| |
| A while ago where the trace events had the following: |
| |
| rcu_read_lock_sched_notrace(); |
| rcu_dereference_sched(...); |
| rcu_read_unlock_sched_notrace(); |
| |
| If the tracepoint is enabled, it could trigger RCU issues if called in |
| the wrong place. And this warning was only triggered if lockdep was |
| enabled. If the tracepoint was never enabled with lockdep, the bug would |
| not be caught. To handle this, the above sequence was done when lockdep |
| was enabled regardless if the tracepoint was enabled or not (although the |
| always enabled code really didn't do anything, it would still trigger a |
| warning). |
| |
| But a lot has changed since that lockdep code was added. One is, that |
| sequence no longer triggers any warning. Another is, the tracepoint when |
| enabled doesn't even do that sequence anymore. |
| |
| The main check we care about today is whether RCU is "watching" or not. |
| So if lockdep is enabled, always check if rcu_is_watching() which will |
| trigger a warning if it is not (tracepoints require RCU to be watching). |
| |
| Note, that old sequence did add a bit of overhead when lockdep was enabled, |
| and with the latest kernel updates, would cause the system to slow down |
| enough to trigger kernel "stalled" warnings. |
| |
| Link: http://lore.kernel.org/lkml/20140806181801.GA4605@redhat.com |
| Link: http://lore.kernel.org/lkml/20140807175204.C257CAC5@viggo.jf.intel.com |
| Link: https://lore.kernel.org/lkml/20230307184645.521db5c9@gandalf.local.home/ |
| Link: https://lore.kernel.org/linux-trace-kernel/20230310172856.77406446@gandalf.local.home |
| |
| Cc: stable@vger.kernel.org |
| Cc: Masami Hiramatsu <mhiramat@kernel.org> |
| Cc: Dave Hansen <dave.hansen@linux.intel.com> |
| Cc: "Paul E. McKenney" <paulmck@kernel.org> |
| Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
| Cc: Joel Fernandes <joel@joelfernandes.org> |
| Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> |
| Acked-by: Paul E. McKenney <paulmck@kernel.org> |
| Fixes: e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use SRCU") |
| Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| include/linux/tracepoint.h | 15 ++++++--------- |
| 1 file changed, 6 insertions(+), 9 deletions(-) |
| |
| --- a/include/linux/tracepoint.h |
| +++ b/include/linux/tracepoint.h |
| @@ -231,12 +231,11 @@ static inline struct tracepoint *tracepo |
| * not add unwanted padding between the beginning of the section and the |
| * structure. Force alignment to the same alignment as the section start. |
| * |
| - * When lockdep is enabled, we make sure to always do the RCU portions of |
| - * the tracepoint code, regardless of whether tracing is on. However, |
| - * don't check if the condition is false, due to interaction with idle |
| - * instrumentation. This lets us find RCU issues triggered with tracepoints |
| - * even when this tracepoint is off. This code has no purpose other than |
| - * poking RCU a bit. |
| + * When lockdep is enabled, we make sure to always test if RCU is |
| + * "watching" regardless if the tracepoint is enabled or not. Tracepoints |
| + * require RCU to be active, and it should always warn at the tracepoint |
| + * site if it is not watching, as it will need to be active when the |
| + * tracepoint is enabled. |
| */ |
| #define __DECLARE_TRACE(name, proto, args, cond, data_proto) \ |
| extern int __traceiter_##name(data_proto); \ |
| @@ -249,9 +248,7 @@ static inline struct tracepoint *tracepo |
| TP_ARGS(args), \ |
| TP_CONDITION(cond), 0); \ |
| if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \ |
| - rcu_read_lock_sched_notrace(); \ |
| - rcu_dereference_sched(__tracepoint_##name.funcs);\ |
| - rcu_read_unlock_sched_notrace(); \ |
| + WARN_ON_ONCE(!rcu_is_watching()); \ |
| } \ |
| } \ |
| __DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args), \ |