bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Jeanson <mjeanson@efficios.com>
To: linux-kernel@vger.kernel.org
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Michael Jeanson <mjeanson@efficios.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Alexei Starovoitov <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	bpf@vger.kernel.org, Joel Fernandes <joel@joelfernandes.org>
Subject: [RFC PATCH 6/6] tracing: use Tasks Trace RCU instead of SRCU for rcuidle tracepoints
Date: Thu, 18 Feb 2021 17:21:25 -0500	[thread overview]
Message-ID: <20210218222125.46565-7-mjeanson@efficios.com> (raw)
In-Reply-To: <20210218222125.46565-1-mjeanson@efficios.com>

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Similarly to SRCU, Tasks Trace RCU can be used for rcuidle tracepoints.
It has the advantage to provide faster RCU read-side. Similarly to
SRCU, Tasks Trace RCU grace periods are ready after core_initcall.

Now that Tasks Trace RCU is used for faultable tracepoints, using it for
rcuidle tracepoints is an overall simplification.

Some distinctions between SRCU and Tasks Trace RCU:

- Tasks Trace RCU can be used from NMI context, which was not possible
  with SRCU,
- Tree SRCU has more scalable grace periods than Tasks Trace RCU, but it
  should not matter for tracing use-cases,
- Tasks Trace RCU has slower grace periods than SRCU (similar to those
  of RCU in upcoming kernels, but similar to Tasks RCU in current
  kernels). This should also be OK for tracing,
- SRCU readers can be used in places where Tasks Trace RCU readers cannot,
  but these places are also all places where tracing is prohibited.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Michael Jeanson <mjeanson@efficios.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.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: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: bpf@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>
---
 include/linux/tracepoint.h | 34 ++++++++--------------------------
 kernel/tracepoint.c        | 25 +++++++++----------------
 2 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 04079cbd2015..c22a87c34a22 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -13,7 +13,6 @@
  */
 
 #include <linux/smp.h>
-#include <linux/srcu.h>
 #include <linux/errno.h>
 #include <linux/types.h>
 #include <linux/cpumask.h>
@@ -34,8 +33,6 @@ struct trace_eval_map {
 
 #define TRACEPOINT_DEFAULT_PRIO	10
 
-extern struct srcu_struct tracepoint_srcu;
-
 extern int
 tracepoint_probe_register(struct tracepoint *tp, void *probe, void *data);
 extern int
@@ -87,7 +84,6 @@ int unregister_tracepoint_module_notifier(struct notifier_block *nb)
 static inline void tracepoint_synchronize_unregister(void)
 {
 	synchronize_rcu_tasks_trace();
-	synchronize_srcu(&tracepoint_srcu);
 	synchronize_rcu();
 }
 #else
@@ -176,30 +172,19 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 #define __DO_TRACE(name, proto, args, cond, rcuidle, tp_flags)		\
 	do {								\
 		struct tracepoint_func *it_func_ptr;			\
-		int __maybe_unused __idx = 0;				\
 		void *__data;						\
 		bool mayfault = (tp_flags) & TRACEPOINT_MAYFAULT;	\
+		bool tasks_trace_rcu = mayfault || (rcuidle);		\
 									\
 		if (!(cond))						\
 			return;						\
 									\
-		/* srcu can't be used from NMI */			\
-		WARN_ON_ONCE(rcuidle && in_nmi());			\
-									\
-		if (mayfault) {						\
-			rcu_read_lock_trace();				\
-		} else {						\
-			/* keep srcu and sched-rcu usage consistent */	\
+		if (!mayfault)						\
 			preempt_disable_notrace();			\
-		}							\
-		/*							\
-		 * For rcuidle callers, use srcu since sched-rcu	\
-		 * doesn't work from the idle path.			\
-		 */							\
-		if (rcuidle) {						\
-			__idx = srcu_read_lock_notrace(&tracepoint_srcu);\
+		if (tasks_trace_rcu)					\
+			rcu_read_lock_trace();				\
+		if (rcuidle)						\
 			rcu_irq_enter_irqson();				\
-		}							\
 									\
 		it_func_ptr =						\
 			rcu_dereference_raw((&__tracepoint_##name)->funcs); \
@@ -209,14 +194,11 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 			__DO_TRACE_CALL(name)(args);			\
 		}							\
 									\
-		if (rcuidle) {						\
+		if (rcuidle)						\
 			rcu_irq_exit_irqson();				\
-			srcu_read_unlock_notrace(&tracepoint_srcu, __idx);\
-		}							\
-									\
-		if (mayfault)						\
+		if (tasks_trace_rcu)					\
 			rcu_read_unlock_trace();			\
-		else							\
+		if (!mayfault)						\
 			preempt_enable_notrace();			\
 	} while (0)
 
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 41fc9c6e17f6..efa49f22d435 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -18,9 +18,6 @@
 extern tracepoint_ptr_t __start___tracepoints_ptrs[];
 extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
 
-DEFINE_SRCU(tracepoint_srcu);
-EXPORT_SYMBOL_GPL(tracepoint_srcu);
-
 /* Set to 1 to enable tracepoint debug output */
 static const int tracepoint_debug;
 
@@ -65,14 +62,9 @@ static void rcu_tasks_trace_free_old_probes(struct rcu_head *head)
 	kfree(container_of(head, struct tp_probes, rcu));
 }
 
-static void srcu_free_old_probes(struct rcu_head *head)
-{
-	call_rcu_tasks_trace(head, rcu_tasks_trace_free_old_probes);
-}
-
 static void rcu_free_old_probes(struct rcu_head *head)
 {
-	call_srcu(&tracepoint_srcu, head, srcu_free_old_probes);
+	call_rcu_tasks_trace(head, rcu_tasks_trace_free_old_probes);
 }
 
 static __init int release_early_probes(void)
@@ -90,7 +82,7 @@ static __init int release_early_probes(void)
 	return 0;
 }
 
