Skip to content

Commit 98bf0fb

Browse files
compudjrostedt
authored andcommitted
tracing: Remove conditional locking from __DO_TRACE()
Remove conditional locking by moving the __DO_TRACE() code into trace_##name(). When the faultable syscall tracepoints were implemented, __DO_TRACE() had a rcuidle argument which selected between SRCU and preempt disable. Therefore, the RCU tasks trace protection for faultable syscall tracepoints was introduced using the same pattern. At that point, it did not appear obvious that this feedback from Linus [1] applied here as well, because the __DO_TRACE() modification was extending a pre-existing pattern. Shortly before pulling the faultable syscall tracepoints modifications, Steven removed the rcuidle argument and SRCU protection scheme entirely from tracepoint.h: commit 48bcda6 ("tracing: Remove definition of trace_*_rcuidle()") This required a rebase of the faultable syscall tracepoints series, which missed a perfect opportunity to integrate the prior recommendation from Linus. In response to the pull request, Linus pointed out [2] that he was not pleased by the implementation, expecting this to be fixed in a follow up patch series. Move __DO_TRACE() code into trace_##name() within each of __DECLARE_TRACE() and __DECLARE_TRACE_SYSCALL(). Use a scoped guard to guard the preempt disable notrace and RCU tasks trace critical sections. Link: https://lore.kernel.org/all/CAHk-=wggDLDeTKbhb5hh--x=-DQd69v41137M72m6NOTmbD-cw@mail.gmail.com/ [1] Link: https://lore.kernel.org/lkml/CAHk-=witPrLcu22dZ93VCyRQonS7+-dFYhQbna=KBa-TAhayMw@mail.gmail.com/ [2] Fixes: a363d27 ("tracing: Allow system call tracepoints to handle page faults") Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Michael Jeanson <mjeanson@efficios.com> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Yonghong Song <yhs@fb.com> Cc: Paul E. McKenney <paulmck@kernel.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com> Cc: bpf@vger.kernel.org Cc: Joel Fernandes <joel@joelfernandes.org> Cc: Jordan Rife <jrife@google.com> Cc: linux-trace-kernel@vger.kernel.org Link: https://lore.kernel.org/20241123153031.2884933-5-mathieu.desnoyers@efficios.com Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent 7c565a4 commit 98bf0fb

File tree

1 file changed

+12
-33
lines changed

1 file changed

+12
-33
lines changed

include/linux/tracepoint.h

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -209,31 +209,6 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
209209
#define __DO_TRACE_CALL(name, args) __traceiter_##name(NULL, args)
210210
#endif /* CONFIG_HAVE_STATIC_CALL */
211211

212-
/*
213-
* With @syscall=0, the tracepoint callback array dereference is
214-
* protected by disabling preemption.
215-
* With @syscall=1, the tracepoint callback array dereference is
216-
* protected by Tasks Trace RCU, which allows probes to handle page
217-
* faults.
218-
*/
219-
#define __DO_TRACE(name, args, cond, syscall) \
220-
do { \
221-
if (!(cond)) \
222-
return; \
223-
\
224-
if (syscall) \
225-
rcu_read_lock_trace(); \
226-
else \
227-
preempt_disable_notrace(); \
228-
\
229-
__DO_TRACE_CALL(name, TP_ARGS(args)); \
230-
\
231-
if (syscall) \
232-
rcu_read_unlock_trace(); \
233-
else \
234-
preempt_enable_notrace(); \
235-
} while (0)
236-
237212
/*
238213
* Make sure the alignment of the structure in the __tracepoints section will
239214
* not add unwanted padding between the beginning of the section and the
@@ -282,10 +257,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
282257
__DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), cond, PARAMS(data_proto)) \
283258
static inline void trace_##name(proto) \
284259
{ \
285-
if (static_branch_unlikely(&__tracepoint_##name.key)) \
286-
__DO_TRACE(name, \
287-
TP_ARGS(args), \
288-
TP_CONDITION(cond), 0); \
260+
if (static_branch_unlikely(&__tracepoint_##name.key)) { \
261+
if (cond) { \
262+
scoped_guard(preempt_notrace) \
263+
__DO_TRACE_CALL(name, TP_ARGS(args)); \
264+
} \
265+
} \
289266
if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
290267
WARN_ONCE(!rcu_is_watching(), \
291268
"RCU not watching for tracepoint"); \
@@ -297,10 +274,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
297274
static inline void trace_##name(proto) \
298275
{ \
299276
might_fault(); \
300-
if (static_branch_unlikely(&__tracepoint_##name.key)) \
301-
__DO_TRACE(name, \
302-
TP_ARGS(args), \
303-
TP_CONDITION(cond), 1); \
277+
if (static_branch_unlikely(&__tracepoint_##name.key)) { \
278+
if (cond) { \
279+
scoped_guard(rcu_tasks_trace) \
280+
__DO_TRACE_CALL(name, TP_ARGS(args)); \
281+
} \
282+
} \
304283
if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
305284
WARN_ONCE(!rcu_is_watching(), \
306285
"RCU not watching for tracepoint"); \

0 commit comments

Comments
 (0)