Re: [PATCH] tracing, hardirq: Do not test lockdep on irq trace points when disabled

[Steven Rostedt]

On Wed, 8 Mar 2023 15:39:25 +0100
Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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