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 48bcda684823 ("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/
Link: https://lore.kernel.org/lkml/CAHk-=witPrLcu22dZ93VCyRQonS7+-dFYhQbna=KBa-TAhayMw@mail.gmail.com/
Fixes: a363d27cdbc2 ("tracing: Allow system call tracepoints to handle page faults")
Cc: Linus Torvalds <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Michael Jeanson <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Yonghong Song <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: [email protected]
Cc: Joel Fernandes <[email protected]>
Cc: Jordan Rife <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/[email protected]
Signed-off-by: Mathieu Desnoyers <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 867f3c1ac7dc77cb595d2d0f1ead4384d3dda91f..832f49b56b1f9a5027d3d9a57e4a977135218a3c 100644 (file)
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -209,31 +209,6 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 #define __DO_TRACE_CALL(name, args)    __traceiter_##name(NULL, args)
 #endif /* CONFIG_HAVE_STATIC_CALL */
 
-/*
- * With @syscall=0, the tracepoint callback array dereference is
- * protected by disabling preemption.
- * With @syscall=1, the tracepoint callback array dereference is
- * protected by Tasks Trace RCU, which allows probes to handle page
- * faults.
- */
-#define __DO_TRACE(name, args, cond, syscall)                          \
-       do {                                                            \
-               if (!(cond))                                            \
-                       return;                                         \
-                                                                       \
-               if (syscall)                                            \
-                       rcu_read_lock_trace();                          \
-               else                                                    \
-                       preempt_disable_notrace();                      \
-                                                                       \
-               __DO_TRACE_CALL(name, TP_ARGS(args));                   \
-                                                                       \
-               if (syscall)                                            \
-                       rcu_read_unlock_trace();                        \
-               else                                                    \
-                       preempt_enable_notrace();                       \
-       } while (0)
-
 /*
  * Make sure the alignment of the structure in the __tracepoints section will
  * 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)
        __DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), cond, PARAMS(data_proto)) \
        static inline void trace_##name(proto)                          \
        {                                                               \
-               if (static_branch_unlikely(&__tracepoint_##name.key))   \
-                       __DO_TRACE(name,                                \
-                               TP_ARGS(args),                          \
-                               TP_CONDITION(cond), 0);                 \
+               if (static_branch_unlikely(&__tracepoint_##name.key)) { \
+                       if (cond) {                                     \
+                               scoped_guard(preempt_notrace)           \
+                                       __DO_TRACE_CALL(name, TP_ARGS(args)); \
+                       }                                               \
+               }                                                       \
                if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) {             \
                        WARN_ONCE(!rcu_is_watching(),                   \
                                  "RCU not watching for tracepoint");   \
@@ -297,10 +274,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
        static inline void trace_##name(proto)                          \
        {                                                               \
                might_fault();                                          \
-               if (static_branch_unlikely(&__tracepoint_##name.key))   \
-                       __DO_TRACE(name,                                \
-                               TP_ARGS(args),                          \
-                               TP_CONDITION(cond), 1);                 \
+               if (static_branch_unlikely(&__tracepoint_##name.key)) { \
+                       if (cond) {                                     \
+                               scoped_guard(rcu_tasks_trace)           \
+                                       __DO_TRACE_CALL(name, TP_ARGS(args)); \
+                       }                                               \
+               }                                                       \
                if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) {             \
                        WARN_ONCE(!rcu_is_watching(),                   \
                                  "RCU not watching for tracepoint");   \