-/* SRCU and Tasks Trace RCU are initialized at core_initcall */
+/* Tasks Trace RCU is initialized at core_initcall */
 postcore_initcall(release_early_probes);
 
 static inline void release_probes(struct tracepoint_func *old)
@@ -100,9 +92,8 @@ static inline void release_probes(struct tracepoint_func *old)
 			struct tp_probes, probes[0]);
 
 		/*
-		 * We can't free probes if SRCU and Tasks Trace RCU are not
-		 * initialized yet. Postpone the freeing till after both are
-		 * initialized.
+		 * We can't free probes if Tasks Trace RCU is not initialized yet.
+		 * Postpone the freeing till after Tasks Trace RCU is initialized.
 		 */
 		if (unlikely(!ok_to_free_tracepoints)) {
 			tp_probes->rcu.next = early_probes;
@@ -111,9 +102,11 @@ static inline void release_probes(struct tracepoint_func *old)
 		}
 
 		/*
-		 * Tracepoint probes are protected by sched RCU, SRCU and
-		 * Tasks Trace RCU by chaining the callbacks we cover all three
-		 * cases and wait for all three grace periods.
+		 * Tracepoint probes are protected by both sched RCU and
+		 * Tasks Trace RCU, by calling the Tasks Trace RCU callback in
+		 * the sched RCU callback we cover both cases. So let us chain
+		 * the Tasks Trace RCU and sched RCU callbacks to wait for both
+		 * grace periods.
 		 */
 		call_rcu(&tp_probes->rcu, rcu_free_old_probes);
 	}
-- 
2.25.1


  parent reply	other threads:[~2021-02-18 22:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18 22:21 [RFC PATCH 0/6] [RFC] Faultable tracepoints (v2) Michael Jeanson
2021-02-18 22:21 ` [RFC PATCH 1/6] tracing: introduce faultable " Michael Jeanson
2021-02-18 22:21 ` [RFC PATCH 2/6] tracing: ftrace: add support for faultable tracepoints Michael Jeanson
2021-02-18 22:21 ` [RFC PATCH 3/6] tracing: bpf-trace: " Michael Jeanson
2021-02-18 22:21 ` [RFC PATCH 4/6] tracing: perf: " Michael Jeanson
2021-02-18 22:21 ` [RFC PATCH 5/6] tracing: convert sys_enter/exit to " Michael Jeanson
2021-02-18 22:21 ` Michael Jeanson [this message]
2021-02-24  2:16 ` [RFC PATCH 0/6] [RFC] Faultable tracepoints (v2) Steven Rostedt
2021-02-24 16:22   ` Michael Jeanson
2021-02-24 16:59     ` Mathieu Desnoyers
2021-02-24 18:14       ` Steven Rostedt
2021-02-25 21:46         ` Mathieu Desnoyers
2021-02-24 23:54       ` Mathieu Desnoyers
2021-02-26  5:28       ` Lai Jiangshan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210218222125.46565-7-mjeanson@efficios.com \
    --to=mjeanson@efficios.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=joel@joelfernandes.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).