Messages in this thread Patch in this message | | | Date | Wed, 8 Mar 2023 11:56:06 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH] tracing, hardirq: Do not test lockdep on irq trace points when disabled |
| |
On Wed, 8 Mar 2023 15:39:25 +0100 Peter Zijlstra <peterz@infradead.org> wrote:
> > if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \ > > rcu_read_lock_sched_notrace(); \ > > rcu_dereference_sched(__tracepoint_##name.funcs);\ > > rcu_read_unlock_sched_notrace(); \ > > } \ > > } \ > > > Where it will test lockdep for trace points even when they are not > > enabled, to make sure they do not cause RCU issues, and lockdep will > > trigger even when the trace points are not enabled. > > I'm confused what that's actually trying to do.. > > You're not tickling the rcu_is_watching() check, because > rcu_read_lock_sched_notrace() doesn't have that. You're not tickling > RCU_LOCKDEP_WARN() because you did rcu_read_lock_sched_notrace(). > > So what?!?
Actually, I think the proper changes is to just add "rcu_is_watching()" warning here?
That code is from 2014 where it simulated what was done in DO_TRACE() and I think back then, those calls did trigger warnings. But this code has not kept up with the changes in DO_TRACE.
So something like this instead?
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h index e299f29375bb..d3a221158ab1 100644 --- a/include/linux/tracepoint.h +++ b/include/linux/tracepoint.h @@ -260,9 +260,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) 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), \
-- Steve
| |