diff mbox series

[V4,05/19] trace/osnoise: Allow multiple instances of the same tracer

Message ID 6da5c06e2455e4edca4f4115199bf2bf3844798c.1634820694.git.bristot@kernel.org (mailing list archive)
State Superseded
Headers show
Series RTLA: An interface for osnoise/timerlat tracers | expand

Commit Message

Daniel Bristot de Oliveira Oct. 21, 2021, 12:56 p.m. UTC
Allow more than one instance of the same tracer. The workload will
start when the first trace_array (instance) is registered and will
stop when the last instance is unregistered.

Osnoise and timerlat are still mutually exclusive because of the
different behavior of the osnoise: tracepoints.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Tom Zanussi <zanussi@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Clark Williams <williams@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: linux-rt-users@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 kernel/trace/trace_osnoise.c | 106 ++++++++++++++++++++++++++---------
 1 file changed, 80 insertions(+), 26 deletions(-)

Comments

Steven Rostedt Oct. 23, 2021, 2:47 a.m. UTC | #1
On Thu, 21 Oct 2021 14:56:43 +0200
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> Allow more than one instance of the same tracer. The workload will
> start when the first trace_array (instance) is registered and will
> stop when the last instance is unregistered.
> 


> Osnoise and timerlat are still mutually exclusive because of the
> different behavior of the osnoise: tracepoints.

I don't understand what the above means, or how it is relative to this
commit?

-- Steve
Daniel Bristot de Oliveira Oct. 25, 2021, 8:08 a.m. UTC | #2
On 10/23/21 04:47, Steven Rostedt wrote:
> On Thu, 21 Oct 2021 14:56:43 +0200
> Daniel Bristot de Oliveira <bristot@kernel.org> wrote:
> 
>> Allow more than one instance of the same tracer. The workload will
>> start when the first trace_array (instance) is registered and will
>> stop when the last instance is unregistered.
>>
> 
> 
>> Osnoise and timerlat are still mutually exclusive because of the
>> different behavior of the osnoise: tracepoints.
> 
> I don't understand what the above means, or how it is relative to this
> commit?


osnoise and timerlat tracers share a lot of code, including the osnoise: events.

However, the osnoise: events have different behavior depending on which tracer
is enabled. So, while this patch allows the same tracer to run in multiple
instances (e.g., two instances running osnoise), it still does not allow
instances of timerlat and osnoise in parallel (e.g., one timerlat and osnoise).

Should I add the text above to the log?

It is possible to extend the current behavior to allow concurrent osnoise and
timerlat tracers to run on different CPUs. For instance, osnoise on cpu 0,1 and
timerlat on cpu 2,3. This is on my to-do list, but it is not necessary for now.

-- Daniel


> -- Steve
>
Steven Rostedt Oct. 25, 2021, 2:36 p.m. UTC | #3
On Mon, 25 Oct 2021 10:08:39 +0200
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> osnoise and timerlat tracers share a lot of code, including the osnoise: events.
> 
> However, the osnoise: events have different behavior depending on which tracer
> is enabled. So, while this patch allows the same tracer to run in multiple
> instances (e.g., two instances running osnoise), it still does not allow
> instances of timerlat and osnoise in parallel (e.g., one timerlat and osnoise).
> 
> Should I add the text above to the log?

Yes please.

-- Steve
diff mbox series

Patch

diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index a6d8f514bd7c..f913964f6861 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -64,6 +64,24 @@  static bool osnoise_has_registered_instances(void)
 					list);
 }
 
+/*
+ * osnoise_instance_registered - check if a tr is already registered
+ */
+static int osnoise_instance_registered(struct trace_array *tr)
+{
+	struct osnoise_instance *inst;
+	int found = 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(inst, &osnoise_instances, list) {
+		if (inst->tr == tr)
+			found = 1;
+	}
+	rcu_read_unlock();
+
+	return found;
+}
+
 /*
  * osnoise_register_instance - register a new trace instance
  *
@@ -106,10 +124,8 @@  static void osnoise_unregister_instance(struct trace_array *tr)
 	}
 	rcu_read_unlock();
 
-	if (!found) {
-		WARN("osnoise unregister did not find tr %s\n", tr->name);
+	if (!found)
 		return;
-	}
 
 	synchronize_rcu();
 	kfree(inst);
@@ -1190,6 +1206,7 @@  static __always_inline void osnoise_stop_tracing(void)
 				"stop tracing hit on cpu %d\n", smp_processor_id());
 
 		tracer_tracing_off(tr);
+
 	}
 	rcu_read_unlock();
 }
@@ -2041,6 +2058,16 @@  static int osnoise_workload_start(void)
 {
 	int retval;
 
+	/*
+	 * Instances need to be registered after calling workload
+	 * start. Hence, if there is already an instance, the
+	 * workload was already registered. Otherwise, this
+	 * code is on the way to register the first instance,
+	 * and the workload will start.
+	 */
+	if (osnoise_has_registered_instances())
+		return 0;
+
 	osn_var_reset_all();
 
 	retval = osnoise_hook_events();
