On 2024-10-04 00:29, Steven Rostedt wrote:
On Thu, 3 Oct 2024 11:16:35 -0400
[...[
-#define __DO_TRACE(name, args, cond, rcuidle) \ +#define __DO_TRACE(name, args, cond, rcuidle, syscall) \ do { \ int __maybe_unused __idx = 0; \ \ @@ -222,8 +224,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) "Bad RCU usage for tracepoint")) \ return; \ \ - /* keep srcu and sched-rcu usage consistent */ \ - preempt_disable_notrace(); \ + if (syscall) { \ + rcu_read_lock_trace(); \ + } else { \ + /* keep srcu and sched-rcu usage consistent */ \ + preempt_disable_notrace(); \ + } \ \I'm thinking we just use rcu_read_lock_trace() and get rid of the preempt_disable and srcu locks for all tracepoints. Oh crap! I should get rid of srcu locking too, as it was only needed for the rcuidle code :-p
How about we do it one step at a time ? First introduce use of the (lightly tested) rcu_read_lock_trace() (at least in comparison with preempt disable RCU) only for syscalls, and if this works well, then eventually consider moving the preempt off users to rcu_read_lock_trace as well ? Of course it should all work well, in theory. But considering the vast number of tracepoints we have in the kernel, I am reluctant to change too many things at once in that area. We may very well be bitten by unforeseen corner-cases. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