@@ -2066,6 +2093,13 @@  static int osnoise_workload_start(void)
  */
 static void osnoise_workload_stop(void)
 {
+	/*
+	 * Instances need to be unregistered before calling
+	 * stop. Hence, if there is a registered instance, more
+	 * than one instance is running, and the workload will not
+	 * yet stop. Otherwise, this code is on the way to disable
+	 * the last instance, and the workload can stop.
+	 */
 	if (osnoise_has_registered_instances())
 		return;
 
@@ -2083,7 +2117,11 @@  static void osnoise_tracer_start(struct trace_array *tr)
 {
 	int retval;
 
-	if (osnoise_has_registered_instances())
+	/*
+	 * If the instance is already registered, there is no need to
+	 * register it again.
+	 */
+	if (osnoise_instance_registered(tr))
 		return;
 
 	retval = osnoise_workload_start();
@@ -2095,18 +2133,17 @@  static void osnoise_tracer_start(struct trace_array *tr)
 
 static void osnoise_tracer_stop(struct trace_array *tr)
 {
-	if (!osnoise_has_registered_instances())
-		return;
-
 	osnoise_unregister_instance(tr);
 	osnoise_workload_stop();
 }
 
 static int osnoise_tracer_init(struct trace_array *tr)
 {
-
-	/* Only allow one instance to enable this */
-	if (osnoise_has_registered_instances())
+	/*
+	 * Only allow osnoise tracer if timerlat tracer is not running
+	 * already.
+	 */
+	if (osnoise_data.timerlat_tracer)
 		return -EBUSY;
 
 	tr->max_latency = 0;
@@ -2135,45 +2172,55 @@  static void timerlat_tracer_start(struct trace_array *tr)
 {
 	int retval;
 
-	if (osnoise_has_registered_instances())
+	/*
+	 * If the instance is already registered, there is no need to
+	 * register it again.
+	 */
+	if (osnoise_instance_registered(tr))
 		return;
 
-	osnoise_data.timerlat_tracer = 1;
-
 	retval = osnoise_workload_start();
 	if (retval)
-		goto out_err;
+		pr_err(BANNER "Error starting timerlat tracer\n");
 
 	osnoise_register_instance(tr);
 
 	return;
-out_err:
-	pr_err(BANNER "Error starting timerlat tracer\n");
 }
 
 static void timerlat_tracer_stop(struct trace_array *tr)
 {
 	int cpu;
 
-	if (!osnoise_has_registered_instances())
-		return;
-
-	for_each_online_cpu(cpu)
-		per_cpu(per_cpu_osnoise_var, cpu).sampling = 0;
+	osnoise_unregister_instance(tr);
 
-	osnoise_tracer_stop(tr);
+	/*
+	 * Instruct the threads to stop only if this is the last instance.
+	 */
+	if (!osnoise_has_registered_instances()) {
+		for_each_online_cpu(cpu)
+			per_cpu(per_cpu_osnoise_var, cpu).sampling = 0;
+	}
 
-	osnoise_data.timerlat_tracer = 0;
+	osnoise_workload_stop();
 }
 
 static int timerlat_tracer_init(struct trace_array *tr)
 {
-	/* Only allow one instance to enable this */
-	if (osnoise_has_registered_instances())
+	/*
+	 * Only allow timerlat tracer if osnoise tracer is not running already.
+	 */
+	if (osnoise_has_registered_instances() && !osnoise_data.timerlat_tracer)
 		return -EBUSY;
 
-	tr->max_latency = 0;
+	/*
+	 * If this is the first instance, set timerlat_tracer to block
+	 * osnoise tracer start.
+	 */
+	if (!osnoise_has_registered_instances())
+		osnoise_data.timerlat_tracer = 1;
 
+	tr->max_latency = 0;
 	timerlat_tracer_start(tr);
 
 	return 0;
@@ -2182,6 +2229,13 @@  static int timerlat_tracer_init(struct trace_array *tr)
 static void timerlat_tracer_reset(struct trace_array *tr)
 {
 	timerlat_tracer_stop(tr);
+
+	/*
+	 * If this is the last instance, reset timerlat_tracer allowing
+	 * osnoise to be started.
+	 */
+	if (!osnoise_has_registered_instances())
+		osnoise_data.timerlat_tracer = 0;
 }
 
 static struct tracer timerlat_tracer __read_mostly = {