linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] rtla: Always set all tracer options
@ 2025-03-20  9:24 Tomas Glozar
  2025-03-20  9:24 ` [PATCH 1/6] rtla/osnoise: Unify params struct Tomas Glozar
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

rtla should not rely on osnoise tracer being set to default options,
since this might not be the case - either due to the user using the
tracer without rtla, or due to rtla exiting abnormally and not resetting
the options to their previous value.

A part of the problem is already fixed by commits 217f0b1e990e
("rtla/timerlat_top: Set OSNOISE_WORKLOAD for kernel threads") and
d8d866171a41 ("rtla/timerlat_hist: Set OSNOISE_WORKLOAD for kernel
threads"), but for other options than for OSNOISE_WORKLOAD, and for all
options in the case of rtla-osnoise, it is not fixed.

This patchset sets all options to their default values, unless they are
set by command line options to different values.

Before doing that, code setting the tracer options is unified between
top and hist for both osnoise and timerlat.

Notes:
- The unification depends on a commit in linux-next that already does
that for rtla-timerlat, for the purpose of the implementation of the BPF
sample collection [1]. For rtla-osnoise, this is done in an analogous
commit in this patchset.
- Because of the former and because of the unification requiring changes
for older versions of the kernel, I do not Cc stable here. If needed,
I can sent the backport to stable manually after both this and the BPF
sample collection patchsets are in Linus's tree.

[1] https://lore.kernel.org/linux-trace-kernel/20250218145859.27762-2-tglozar@redhat.com/

Tomas Glozar (6):
  rtla/osnoise: Unify params struct
  rtla: Unify apply_config between top and hist
  rtla/osnoise: Set OSNOISE_WORKLOAD to true
  rtla: Always set all tracer options
  rtla/tests: Reset osnoise options before check
  rtla/tests: Test setting default options

 tools/tracing/rtla/src/osnoise.c       |  86 +++++++++++++++++-
 tools/tracing/rtla/src/osnoise.h       |  48 ++++++++++
 tools/tracing/rtla/src/osnoise_hist.c  | 118 +++---------------------
 tools/tracing/rtla/src/osnoise_top.c   | 120 +++----------------------
 tools/tracing/rtla/src/timerlat.c      | 106 ++++++++++++++++++++++
 tools/tracing/rtla/src/timerlat.h      |  12 +--
 tools/tracing/rtla/src/timerlat_hist.c | 119 ++++--------------------
 tools/tracing/rtla/src/timerlat_top.c  | 110 +++--------------------
 tools/tracing/rtla/tests/engine.sh     |  66 ++++++++++++++
 tools/tracing/rtla/tests/osnoise.t     |   6 ++
 10 files changed, 370 insertions(+), 421 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/6] rtla/osnoise: Unify params struct
  2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
@ 2025-03-20  9:24 ` Tomas Glozar
  2025-03-20 19:00   ` John Kacur
  2025-03-20  9:24 ` [PATCH 2/6] rtla: Unify apply_config between top and hist Tomas Glozar
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

Instead of having separate structs osnoise_top_params and
osnoise_hist_params, use one struct osnoise_params for both.

This allows code using the structs to be shared between osnoise-top and
osnoise-hist.

Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/src/osnoise.c      |  1 -
 tools/tracing/rtla/src/osnoise.h      | 47 +++++++++++++++++++++++
 tools/tracing/rtla/src/osnoise_hist.c | 52 ++++++--------------------
 tools/tracing/rtla/src/osnoise_top.c  | 54 +++++----------------------
 tools/tracing/rtla/src/timerlat.h     |  1 -
 5 files changed, 68 insertions(+), 87 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 85f398b89597..93d485c0e949 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -14,7 +14,6 @@
 #include <stdio.h>
 
 #include "osnoise.h"
-#include "utils.h"
 
 /*
  * osnoise_get_cpus - return the original "osnoise/cpus" content
diff --git a/tools/tracing/rtla/src/osnoise.h b/tools/tracing/rtla/src/osnoise.h
index 056c8b113dee..f78ffbdc8c8d 100644
--- a/tools/tracing/rtla/src/osnoise.h
+++ b/tools/tracing/rtla/src/osnoise.h
@@ -1,8 +1,55 @@
 // SPDX-License-Identifier: GPL-2.0
 #pragma once
 
+#include "utils.h"
 #include "trace.h"
 
+enum osnoise_mode {
+	MODE_OSNOISE = 0,
+	MODE_HWNOISE
+};
+
+struct osnoise_params {
+	/* Common params */
+	char			*cpus;
+	cpu_set_t		monitored_cpus;
+	char			*trace_output;
+	char			*cgroup_name;
+	unsigned long long	runtime;
+	unsigned long long	period;
+	long long		threshold;
+	long long		stop_us;
+	long long		stop_total_us;
+	int			sleep_time;
+	int			duration;
+	int			set_sched;
+	int			cgroup;
+	int			hk_cpus;
+	cpu_set_t		hk_cpu_set;
+	struct sched_attr	sched_param;
+	struct trace_events	*events;
+	int			warmup;
+	int			buffer_size;
+	union {
+		struct {
+			/* top only */
+			int			quiet;
+			int			pretty_output;
+			enum osnoise_mode	mode;
+		};
+		struct {
+			/* hist only */
+			int			output_divisor;
+			char			no_header;
+			char			no_summary;
+			char			no_index;
+			char			with_zeros;
+			int			bucket_size;
+			int			entries;
+		};
+	};
+};
+
 /*
  * osnoise_context - read, store, write, restore osnoise configs.
  */
diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index f4c9051c33c4..4721f15f77cd 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -14,38 +14,8 @@
 #include <time.h>
 #include <sched.h>
 
-#include "utils.h"
 #include "osnoise.h"
 
-struct osnoise_hist_params {
-	char			*cpus;
-	cpu_set_t		monitored_cpus;
-	char			*trace_output;
-	char			*cgroup_name;
-	unsigned long long	runtime;
-	unsigned long long	period;
-	long long		threshold;
-	long long		stop_us;
-	long long		stop_total_us;
-	int			sleep_time;
-	int			duration;
-	int			set_sched;
-	int			output_divisor;
-	int			cgroup;
-	int			hk_cpus;
-	cpu_set_t		hk_cpu_set;
-	struct sched_attr	sched_param;
-	struct trace_events	*events;
-	char			no_header;
-	char			no_summary;
-	char			no_index;
-	char			with_zeros;
-	int			bucket_size;
-	int			entries;
-	int			warmup;
-	int			buffer_size;
-};
-
 struct osnoise_hist_cpu {
 	int			*samples;
 	int			count;
@@ -126,7 +96,7 @@ static struct osnoise_hist_data
 static void osnoise_hist_update_multiple(struct osnoise_tool *tool, int cpu,
 					 unsigned long long duration, int count)
 {
-	struct osnoise_hist_params *params = tool->params;
+	struct osnoise_params *params = tool->params;
 	struct osnoise_hist_data *data = tool->data;
 	unsigned long long total_duration;
 	int entries = data->entries;
@@ -168,7 +138,7 @@ static void osnoise_destroy_trace_hist(struct osnoise_tool *tool)
  */
 static int osnoise_init_trace_hist(struct osnoise_tool *tool)
 {
-	struct osnoise_hist_params *params = tool->params;
+	struct osnoise_params *params = tool->params;
 	struct osnoise_hist_data *data = tool->data;
 	int bucket_size;
 	char buff[128];
@@ -253,7 +223,7 @@ static void osnoise_read_trace_hist(struct osnoise_tool *tool)
  */
 static void osnoise_hist_header(struct osnoise_tool *tool)
 {
-	struct osnoise_hist_params *params = tool->params;
+	struct osnoise_params *params = tool->params;
 	struct osnoise_hist_data *data = tool->data;
 	struct trace_seq *s = tool->trace.seq;
 	char duration[26];
@@ -292,7 +262,7 @@ static void osnoise_hist_header(struct osnoise_tool *tool)
  * osnoise_print_summary - print the summary of the hist data to the output
  */
 static void
-osnoise_print_summary(struct osnoise_hist_params *params,
+osnoise_print_summary(struct osnoise_params *params,
 		       struct trace_instance *trace,
 		       struct osnoise_hist_data *data)
 {
@@ -370,7 +340,7 @@ osnoise_print_summary(struct osnoise_hist_params *params,
  * osnoise_print_stats - print data for all CPUs
  */
 static void
-osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *tool)
+osnoise_print_stats(struct osnoise_params *params, struct osnoise_tool *tool)
 {
 	struct osnoise_hist_data *data = tool->data;
 	struct trace_instance *trace = &tool->trace;
@@ -508,10 +478,10 @@ static void osnoise_hist_usage(char *usage)
 /*
  * osnoise_hist_parse_args - allocs, parse and fill the cmd line parameters
  */
-static struct osnoise_hist_params
+static struct osnoise_params
 *osnoise_hist_parse_args(int argc, char *argv[])
 {
-	struct osnoise_hist_params *params;
+	struct osnoise_params *params;
 	struct trace_events *tevent;
 	int retval;
 	int c;
@@ -731,7 +701,7 @@ static struct osnoise_hist_params
  * osnoise_hist_apply_config - apply the hist configs to the initialized tool
  */
 static int
-osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_hist_params *params)
+osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
 {
 	int retval;
 
@@ -808,7 +778,7 @@ osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_hist_params
  * osnoise_init_hist - initialize a osnoise hist tool with parameters
  */
 static struct osnoise_tool
-*osnoise_init_hist(struct osnoise_hist_params *params)
+*osnoise_init_hist(struct osnoise_params *params)
 {
 	struct osnoise_tool *tool;
 	int nr_cpus;
@@ -842,7 +812,7 @@ static void stop_hist(int sig)
  * osnoise_hist_set_signals - handles the signal to stop the tool
  */
 static void
-osnoise_hist_set_signals(struct osnoise_hist_params *params)
+osnoise_hist_set_signals(struct osnoise_params *params)
 {
 	signal(SIGINT, stop_hist);
 	if (params->duration) {
@@ -853,7 +823,7 @@ osnoise_hist_set_signals(struct osnoise_hist_params *params)
 
 int osnoise_hist_main(int argc, char *argv[])
 {
-	struct osnoise_hist_params *params;
+	struct osnoise_params *params;
 	struct osnoise_tool *record = NULL;
 	struct osnoise_tool *tool = NULL;
 	struct trace_instance *trace;
diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
index dacec2f99017..7f393019bbf5 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -14,40 +14,6 @@
 #include <sched.h>
 
 #include "osnoise.h"
-#include "utils.h"
-
-enum osnoise_mode {
-	MODE_OSNOISE = 0,
-	MODE_HWNOISE
-};
-
-/*
- * osnoise top parameters
- */
-struct osnoise_top_params {
-	char			*cpus;
-	cpu_set_t		monitored_cpus;
-	char			*trace_output;
-	char			*cgroup_name;
-	unsigned long long	runtime;
-	unsigned long long	period;
-	long long		threshold;
-	long long		stop_us;
-	long long		stop_total_us;
-	int			sleep_time;
-	int			duration;
-	int			quiet;
-	int			set_sched;
-	int			cgroup;
-	int			hk_cpus;
-	int			warmup;
-	int			buffer_size;
-	int			pretty_output;
-	cpu_set_t		hk_cpu_set;
-	struct sched_attr	sched_param;
-	struct trace_events	*events;
-	enum osnoise_mode	mode;
-};
 
 struct osnoise_top_cpu {
 	unsigned long long	sum_runtime;
@@ -158,7 +124,7 @@ osnoise_top_handler(struct trace_seq *s, struct tep_record *record,
  */
 static void osnoise_top_header(struct osnoise_tool *top)
 {
-	struct osnoise_top_params *params = top->params;
+	struct osnoise_params *params = top->params;
 	struct trace_seq *s = top->trace.seq;
 	char duration[26];
 
@@ -218,7 +184,7 @@ static void clear_terminal(struct trace_seq *seq)
  */
 static void osnoise_top_print(struct osnoise_tool *tool, int cpu)
 {
-	struct osnoise_top_params *params = tool->params;
+	struct osnoise_params *params = tool->params;
 	struct trace_seq *s = tool->trace.seq;
 	struct osnoise_top_cpu *cpu_data;
 	struct osnoise_top_data *data;
@@ -258,7 +224,7 @@ static void osnoise_top_print(struct osnoise_tool *tool, int cpu)
  * osnoise_print_stats - print data for all cpus
  */
 static void
-osnoise_print_stats(struct osnoise_top_params *params, struct osnoise_tool *top)
+osnoise_print_stats(struct osnoise_params *params, struct osnoise_tool *top)
 {
 	struct trace_instance *trace = &top->trace;
 	static int nr_cpus = -1;
@@ -286,7 +252,7 @@ osnoise_print_stats(struct osnoise_top_params *params, struct osnoise_tool *top)
 /*
  * osnoise_top_usage - prints osnoise top usage message
  */
-static void osnoise_top_usage(struct osnoise_top_params *params, char *usage)
+static void osnoise_top_usage(struct osnoise_params *params, char *usage)
 {
 	int i;
 
@@ -354,9 +320,9 @@ static void osnoise_top_usage(struct osnoise_top_params *params, char *usage)
 /*
  * osnoise_top_parse_args - allocs, parse and fill the cmd line parameters
  */
-struct osnoise_top_params *osnoise_top_parse_args(int argc, char **argv)
+struct osnoise_params *osnoise_top_parse_args(int argc, char **argv)
 {
-	struct osnoise_top_params *params;
+	struct osnoise_params *params;
 	struct trace_events *tevent;
 	int retval;
 	int c;
@@ -553,7 +519,7 @@ struct osnoise_top_params *osnoise_top_parse_args(int argc, char **argv)
  * osnoise_top_apply_config - apply the top configs to the initialized tool
  */
 static int
-osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_top_params *params)
+osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
 {
 	int retval;
 
@@ -640,7 +606,7 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_top_params *p
 /*
  * osnoise_init_top - initialize a osnoise top tool with parameters
  */
-struct osnoise_tool *osnoise_init_top(struct osnoise_top_params *params)
+struct osnoise_tool *osnoise_init_top(struct osnoise_params *params)
 {
 	struct osnoise_tool *tool;
 	int nr_cpus;
@@ -674,7 +640,7 @@ static void stop_top(int sig)
 /*
  * osnoise_top_set_signals - handles the signal to stop the tool
  */
-static void osnoise_top_set_signals(struct osnoise_top_params *params)
+static void osnoise_top_set_signals(struct osnoise_params *params)
 {
 	signal(SIGINT, stop_top);
 	if (params->duration) {
@@ -685,7 +651,7 @@ static void osnoise_top_set_signals(struct osnoise_top_params *params)
 
 int osnoise_top_main(int argc, char **argv)
 {
-	struct osnoise_top_params *params;
+	struct osnoise_params *params;
 	struct osnoise_tool *record = NULL;
 	struct osnoise_tool *tool = NULL;
 	struct trace_instance *trace;
diff --git a/tools/tracing/rtla/src/timerlat.h b/tools/tracing/rtla/src/timerlat.h
index e452d385cb0f..cadc613dc82e 100644
--- a/tools/tracing/rtla/src/timerlat.h
+++ b/tools/tracing/rtla/src/timerlat.h
@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0
-#include "utils.h"
 #include "osnoise.h"
 
 struct timerlat_params {
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/6] rtla: Unify apply_config between top and hist
  2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
  2025-03-20  9:24 ` [PATCH 1/6] rtla/osnoise: Unify params struct Tomas Glozar
@ 2025-03-20  9:24 ` Tomas Glozar
  2025-03-20 19:27   ` John Kacur
  2025-03-20  9:24 ` [PATCH 3/6] rtla/osnoise: Set OSNOISE_WORKLOAD to true Tomas Glozar
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

The functions osnoise_top_apply_config and osnoise_hist_apply_config, as
well as timerlat_top_apply_config and timerlat_hist_apply_config, are
mostly the same.

Move common part from them into separate functions osnoise_apply_config
and timerlat_apply_config.

For rtla-timerlat, also unify params->user_hist and params->user_top
into one field called params->user_data, and move several fields used
only by timerlat-top into the top-only section of struct
timerlat_params.

Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/src/osnoise.c       |  79 ++++++++++++++++
 tools/tracing/rtla/src/osnoise.h       |   1 +
 tools/tracing/rtla/src/osnoise_hist.c  |  66 +-------------
 tools/tracing/rtla/src/osnoise_top.c   |  66 +-------------
 tools/tracing/rtla/src/timerlat.c      | 109 ++++++++++++++++++++++
 tools/tracing/rtla/src/timerlat.h      |  11 +--
 tools/tracing/rtla/src/timerlat_hist.c | 119 ++++---------------------
 tools/tracing/rtla/src/timerlat_top.c  | 110 +++--------------------
 8 files changed, 227 insertions(+), 334 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 93d485c0e949..1735a36466c4 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2021 Red Hat Inc, Daniel Bristot de Oliveira <bristot@kernel.org>
  */
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <pthread.h>
@@ -12,6 +13,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <sched.h>
 
 #include "osnoise.h"
 
@@ -1114,6 +1116,83 @@ osnoise_report_missed_events(struct osnoise_tool *tool)
 	}
 }
 
+/*
+ * osnoise_apply_config - apply common configs to the initialized tool
+ */
+int
+osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
+{
+	int retval;
+
+	if (!params->sleep_time)
+		params->sleep_time = 1;
+
+	if (params->cpus) {
+		retval = osnoise_set_cpus(tool->context, params->cpus);
+		if (retval) {
+			err_msg("Failed to apply CPUs config\n");
+			goto out_err;
+		}
+	}
+
+	if (params->runtime || params->period) {
+		retval = osnoise_set_runtime_period(tool->context,
+						    params->runtime,
+						    params->period);
+		if (retval) {
+			err_msg("Failed to set runtime and/or period\n");
+			goto out_err;
+		}
+	}
+
+	if (params->stop_us) {
+		retval = osnoise_set_stop_us(tool->context, params->stop_us);
+		if (retval) {
+			err_msg("Failed to set stop us\n");
+			goto out_err;
+		}
+	}
+
+	if (params->stop_total_us) {
+		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
+		if (retval) {
+			err_msg("Failed to set stop total us\n");
+			goto out_err;
+		}
+	}
+
+	if (params->threshold) {
+		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
+		if (retval) {
+			err_msg("Failed to set tracing_thresh\n");
+			goto out_err;
+		}
+	}
+
+	if (params->hk_cpus) {
+		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
+					   &params->hk_cpu_set);
+		if (retval == -1) {
+			err_msg("Failed to set rtla to the house keeping CPUs\n");
+			goto out_err;
+		}
+	} else if (params->cpus) {
+		/*
+		 * Even if the user do not set a house-keeping CPU, try to
+		 * move rtla to a CPU set different to the one where the user
+		 * set the workload to run.
+		 *
+		 * No need to check results as this is an automatic attempt.
+		 */
+		auto_house_keeping(&params->monitored_cpus);
+	}
+
+	return 0;
+
+out_err:
+	return -1;
+}
+
 static void osnoise_usage(int err)
 {
 	int i;
diff --git a/tools/tracing/rtla/src/osnoise.h b/tools/tracing/rtla/src/osnoise.h
index f78ffbdc8c8d..ac1c99910744 100644
--- a/tools/tracing/rtla/src/osnoise.h
+++ b/tools/tracing/rtla/src/osnoise.h
@@ -155,6 +155,7 @@ struct osnoise_tool *osnoise_init_tool(char *tool_name);
 struct osnoise_tool *osnoise_init_trace_tool(char *tracer);
 void osnoise_report_missed_events(struct osnoise_tool *tool);
 bool osnoise_trace_is_off(struct osnoise_tool *tool, struct osnoise_tool *record);
+int osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params);
 
 int osnoise_hist_main(int argc, char *argv[]);
 int osnoise_top_main(int argc, char **argv);
diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index 4721f15f77cd..d9d15c8f27c7 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -12,7 +12,6 @@
 #include <errno.h>
 #include <stdio.h>
 #include <time.h>
-#include <sched.h>
 
 #include "osnoise.h"
 
@@ -705,68 +704,9 @@ osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_params *para
 {
 	int retval;
 
-	if (!params->sleep_time)
-		params->sleep_time = 1;
-
-	if (params->cpus) {
-		retval = osnoise_set_cpus(tool->context, params->cpus);
-		if (retval) {
-			err_msg("Failed to apply CPUs config\n");
-			goto out_err;
-		}
-	}
-
-	if (params->runtime || params->period) {
-		retval = osnoise_set_runtime_period(tool->context,
-						    params->runtime,
-						    params->period);
-		if (retval) {
-			err_msg("Failed to set runtime and/or period\n");
-			goto out_err;
-		}
-	}
-
-	if (params->stop_us) {
-		retval = osnoise_set_stop_us(tool->context, params->stop_us);
-		if (retval) {
-			err_msg("Failed to set stop us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->stop_total_us) {
-		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
-		if (retval) {
-			err_msg("Failed to set stop total us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->threshold) {
-		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
-		if (retval) {
-			err_msg("Failed to set tracing_thresh\n");
-			goto out_err;
-		}
-	}
-
-	if (params->hk_cpus) {
-		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
-					   &params->hk_cpu_set);
-		if (retval == -1) {
-			err_msg("Failed to set rtla to the house keeping CPUs\n");
-			goto out_err;
-		}
-	} else if (params->cpus) {
-		/*
-		 * Even if the user do not set a house-keeping CPU, try to
-		 * move rtla to a CPU set different to the one where the user
-		 * set the workload to run.
-		 *
-		 * No need to check results as this is an automatic attempt.
-		 */
-		auto_house_keeping(&params->monitored_cpus);
-	}
+	retval = osnoise_apply_config(tool, params);
+	if (retval)
+		goto out_err;
 
 	return 0;
 
diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
index 7f393019bbf5..3455ee73e2e6 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -11,7 +11,6 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <time.h>
-#include <sched.h>
 
 #include "osnoise.h"
 
@@ -523,50 +522,9 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_params *param
 {
 	int retval;
 
-	if (!params->sleep_time)
-		params->sleep_time = 1;
-
-	if (params->cpus) {
-		retval = osnoise_set_cpus(tool->context, params->cpus);
-		if (retval) {
-			err_msg("Failed to apply CPUs config\n");
-			goto out_err;
-		}
-	}
-
-	if (params->runtime || params->period) {
-		retval = osnoise_set_runtime_period(tool->context,
-						    params->runtime,
-						    params->period);
-		if (retval) {
-			err_msg("Failed to set runtime and/or period\n");
-			goto out_err;
-		}
-	}
-
-	if (params->stop_us) {
-		retval = osnoise_set_stop_us(tool->context, params->stop_us);
-		if (retval) {
-			err_msg("Failed to set stop us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->stop_total_us) {
-		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
-		if (retval) {
-			err_msg("Failed to set stop total us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->threshold) {
-		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
-		if (retval) {
-			err_msg("Failed to set tracing_thresh\n");
-			goto out_err;
-		}
-	}
+	retval = osnoise_apply_config(tool, params);
+	if (retval)
+		goto out_err;
 
 	if (params->mode == MODE_HWNOISE) {
 		retval = osnoise_set_irq_disable(tool->context, 1);
@@ -576,24 +534,6 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_params *param
 		}
 	}
 
-	if (params->hk_cpus) {
-		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
-					   &params->hk_cpu_set);
-		if (retval == -1) {
-			err_msg("Failed to set rtla to the house keeping CPUs\n");
-			goto out_err;
-		}
-	} else if (params->cpus) {
-		/*
-		 * Even if the user do not set a house-keeping CPU, try to
-		 * move rtla to a CPU set different to the one where the user
-		 * set the workload to run.
-		 *
-		 * No need to check results as this is an automatic attempt.
-		 */
-		auto_house_keeping(&params->monitored_cpus);
-	}
-
 	if (isatty(STDOUT_FILENO) && !params->quiet)
 		params->pretty_output = 1;
 
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 21cdcc5c4a29..448fb1f7d3a6 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) 2021 Red Hat Inc, Daniel Bristot de Oliveira <bristot@kernel.org>
  */
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <pthread.h>
@@ -11,9 +12,117 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <sched.h>
 
 #include "timerlat.h"
 
+/*
+ * timerlat_apply_config - apply common configs to the initialized tool
+ */
+int
+timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
+{
+	int retval, i;
+
+	if (!params->sleep_time)
+		params->sleep_time = 1;
+
+	if (params->cpus) {
+		retval = osnoise_set_cpus(tool->context, params->cpus);
+		if (retval) {
+			err_msg("Failed to apply CPUs config\n");
+			goto out_err;
+		}
+	} else {
+		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
+			CPU_SET(i, &params->monitored_cpus);
+	}
+
+	if (params->stop_us) {
+		retval = osnoise_set_stop_us(tool->context, params->stop_us);
+		if (retval) {
+			err_msg("Failed to set stop us\n");
+			goto out_err;
+		}
+	}
+
+	if (params->stop_total_us) {
+		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
+		if (retval) {
+			err_msg("Failed to set stop total us\n");
+			goto out_err;
+		}
+	}
+
+
+	if (params->timerlat_period_us) {
+		retval = osnoise_set_timerlat_period_us(tool->context, params->timerlat_period_us);
+		if (retval) {
+			err_msg("Failed to set timerlat period\n");
+			goto out_err;
+		}
+	}
+
+
+	if (params->print_stack) {
+		retval = osnoise_set_print_stack(tool->context, params->print_stack);
+		if (retval) {
+			err_msg("Failed to set print stack\n");
+			goto out_err;
+		}
+	}
+
+	if (params->hk_cpus) {
+		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
+					   &params->hk_cpu_set);
+		if (retval == -1) {
+			err_msg("Failed to set rtla to the house keeping CPUs\n");
+			goto out_err;
+		}
+	} else if (params->cpus) {
+		/*
+		 * Even if the user do not set a house-keeping CPU, try to
+		 * move rtla to a CPU set different to the one where the user
+		 * set the workload to run.
+		 *
+		 * No need to check results as this is an automatic attempt.
+		 */
+		auto_house_keeping(&params->monitored_cpus);
+	}
+
+	/*
+	 * If the user did not specify a type of thread, try user-threads first.
+	 * Fall back to kernel threads otherwise.
+	 */
+	if (!params->kernel_workload && !params->user_data) {
+		retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd");
+		if (retval) {
+			debug_msg("User-space interface detected, setting user-threads\n");
+			params->user_workload = 1;
+			params->user_data = 1;
+		} else {
+			debug_msg("User-space interface not detected, setting kernel-threads\n");
+			params->kernel_workload = 1;
+		}
+	}
+
+	/*
+	 * Set workload according to type of thread if the kernel supports it.
+	 * On kernels without support, user threads will have already failed
+	 * on missing timerlat_fd, and kernel threads do not need it.
+	 */
+	retval = osnoise_set_workload(tool->context, params->kernel_workload);
+	if (retval < -1) {
+		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
+		goto out_err;
+	}
+
+	return 0;
+
+out_err:
+	return -1;
+}
+
 static void timerlat_usage(int err)
 {
 	int i;
diff --git a/tools/tracing/rtla/src/timerlat.h b/tools/tracing/rtla/src/timerlat.h
index cadc613dc82e..73045aef23fa 100644
--- a/tools/tracing/rtla/src/timerlat.h
+++ b/tools/tracing/rtla/src/timerlat.h
@@ -15,17 +15,15 @@ struct timerlat_params {
 	int			sleep_time;
 	int			output_divisor;
 	int			duration;
-	int			quiet;
 	int			set_sched;
 	int			dma_latency;
 	int			no_aa;
-	int			aa_only;
 	int			dump_tasks;
 	int			cgroup;
 	int			hk_cpus;
 	int			user_workload;
 	int			kernel_workload;
-	int			pretty_output;
+	int			user_data;
 	int			warmup;
 	int			buffer_size;
 	int			deepest_idle_state;
@@ -35,11 +33,12 @@ struct timerlat_params {
 	union {
 		struct {
 			/* top only */
-			int			user_top;
+			int			quiet;
+			int			aa_only;
+			int			pretty_output;
 		};
 		struct {
 			/* hist only */
-			int			user_hist;
 			char			no_irq;
 			char			no_thread;
 			char			no_header;
@@ -52,6 +51,8 @@ struct timerlat_params {
 	};
 };
 
+int timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params);
+
 int timerlat_hist_main(int argc, char *argv[]);
 int timerlat_top_main(int argc, char *argv[]);
 int timerlat_main(int argc, char *argv[]);
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 822c068b4776..9d9efeedc4c2 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -315,7 +315,7 @@ static void timerlat_hist_header(struct osnoise_tool *tool)
 		if (!params->no_thread)
 			trace_seq_printf(s, "   Thr-%03d", cpu);
 
-		if (params->user_hist)
+		if (params->user_data)
 			trace_seq_printf(s, "   Usr-%03d", cpu);
 	}
 	trace_seq_printf(s, "\n");
@@ -371,7 +371,7 @@ timerlat_print_summary(struct timerlat_params *params,
 			trace_seq_printf(trace->seq, "%9llu ",
 					data->hist[cpu].thread_count);
 
-		if (params->user_hist)
+		if (params->user_data)
 			trace_seq_printf(trace->seq, "%9llu ",
 					 data->hist[cpu].user_count);
 	}
@@ -399,7 +399,7 @@ timerlat_print_summary(struct timerlat_params *params,
 					     data->hist[cpu].min_thread,
 					     false);
 
-		if (params->user_hist)
+		if (params->user_data)
 			format_summary_value(trace->seq,
 					     data->hist[cpu].user_count,
 					     data->hist[cpu].min_user,
@@ -429,7 +429,7 @@ timerlat_print_summary(struct timerlat_params *params,
 					     data->hist[cpu].sum_thread,
 					     true);
 
-		if (params->user_hist)
+		if (params->user_data)
 			format_summary_value(trace->seq,
 					     data->hist[cpu].user_count,
 					     data->hist[cpu].sum_user,
@@ -459,7 +459,7 @@ timerlat_print_summary(struct timerlat_params *params,
 					     data->hist[cpu].max_thread,
 					     false);
 
-		if (params->user_hist)
+		if (params->user_data)
 			format_summary_value(trace->seq,
 					     data->hist[cpu].user_count,
 					     data->hist[cpu].max_user,
@@ -521,7 +521,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
 	if (!params->no_thread)
 		trace_seq_printf(trace->seq, "       Thr");
 
-	if (params->user_hist)
+	if (params->user_data)
 		trace_seq_printf(trace->seq, "       Usr");
 
 	trace_seq_printf(trace->seq, "\n");
@@ -537,7 +537,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
 		trace_seq_printf(trace->seq, "%9llu ",
 				 sum.thread_count);
 
-	if (params->user_hist)
+	if (params->user_data)
 		trace_seq_printf(trace->seq, "%9llu ",
 				 sum.user_count);
 
@@ -558,7 +558,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
 				     sum.min_thread,
 				     false);
 
-	if (params->user_hist)
+	if (params->user_data)
 		format_summary_value(trace->seq,
 				     sum.user_count,
 				     sum.min_user,
@@ -581,7 +581,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
 				     sum.sum_thread,
 				     true);
 
-	if (params->user_hist)
+	if (params->user_data)
 		format_summary_value(trace->seq,
 				     sum.user_count,
 				     sum.sum_user,
@@ -604,7 +604,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
 				     sum.max_thread,
 				     false);
 
-	if (params->user_hist)
+	if (params->user_data)
 		format_summary_value(trace->seq,
 				     sum.user_count,
 				     sum.max_user,
@@ -654,7 +654,7 @@ timerlat_print_stats(struct timerlat_params *params, struct osnoise_tool *tool)
 						data->hist[cpu].thread[bucket]);
 			}
 
-			if (params->user_hist) {
+			if (params->user_data) {
 				total += data->hist[cpu].user[bucket];
 				trace_seq_printf(trace->seq, "%9d ",
 						data->hist[cpu].user[bucket]);
@@ -690,7 +690,7 @@ timerlat_print_stats(struct timerlat_params *params, struct osnoise_tool *tool)
 			trace_seq_printf(trace->seq, "%9d ",
 					 data->hist[cpu].thread[data->entries]);
 
-		if (params->user_hist)
+		if (params->user_data)
 			trace_seq_printf(trace->seq, "%9d ",
 					 data->hist[cpu].user[data->entries]);
 	}
@@ -965,7 +965,7 @@ static struct timerlat_params
 			params->user_workload = 1;
 			/* fallback: -u implies in -U */
 		case 'U':
-			params->user_hist = 1;
+			params->user_data = 1;
 			break;
 		case '0': /* no irq */
 			params->no_irq = 1;
@@ -1063,98 +1063,11 @@ static struct timerlat_params
 static int
 timerlat_hist_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
 {
-	int retval, i;
-
-	if (!params->sleep_time)
-		params->sleep_time = 1;
-
-	if (params->cpus) {
-		retval = osnoise_set_cpus(tool->context, params->cpus);
-		if (retval) {
-			err_msg("Failed to apply CPUs config\n");
-			goto out_err;
-		}
-	} else {
-		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
-			CPU_SET(i, &params->monitored_cpus);
-	}
-
-	if (params->stop_us) {
-		retval = osnoise_set_stop_us(tool->context, params->stop_us);
-		if (retval) {
-			err_msg("Failed to set stop us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->stop_total_us) {
-		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
-		if (retval) {
-			err_msg("Failed to set stop total us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->timerlat_period_us) {
-		retval = osnoise_set_timerlat_period_us(tool->context, params->timerlat_period_us);
-		if (retval) {
-			err_msg("Failed to set timerlat period\n");
-			goto out_err;
-		}
-	}
-
-	if (params->print_stack) {
-		retval = osnoise_set_print_stack(tool->context, params->print_stack);
-		if (retval) {
-			err_msg("Failed to set print stack\n");
-			goto out_err;
-		}
-	}
-
-	if (params->hk_cpus) {
-		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
-					   &params->hk_cpu_set);
-		if (retval == -1) {
-			err_msg("Failed to set rtla to the house keeping CPUs\n");
-			goto out_err;
-		}
-	} else if (params->cpus) {
-		/*
-		 * Even if the user do not set a house-keeping CPU, try to
-		 * move rtla to a CPU set different to the one where the user
-		 * set the workload to run.
-		 *
-		 * No need to check results as this is an automatic attempt.
-		 */
-		auto_house_keeping(&params->monitored_cpus);
-	}
-
-	/*
-	 * If the user did not specify a type of thread, try user-threads first.
-	 * Fall back to kernel threads otherwise.
-	 */
-	if (!params->kernel_workload && !params->user_hist) {
-		retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd");
-		if (retval) {
-			debug_msg("User-space interface detected, setting user-threads\n");
-			params->user_workload = 1;
-			params->user_hist = 1;
-		} else {
-			debug_msg("User-space interface not detected, setting kernel-threads\n");
-			params->kernel_workload = 1;
-		}
-	}
+	int retval;
 
-	/*
-	* Set workload according to type of thread if the kernel supports it.
-	* On kernels without support, user threads will have already failed
-	* on missing timerlat_fd, and kernel threads do not need it.
-	*/
-	retval = osnoise_set_workload(tool->context, params->kernel_workload);
-	if (retval < -1) {
-		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
+	retval = timerlat_apply_config(tool, params);
+	if (retval)
 		goto out_err;
-	}
 
 	return 0;
 
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index c3196a0bb585..79cb6f28967f 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -266,7 +266,7 @@ static void timerlat_top_header(struct timerlat_params *params, struct osnoise_t
 		trace_seq_printf(s, "\033[2;37;40m");
 
 	trace_seq_printf(s, "                                     Timer Latency                                              ");
-	if (params->user_top)
+	if (params->user_data)
 		trace_seq_printf(s, "                                         ");
 
 	if (params->pretty_output)
@@ -277,7 +277,7 @@ static void timerlat_top_header(struct timerlat_params *params, struct osnoise_t
 			params->output_divisor == 1 ? "ns" : "us",
 			params->output_divisor == 1 ? "ns" : "us");
 
-	if (params->user_top) {
+	if (params->user_data) {
 		trace_seq_printf(s, "      |    Ret user Timer Latency (%s)",
 				params->output_divisor == 1 ? "ns" : "us");
 	}
@@ -287,7 +287,7 @@ static void timerlat_top_header(struct timerlat_params *params, struct osnoise_t
 		trace_seq_printf(s, "\033[2;30;47m");
 
 	trace_seq_printf(s, "CPU COUNT      |      cur       min       avg       max |      cur       min       avg       max");
-	if (params->user_top)
+	if (params->user_data)
 		trace_seq_printf(s, " |      cur       min       avg       max");
 
 	if (params->pretty_output)
@@ -338,7 +338,7 @@ static void timerlat_top_print(struct osnoise_tool *top, int cpu)
 		trace_seq_printf(s, "%9llu", cpu_data->max_thread);
 	}
 
-	if (!params->user_top) {
+	if (!params->user_data) {
 		trace_seq_printf(s, "\n");
 		return;
 	}
@@ -380,7 +380,7 @@ timerlat_top_print_sum(struct osnoise_tool *top, struct timerlat_top_cpu *summar
 	}
 
 	trace_seq_printf(s, "%.*s|%.*s|%.*s", 15, split, 40, split, 39, split);
-	if (params->user_top)
+	if (params->user_data)
 		trace_seq_printf(s, "-|%.*s", 39, split);
 	trace_seq_printf(s, "\n");
 
@@ -405,7 +405,7 @@ timerlat_top_print_sum(struct osnoise_tool *top, struct timerlat_top_cpu *summar
 		trace_seq_printf(s, "%9llu", summary->max_thread);
 	}
 
-	if (!params->user_top) {
+	if (!params->user_data) {
 		trace_seq_printf(s, "\n");
 		return;
 	}
@@ -722,7 +722,7 @@ static struct timerlat_params
 			params->user_workload = true;
 			/* fallback: -u implies -U */
 		case 'U':
-			params->user_top = true;
+			params->user_data = true;
 			break;
 		case '0': /* trigger */
 			if (params->events) {
@@ -800,100 +800,10 @@ static int
 timerlat_top_apply_config(struct osnoise_tool *top, struct timerlat_params *params)
 {
 	int retval;
-	int i;
-
-	if (!params->sleep_time)
-		params->sleep_time = 1;
-
-	if (params->cpus) {
-		retval = osnoise_set_cpus(top->context, params->cpus);
-		if (retval) {
-			err_msg("Failed to apply CPUs config\n");
-			goto out_err;
-		}
-	} else {
-		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
-			CPU_SET(i, &params->monitored_cpus);
-	}
-
-	if (params->stop_us) {
-		retval = osnoise_set_stop_us(top->context, params->stop_us);
-		if (retval) {
-			err_msg("Failed to set stop us\n");
-			goto out_err;
-		}
-	}
-
-	if (params->stop_total_us) {
-		retval = osnoise_set_stop_total_us(top->context, params->stop_total_us);
-		if (retval) {
-			err_msg("Failed to set stop total us\n");
-			goto out_err;
-		}
-	}
-
-
-	if (params->timerlat_period_us) {
-		retval = osnoise_set_timerlat_period_us(top->context, params->timerlat_period_us);
-		if (retval) {
-			err_msg("Failed to set timerlat period\n");
-			goto out_err;
-		}
-	}
 
-
-	if (params->print_stack) {
-		retval = osnoise_set_print_stack(top->context, params->print_stack);
-		if (retval) {
-			err_msg("Failed to set print stack\n");
-			goto out_err;
-		}
-	}
-
-	if (params->hk_cpus) {
-		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
-					   &params->hk_cpu_set);
-		if (retval == -1) {
-			err_msg("Failed to set rtla to the house keeping CPUs\n");
-			goto out_err;
-		}
-	} else if (params->cpus) {
-		/*
-		 * Even if the user do not set a house-keeping CPU, try to
-		 * move rtla to a CPU set different to the one where the user
-		 * set the workload to run.
-		 *
-		 * No need to check results as this is an automatic attempt.
-		 */
-		auto_house_keeping(&params->monitored_cpus);
-	}
-
-	/*
-	 * If the user did not specify a type of thread, try user-threads first.
-	 * Fall back to kernel threads otherwise.
-	 */
-	if (!params->kernel_workload && !params->user_top) {
-		retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd");
-		if (retval) {
-			debug_msg("User-space interface detected, setting user-threads\n");
-			params->user_workload = 1;
-			params->user_top = 1;
-		} else {
-			debug_msg("User-space interface not detected, setting kernel-threads\n");
-			params->kernel_workload = 1;
-		}
-	}
-
-	/*
-	* Set workload according to type of thread if the kernel supports it.
-	* On kernels without support, user threads will have already failed
-	* on missing timerlat_fd, and kernel threads do not need it.
-	*/
-	retval = osnoise_set_workload(top->context, params->kernel_workload);
-	if (retval < -1) {
-		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
+	retval = timerlat_apply_config(top, params);
+	if (retval)
 		goto out_err;
-	}
 
 	if (isatty(STDOUT_FILENO) && !params->quiet)
 		params->pretty_output = 1;
@@ -1142,7 +1052,7 @@ int timerlat_top_main(int argc, char *argv[])
 		}
 	}
 
-	if (params->cgroup && !params->user_top) {
+	if (params->cgroup && !params->user_data) {
 		retval = set_comm_cgroup("timerlat/", params->cgroup_name);
 		if (!retval) {
 			err_msg("Failed to move threads to cgroup\n");
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/6] rtla/osnoise: Set OSNOISE_WORKLOAD to true
  2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
  2025-03-20  9:24 ` [PATCH 1/6] rtla/osnoise: Unify params struct Tomas Glozar
  2025-03-20  9:24 ` [PATCH 2/6] rtla: Unify apply_config between top and hist Tomas Glozar
@ 2025-03-20  9:24 ` Tomas Glozar
  2025-03-20 19:40   ` John Kacur
  2025-03-20  9:24 ` [PATCH 4/6] rtla: Always set all tracer options Tomas Glozar
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

If running rtla osnoise with NO_OSNOISE_WORKLOAD, it reports no samples:

$ echo NO_OSNOISE_WORKLOAD > /sys/kernel/tracing/osnoise/options
$ rtla osnoise hist -d 10s
Index
over: 0
count: 0
min: 0
avg: 0
max: 0

This situation can also happen when running rtla-osnoise after an
improperly exited rtla-timerlat run.

Set OSNOISE_WORKLOAD in rtla-osnoise, too, similarly to what we
already did for timerlat in commit 217f0b1e990e ("rtla/timerlat_top: Set
OSNOISE_WORKLOAD for kernel threads") and commit d8d866171a41
("rtla/timerlat_hist: Set OSNOISE_WORKLOAD for kernel threads").

Note that there is no user workload mode for rtla-osnoise yet, so
OSNOISE_WORKLOAD is always set to true.

Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
Fixes: 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/src/osnoise.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 1735a36466c4..a71618d876e9 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -1187,6 +1187,12 @@ osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
 		auto_house_keeping(&params->monitored_cpus);
 	}
 
+	retval = osnoise_set_workload(tool->context, true);
+	if (retval < -1) {
+		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
+		goto out_err;
+	}
+
 	return 0;
 
 out_err:
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/6] rtla: Always set all tracer options
  2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
                   ` (2 preceding siblings ...)
  2025-03-20  9:24 ` [PATCH 3/6] rtla/osnoise: Set OSNOISE_WORKLOAD to true Tomas Glozar
@ 2025-03-20  9:24 ` Tomas Glozar
  2025-03-20 20:31   ` John Kacur
  2025-03-20  9:24 ` [PATCH 5/6] rtla/tests: Reset osnoise options before check Tomas Glozar
  2025-03-20  9:25 ` [PATCH 6/6] rtla/tests: Test setting default options Tomas Glozar
  5 siblings, 1 reply; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

rtla currently only sets tracer options that are explicitly set by the
user, with the exception of OSNOISE_WORKLOAD.

This leads to improper behavior in case rtla is run with those options
not set to the default value. rtla does reset them to the original
value upon exiting, but that does not protect it from starting with
non-default values set either by an improperly exited rtla or by another
user of the tracers.

For example, after running this command:

$ echo 1 > /sys/kernel/tracing/osnoise/stop_tracing_us

all runs of rtla will stop at the 1us threshold, even if not requested
by the user:

$ rtla osnoise hist
Index   CPU-000   CPU-001
1             8         5
2             5         9
3             1         2
4             6         1
5             2         1
6             0         1
8             1         1
12            0         1
14            1         0
15            1         0
over:         0         0
count:       25        21
min:          1         1
avg:       3.68      3.05
max:         15        12
rtla osnoise hit stop tracing

Fix the problem by setting the default value for all tracer options if
the user has not provided their own value.

For most of the options, it's enough to just drop the if clause checking
for the value being set. For cpus, "all" is used as the default value,
and for osnoise default period and runtime, default values of
the osnoise_data variable in trace_osnoise.c are used.

Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
Fixes: 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
Fixes: a828cd18bc4a ("rtla: Add timerlat tool and timelart top mode")
Fixes: 1eeb6328e8b3 ("rtla/timerlat: Add timerlat hist mode")
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/src/osnoise.c  | 56 ++++++++++++++---------------
 tools/tracing/rtla/src/timerlat.c | 59 +++++++++++++++----------------
 2 files changed, 56 insertions(+), 59 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index a71618d876e9..2dc3e4539e99 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -17,6 +17,9 @@
 
 #include "osnoise.h"
 
+#define DEFAULT_SAMPLE_PERIOD	1000000			/* 1s */
+#define DEFAULT_SAMPLE_RUNTIME	1000000			/* 1s */
+
 /*
  * osnoise_get_cpus - return the original "osnoise/cpus" content
  *
@@ -1127,46 +1130,43 @@ osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
 	if (!params->sleep_time)
 		params->sleep_time = 1;
 
-	if (params->cpus) {
-		retval = osnoise_set_cpus(tool->context, params->cpus);
-		if (retval) {
-			err_msg("Failed to apply CPUs config\n");
-			goto out_err;
-		}
+	retval = osnoise_set_cpus(tool->context, params->cpus ? params->cpus : "all");
+	if (retval) {
+		err_msg("Failed to apply CPUs config\n");
+		goto out_err;
 	}
 
 	if (params->runtime || params->period) {
 		retval = osnoise_set_runtime_period(tool->context,
 						    params->runtime,
 						    params->period);
-		if (retval) {
-			err_msg("Failed to set runtime and/or period\n");
-			goto out_err;
-		}
+	} else {
+		retval = osnoise_set_runtime_period(tool->context,
+						    DEFAULT_SAMPLE_PERIOD,
+						    DEFAULT_SAMPLE_RUNTIME);
 	}
 
-	if (params->stop_us) {
-		retval = osnoise_set_stop_us(tool->context, params->stop_us);
-		if (retval) {
-			err_msg("Failed to set stop us\n");
-			goto out_err;
-		}
+	if (retval) {
+		err_msg("Failed to set runtime and/or period\n");
+		goto out_err;
 	}
 
-	if (params->stop_total_us) {
-		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
-		if (retval) {
-			err_msg("Failed to set stop total us\n");
-			goto out_err;
-		}
+	retval = osnoise_set_stop_us(tool->context, params->stop_us);
+	if (retval) {
+		err_msg("Failed to set stop us\n");
+		goto out_err;
 	}
 
-	if (params->threshold) {
-		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
-		if (retval) {
-			err_msg("Failed to set tracing_thresh\n");
-			goto out_err;
-		}
+	retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
+	if (retval) {
+		err_msg("Failed to set stop total us\n");
+		goto out_err;
+	}
+
+	retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
+	if (retval) {
+		err_msg("Failed to set tracing_thresh\n");
+		goto out_err;
 	}
 
 	if (params->hk_cpus) {
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 448fb1f7d3a6..c29e2ba2d7d8 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -16,6 +16,8 @@
 
 #include "timerlat.h"
 
+#define DEFAULT_TIMERLAT_PERIOD	1000			/* 1ms */
+
 /*
  * timerlat_apply_config - apply common configs to the initialized tool
  */
@@ -27,49 +29,44 @@ timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
 	if (!params->sleep_time)
 		params->sleep_time = 1;
 
-	if (params->cpus) {
-		retval = osnoise_set_cpus(tool->context, params->cpus);
-		if (retval) {
-			err_msg("Failed to apply CPUs config\n");
-			goto out_err;
-		}
-	} else {
+	retval = osnoise_set_cpus(tool->context, params->cpus ? params->cpus : "all");
+	if (retval) {
+		err_msg("Failed to apply CPUs config\n");
+		goto out_err;
+	}
+
+	if (!params->cpus) {
 		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
 			CPU_SET(i, &params->monitored_cpus);
 	}
 
-	if (params->stop_us) {
-		retval = osnoise_set_stop_us(tool->context, params->stop_us);
-		if (retval) {
-			err_msg("Failed to set stop us\n");
-			goto out_err;
-		}
+	retval = osnoise_set_stop_us(tool->context, params->stop_us);
+	if (retval) {
+		err_msg("Failed to set stop us\n");
+		goto out_err;
 	}
 
-	if (params->stop_total_us) {
-		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
-		if (retval) {
-			err_msg("Failed to set stop total us\n");
-			goto out_err;
-		}
+	retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
+	if (retval) {
+		err_msg("Failed to set stop total us\n");
+		goto out_err;
 	}
 
 
-	if (params->timerlat_period_us) {
-		retval = osnoise_set_timerlat_period_us(tool->context, params->timerlat_period_us);
-		if (retval) {
-			err_msg("Failed to set timerlat period\n");
-			goto out_err;
-		}
+	retval = osnoise_set_timerlat_period_us(tool->context,
+						params->timerlat_period_us ?
+						params->timerlat_period_us :
+						DEFAULT_TIMERLAT_PERIOD);
+	if (retval) {
+		err_msg("Failed to set timerlat period\n");
+		goto out_err;
 	}
 
 
-	if (params->print_stack) {
-		retval = osnoise_set_print_stack(tool->context, params->print_stack);
-		if (retval) {
-			err_msg("Failed to set print stack\n");
-			goto out_err;
-		}
+	retval = osnoise_set_print_stack(tool->context, params->print_stack);
+	if (retval) {
+		err_msg("Failed to set print stack\n");
+		goto out_err;
 	}
 
 	if (params->hk_cpus) {
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/6] rtla/tests: Reset osnoise options before check
  2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
                   ` (3 preceding siblings ...)
  2025-03-20  9:24 ` [PATCH 4/6] rtla: Always set all tracer options Tomas Glozar
@ 2025-03-20  9:24 ` Tomas Glozar
  2025-03-20 20:36   ` John Kacur
  2025-03-20  9:25 ` [PATCH 6/6] rtla/tests: Test setting default options Tomas Glozar
  5 siblings, 1 reply; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

Remove any dangling tracing instances from previous improperly exited
runs of rtla, and reset osnoise options to default before running a test
case.

This ensures that the test results are deterministic. Specific test
cases checked that rtla behaves correctly even when the tracer state is
not clean will be added later.

Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/tests/engine.sh | 40 ++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tools/tracing/rtla/tests/engine.sh b/tools/tracing/rtla/tests/engine.sh
index 64d0446dc28e..5db8aa4bc031 100644
--- a/tools/tracing/rtla/tests/engine.sh
+++ b/tools/tracing/rtla/tests/engine.sh
@@ -8,12 +8,44 @@ test_begin() {
 	[ -n "$TEST_COUNT" ] && echo "1..$TEST_COUNT"
 }
 
+reset_osnoise() {
+	# Reset osnoise options to default and remove any dangling instances created
+	# by improperly exited rtla runs.
+	pushd /sys/kernel/tracing || return 1
+
+	# Remove dangling instances created by previous rtla run
+	echo 0 > tracing_thresh
+	cd instances
+	for i in osnoise_top osnoise_hist timerlat_top timerlat_hist
+	do
+		[ ! -d "$i" ] && continue
+		rmdir "$i"
+	done
+
+	# Reset options to default
+	# Note: those are copied from the default values of osnoise_data
+	# in kernel/trace/trace_osnoise.c
+	cd ../osnoise
+	echo all > cpus
+	echo DEFAULTS > options
+	echo 1000000 > period_us
+	echo 0 > print_stack
+	echo 1000000 > runtime_us
+	echo 0 > stop_tracing_total_us
+	echo 0 > stop_tracing_us
+	echo 1000 > timerlat_period_us
+
+	popd
+}
+
 check() {
 	# Simple check: run rtla with given arguments and test exit code.
 	# If TEST_COUNT is set, run the test. Otherwise, just count.
 	ctr=$(($ctr + 1))
 	if [ -n "$TEST_COUNT" ]
 	then
+		# Reset osnoise options before running test.
+		[ "$NO_RESET_OSNOISE" == 1 ] || reset_osnoise
 		# Run rtla; in case of failure, include its output as comment
 		# in the test results.
 		result=$(stdbuf -oL $TIMEOUT "$RTLA" $2 2>&1); exitcode=$?
@@ -37,6 +69,14 @@ unset_timeout() {
 	unset TIMEOUT
 }
 
+set_no_reset_osnoise() {
+	NO_RESET_OSNOISE=1
+}
+
+unset_no_reset_osnoise() {
+	unset NO_RESET_OSNOISE
+}
+
 test_end() {
 	# If running without TEST_COUNT, tests are not actually run, just
 	# counted. In that case, re-run the test with the correct count.
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/6] rtla/tests: Test setting default options
  2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
                   ` (4 preceding siblings ...)
  2025-03-20  9:24 ` [PATCH 5/6] rtla/tests: Reset osnoise options before check Tomas Glozar
@ 2025-03-20  9:25 ` Tomas Glozar
  2025-03-20 20:42   ` John Kacur
  5 siblings, 1 reply; 14+ messages in thread
From: Tomas Glozar @ 2025-03-20  9:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-trace-kernel, linux-kernel, John Kacur, Luis Goncalves,
	Tomas Glozar

Add function to test engine to test with pre-set osnoise options, and
use it to test whether osnoise period (as an example) is set correctly.

The test works by pre-setting a high period of 10 minutes and stop on
threshold. Thus, it is easy to check whether rtla is properly resetting
the period to default: if it is, the test will complete on time, since
the first sample will overflow the threshold. If not, it will time out.

Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/tests/engine.sh | 26 ++++++++++++++++++++++++++
 tools/tracing/rtla/tests/osnoise.t |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/tools/tracing/rtla/tests/engine.sh b/tools/tracing/rtla/tests/engine.sh
index 5db8aa4bc031..b1697b3e3f52 100644
--- a/tools/tracing/rtla/tests/engine.sh
+++ b/tools/tracing/rtla/tests/engine.sh
@@ -61,6 +61,32 @@ check() {
 	fi
 }
 
+check_with_osnoise_options() {
+	# Do the same as "check", but with pre-set osnoise options.
+	# Note: rtla should reset the osnoise options, this is used to test
+	# if it indeed does so.
+	# Save original arguments
+	arg1=$1
+	arg2=$2
+
+	# Apply osnoise options (if not dry run)
+	if [ -n "$TEST_COUNT" ]
+	then
+		[ "$NO_RESET_OSNOISE" == 1 ] || reset_osnoise
+		shift
+		while shift
+		do
+			[ "$1" == "" ] && continue
+			option=$(echo $1 | cut -d '=' -f 1)
+			value=$(echo $1 | cut -d '=' -f 2)
+			echo "option: $option, value: $value"
+			echo "$value" > "/sys/kernel/tracing/osnoise/$option" || return 1
+		done
+	fi
+
+	NO_RESET_OSNOISE=1 check "$arg1" "$arg2"
+}
+
 set_timeout() {
 	TIMEOUT="timeout -v -k 15s $1"
 }
diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
index 86596e547893..e5995c03c790 100644
--- a/tools/tracing/rtla/tests/osnoise.t
+++ b/tools/tracing/rtla/tests/osnoise.t
@@ -16,4 +16,10 @@ check "verify the  --trace param" \
 check "verify the --entries/-E param" \
 	"osnoise hist -P F:1 -c 0 -r 900000 -d 1M -b 10 -E 25"
 
+# Test setting default period by putting an absurdly high period
+# and stopping on threshold.
+# If default period is not set, this will time out.
+check_with_osnoise_options "apply default period" \
+	"osnoise hist -s 1" period_us=600000000
+
 test_end
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/6] rtla/osnoise: Unify params struct
  2025-03-20  9:24 ` [PATCH 1/6] rtla/osnoise: Unify params struct Tomas Glozar
@ 2025-03-20 19:00   ` John Kacur
  2025-03-24 16:06     ` Steven Rostedt
  0 siblings, 1 reply; 14+ messages in thread
From: John Kacur @ 2025-03-20 19:00 UTC (permalink / raw)
  To: Tomas Glozar
  Cc: Steven Rostedt, linux-trace-kernel, linux-kernel, Luis Goncalves



On Thu, 20 Mar 2025, Tomas Glozar wrote:

> Instead of having separate structs osnoise_top_params and
> osnoise_hist_params, use one struct osnoise_params for both.
> 
> This allows code using the structs to be shared between osnoise-top and
> osnoise-hist.
> 
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/src/osnoise.c      |  1 -
>  tools/tracing/rtla/src/osnoise.h      | 47 +++++++++++++++++++++++
>  tools/tracing/rtla/src/osnoise_hist.c | 52 ++++++--------------------
>  tools/tracing/rtla/src/osnoise_top.c  | 54 +++++----------------------
>  tools/tracing/rtla/src/timerlat.h     |  1 -
>  5 files changed, 68 insertions(+), 87 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
> index 85f398b89597..93d485c0e949 100644
> --- a/tools/tracing/rtla/src/osnoise.c
> +++ b/tools/tracing/rtla/src/osnoise.c
> @@ -14,7 +14,6 @@
>  #include <stdio.h>
>  
>  #include "osnoise.h"
> -#include "utils.h"
>  
>  /*
>   * osnoise_get_cpus - return the original "osnoise/cpus" content
> diff --git a/tools/tracing/rtla/src/osnoise.h b/tools/tracing/rtla/src/osnoise.h
> index 056c8b113dee..f78ffbdc8c8d 100644
> --- a/tools/tracing/rtla/src/osnoise.h
> +++ b/tools/tracing/rtla/src/osnoise.h
> @@ -1,8 +1,55 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #pragma once
>  
> +#include "utils.h"
>  #include "trace.h"
>  
> +enum osnoise_mode {
> +	MODE_OSNOISE = 0,
> +	MODE_HWNOISE
> +};
> +
> +struct osnoise_params {
> +	/* Common params */
> +	char			*cpus;
> +	cpu_set_t		monitored_cpus;
> +	char			*trace_output;
> +	char			*cgroup_name;
> +	unsigned long long	runtime;
> +	unsigned long long	period;
> +	long long		threshold;
> +	long long		stop_us;
> +	long long		stop_total_us;
> +	int			sleep_time;
> +	int			duration;
> +	int			set_sched;
> +	int			cgroup;
> +	int			hk_cpus;
> +	cpu_set_t		hk_cpu_set;
> +	struct sched_attr	sched_param;
> +	struct trace_events	*events;
> +	int			warmup;
> +	int			buffer_size;
> +	union {
> +		struct {
> +			/* top only */
> +			int			quiet;
> +			int			pretty_output;
> +			enum osnoise_mode	mode;
> +		};
> +		struct {
> +			/* hist only */
> +			int			output_divisor;
> +			char			no_header;
> +			char			no_summary;
> +			char			no_index;
> +			char			with_zeros;
> +			int			bucket_size;
> +			int			entries;
> +		};
> +	};
> +};
> +
>  /*
>   * osnoise_context - read, store, write, restore osnoise configs.
>   */
> diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
> index f4c9051c33c4..4721f15f77cd 100644
> --- a/tools/tracing/rtla/src/osnoise_hist.c
> +++ b/tools/tracing/rtla/src/osnoise_hist.c
> @@ -14,38 +14,8 @@
>  #include <time.h>
>  #include <sched.h>
>  
> -#include "utils.h"
>  #include "osnoise.h"
>  
> -struct osnoise_hist_params {
> -	char			*cpus;
> -	cpu_set_t		monitored_cpus;
> -	char			*trace_output;
> -	char			*cgroup_name;
> -	unsigned long long	runtime;
> -	unsigned long long	period;
> -	long long		threshold;
> -	long long		stop_us;
> -	long long		stop_total_us;
> -	int			sleep_time;
> -	int			duration;
> -	int			set_sched;
> -	int			output_divisor;
> -	int			cgroup;
> -	int			hk_cpus;
> -	cpu_set_t		hk_cpu_set;
> -	struct sched_attr	sched_param;
> -	struct trace_events	*events;
> -	char			no_header;
> -	char			no_summary;
> -	char			no_index;
> -	char			with_zeros;
> -	int			bucket_size;
> -	int			entries;
> -	int			warmup;
> -	int			buffer_size;
> -};
> -
>  struct osnoise_hist_cpu {
>  	int			*samples;
>  	int			count;
> @@ -126,7 +96,7 @@ static struct osnoise_hist_data
>  static void osnoise_hist_update_multiple(struct osnoise_tool *tool, int cpu,
>  					 unsigned long long duration, int count)
>  {
> -	struct osnoise_hist_params *params = tool->params;
> +	struct osnoise_params *params = tool->params;
>  	struct osnoise_hist_data *data = tool->data;
>  	unsigned long long total_duration;
>  	int entries = data->entries;
> @@ -168,7 +138,7 @@ static void osnoise_destroy_trace_hist(struct osnoise_tool *tool)
>   */
>  static int osnoise_init_trace_hist(struct osnoise_tool *tool)
>  {
> -	struct osnoise_hist_params *params = tool->params;
> +	struct osnoise_params *params = tool->params;
>  	struct osnoise_hist_data *data = tool->data;
>  	int bucket_size;
>  	char buff[128];
> @@ -253,7 +223,7 @@ static void osnoise_read_trace_hist(struct osnoise_tool *tool)
>   */
>  static void osnoise_hist_header(struct osnoise_tool *tool)
>  {
> -	struct osnoise_hist_params *params = tool->params;
> +	struct osnoise_params *params = tool->params;
>  	struct osnoise_hist_data *data = tool->data;
>  	struct trace_seq *s = tool->trace.seq;
>  	char duration[26];
> @@ -292,7 +262,7 @@ static void osnoise_hist_header(struct osnoise_tool *tool)
>   * osnoise_print_summary - print the summary of the hist data to the output
>   */
>  static void
> -osnoise_print_summary(struct osnoise_hist_params *params,
> +osnoise_print_summary(struct osnoise_params *params,
>  		       struct trace_instance *trace,
>  		       struct osnoise_hist_data *data)
>  {
> @@ -370,7 +340,7 @@ osnoise_print_summary(struct osnoise_hist_params *params,
>   * osnoise_print_stats - print data for all CPUs
>   */
>  static void
> -osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *tool)
> +osnoise_print_stats(struct osnoise_params *params, struct osnoise_tool *tool)
>  {
>  	struct osnoise_hist_data *data = tool->data;
>  	struct trace_instance *trace = &tool->trace;
> @@ -508,10 +478,10 @@ static void osnoise_hist_usage(char *usage)
>  /*
>   * osnoise_hist_parse_args - allocs, parse and fill the cmd line parameters
>   */
> -static struct osnoise_hist_params
> +static struct osnoise_params
>  *osnoise_hist_parse_args(int argc, char *argv[])
>  {
> -	struct osnoise_hist_params *params;
> +	struct osnoise_params *params;
>  	struct trace_events *tevent;
>  	int retval;
>  	int c;
> @@ -731,7 +701,7 @@ static struct osnoise_hist_params
>   * osnoise_hist_apply_config - apply the hist configs to the initialized tool
>   */
>  static int
> -osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_hist_params *params)
> +osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
>  {
>  	int retval;
>  
> @@ -808,7 +778,7 @@ osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_hist_params
>   * osnoise_init_hist - initialize a osnoise hist tool with parameters
>   */
>  static struct osnoise_tool
> -*osnoise_init_hist(struct osnoise_hist_params *params)
> +*osnoise_init_hist(struct osnoise_params *params)
>  {
>  	struct osnoise_tool *tool;
>  	int nr_cpus;
> @@ -842,7 +812,7 @@ static void stop_hist(int sig)
>   * osnoise_hist_set_signals - handles the signal to stop the tool
>   */
>  static void
> -osnoise_hist_set_signals(struct osnoise_hist_params *params)
> +osnoise_hist_set_signals(struct osnoise_params *params)
>  {
>  	signal(SIGINT, stop_hist);
>  	if (params->duration) {
> @@ -853,7 +823,7 @@ osnoise_hist_set_signals(struct osnoise_hist_params *params)
>  
>  int osnoise_hist_main(int argc, char *argv[])
>  {
> -	struct osnoise_hist_params *params;
> +	struct osnoise_params *params;
>  	struct osnoise_tool *record = NULL;
>  	struct osnoise_tool *tool = NULL;
>  	struct trace_instance *trace;
> diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
> index dacec2f99017..7f393019bbf5 100644
> --- a/tools/tracing/rtla/src/osnoise_top.c
> +++ b/tools/tracing/rtla/src/osnoise_top.c
> @@ -14,40 +14,6 @@
>  #include <sched.h>
>  
>  #include "osnoise.h"
> -#include "utils.h"
> -
> -enum osnoise_mode {
> -	MODE_OSNOISE = 0,
> -	MODE_HWNOISE
> -};
> -
> -/*
> - * osnoise top parameters
> - */
> -struct osnoise_top_params {
> -	char			*cpus;
> -	cpu_set_t		monitored_cpus;
> -	char			*trace_output;
> -	char			*cgroup_name;
> -	unsigned long long	runtime;
> -	unsigned long long	period;
> -	long long		threshold;
> -	long long		stop_us;
> -	long long		stop_total_us;
> -	int			sleep_time;
> -	int			duration;
> -	int			quiet;
> -	int			set_sched;
> -	int			cgroup;
> -	int			hk_cpus;
> -	int			warmup;
> -	int			buffer_size;
> -	int			pretty_output;
> -	cpu_set_t		hk_cpu_set;
> -	struct sched_attr	sched_param;
> -	struct trace_events	*events;
> -	enum osnoise_mode	mode;
> -};
>  
>  struct osnoise_top_cpu {
>  	unsigned long long	sum_runtime;
> @@ -158,7 +124,7 @@ osnoise_top_handler(struct trace_seq *s, struct tep_record *record,
>   */
>  static void osnoise_top_header(struct osnoise_tool *top)
>  {
> -	struct osnoise_top_params *params = top->params;
> +	struct osnoise_params *params = top->params;
>  	struct trace_seq *s = top->trace.seq;
>  	char duration[26];
>  
> @@ -218,7 +184,7 @@ static void clear_terminal(struct trace_seq *seq)
>   */
>  static void osnoise_top_print(struct osnoise_tool *tool, int cpu)
>  {
> -	struct osnoise_top_params *params = tool->params;
> +	struct osnoise_params *params = tool->params;
>  	struct trace_seq *s = tool->trace.seq;
>  	struct osnoise_top_cpu *cpu_data;
>  	struct osnoise_top_data *data;
> @@ -258,7 +224,7 @@ static void osnoise_top_print(struct osnoise_tool *tool, int cpu)
>   * osnoise_print_stats - print data for all cpus
>   */
>  static void
> -osnoise_print_stats(struct osnoise_top_params *params, struct osnoise_tool *top)
> +osnoise_print_stats(struct osnoise_params *params, struct osnoise_tool *top)
>  {
>  	struct trace_instance *trace = &top->trace;
>  	static int nr_cpus = -1;
> @@ -286,7 +252,7 @@ osnoise_print_stats(struct osnoise_top_params *params, struct osnoise_tool *top)
>  /*
>   * osnoise_top_usage - prints osnoise top usage message
>   */
> -static void osnoise_top_usage(struct osnoise_top_params *params, char *usage)
> +static void osnoise_top_usage(struct osnoise_params *params, char *usage)
>  {
>  	int i;
>  
> @@ -354,9 +320,9 @@ static void osnoise_top_usage(struct osnoise_top_params *params, char *usage)
>  /*
>   * osnoise_top_parse_args - allocs, parse and fill the cmd line parameters
>   */
> -struct osnoise_top_params *osnoise_top_parse_args(int argc, char **argv)
> +struct osnoise_params *osnoise_top_parse_args(int argc, char **argv)
>  {
> -	struct osnoise_top_params *params;
> +	struct osnoise_params *params;
>  	struct trace_events *tevent;
>  	int retval;
>  	int c;
> @@ -553,7 +519,7 @@ struct osnoise_top_params *osnoise_top_parse_args(int argc, char **argv)
>   * osnoise_top_apply_config - apply the top configs to the initialized tool
>   */
>  static int
> -osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_top_params *params)
> +osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
>  {
>  	int retval;
>  
> @@ -640,7 +606,7 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_top_params *p
>  /*
>   * osnoise_init_top - initialize a osnoise top tool with parameters
>   */
> -struct osnoise_tool *osnoise_init_top(struct osnoise_top_params *params)
> +struct osnoise_tool *osnoise_init_top(struct osnoise_params *params)
>  {
>  	struct osnoise_tool *tool;
>  	int nr_cpus;
> @@ -674,7 +640,7 @@ static void stop_top(int sig)
>  /*
>   * osnoise_top_set_signals - handles the signal to stop the tool
>   */
> -static void osnoise_top_set_signals(struct osnoise_top_params *params)
> +static void osnoise_top_set_signals(struct osnoise_params *params)
>  {
>  	signal(SIGINT, stop_top);
>  	if (params->duration) {
> @@ -685,7 +651,7 @@ static void osnoise_top_set_signals(struct osnoise_top_params *params)
>  
>  int osnoise_top_main(int argc, char **argv)
>  {
> -	struct osnoise_top_params *params;
> +	struct osnoise_params *params;
>  	struct osnoise_tool *record = NULL;
>  	struct osnoise_tool *tool = NULL;
>  	struct trace_instance *trace;
> diff --git a/tools/tracing/rtla/src/timerlat.h b/tools/tracing/rtla/src/timerlat.h
> index e452d385cb0f..cadc613dc82e 100644
> --- a/tools/tracing/rtla/src/timerlat.h
> +++ b/tools/tracing/rtla/src/timerlat.h
> @@ -1,5 +1,4 @@
>  // SPDX-License-Identifier: GPL-2.0
> -#include "utils.h"
>  #include "osnoise.h"
>  
>  struct timerlat_params {
> -- 

Reviewed-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/6] rtla: Unify apply_config between top and hist
  2025-03-20  9:24 ` [PATCH 2/6] rtla: Unify apply_config between top and hist Tomas Glozar
@ 2025-03-20 19:27   ` John Kacur
  0 siblings, 0 replies; 14+ messages in thread
From: John Kacur @ 2025-03-20 19:27 UTC (permalink / raw)
  To: Tomas Glozar
  Cc: Steven Rostedt, linux-trace-kernel, linux-kernel, Luis Goncalves



On Thu, 20 Mar 2025, Tomas Glozar wrote:

> The functions osnoise_top_apply_config and osnoise_hist_apply_config, as
> well as timerlat_top_apply_config and timerlat_hist_apply_config, are
> mostly the same.
> 
> Move common part from them into separate functions osnoise_apply_config
> and timerlat_apply_config.
> 
> For rtla-timerlat, also unify params->user_hist and params->user_top
> into one field called params->user_data, and move several fields used
> only by timerlat-top into the top-only section of struct
> timerlat_params.
> 
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/src/osnoise.c       |  79 ++++++++++++++++
>  tools/tracing/rtla/src/osnoise.h       |   1 +
>  tools/tracing/rtla/src/osnoise_hist.c  |  66 +-------------
>  tools/tracing/rtla/src/osnoise_top.c   |  66 +-------------
>  tools/tracing/rtla/src/timerlat.c      | 109 ++++++++++++++++++++++
>  tools/tracing/rtla/src/timerlat.h      |  11 +--
>  tools/tracing/rtla/src/timerlat_hist.c | 119 ++++---------------------
>  tools/tracing/rtla/src/timerlat_top.c  | 110 +++--------------------
>  8 files changed, 227 insertions(+), 334 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
> index 93d485c0e949..1735a36466c4 100644
> --- a/tools/tracing/rtla/src/osnoise.c
> +++ b/tools/tracing/rtla/src/osnoise.c
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2021 Red Hat Inc, Daniel Bristot de Oliveira <bristot@kernel.org>
>   */
>  
> +#define _GNU_SOURCE
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <pthread.h>
> @@ -12,6 +13,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <stdio.h>
> +#include <sched.h>
>  
>  #include "osnoise.h"
>  
> @@ -1114,6 +1116,83 @@ osnoise_report_missed_events(struct osnoise_tool *tool)
>  	}
>  }
>  
> +/*
> + * osnoise_apply_config - apply common configs to the initialized tool
> + */
> +int
> +osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
> +{
> +	int retval;
> +
> +	if (!params->sleep_time)
> +		params->sleep_time = 1;
> +
> +	if (params->cpus) {
> +		retval = osnoise_set_cpus(tool->context, params->cpus);
> +		if (retval) {
> +			err_msg("Failed to apply CPUs config\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->runtime || params->period) {
> +		retval = osnoise_set_runtime_period(tool->context,
> +						    params->runtime,
> +						    params->period);
> +		if (retval) {
> +			err_msg("Failed to set runtime and/or period\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->stop_us) {
> +		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> +		if (retval) {
> +			err_msg("Failed to set stop us\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->stop_total_us) {
> +		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> +		if (retval) {
> +			err_msg("Failed to set stop total us\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->threshold) {
> +		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
> +		if (retval) {
> +			err_msg("Failed to set tracing_thresh\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->hk_cpus) {
> +		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
> +					   &params->hk_cpu_set);
> +		if (retval == -1) {
> +			err_msg("Failed to set rtla to the house keeping CPUs\n");
> +			goto out_err;
> +		}
> +	} else if (params->cpus) {
> +		/*
> +		 * Even if the user do not set a house-keeping CPU, try to
> +		 * move rtla to a CPU set different to the one where the user
> +		 * set the workload to run.
> +		 *
> +		 * No need to check results as this is an automatic attempt.
> +		 */
> +		auto_house_keeping(&params->monitored_cpus);
> +	}
> +
> +	return 0;
> +
> +out_err:
> +	return -1;
> +}
> +
>  static void osnoise_usage(int err)
>  {
>  	int i;
> diff --git a/tools/tracing/rtla/src/osnoise.h b/tools/tracing/rtla/src/osnoise.h
> index f78ffbdc8c8d..ac1c99910744 100644
> --- a/tools/tracing/rtla/src/osnoise.h
> +++ b/tools/tracing/rtla/src/osnoise.h
> @@ -155,6 +155,7 @@ struct osnoise_tool *osnoise_init_tool(char *tool_name);
>  struct osnoise_tool *osnoise_init_trace_tool(char *tracer);
>  void osnoise_report_missed_events(struct osnoise_tool *tool);
>  bool osnoise_trace_is_off(struct osnoise_tool *tool, struct osnoise_tool *record);
> +int osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params);
>  
>  int osnoise_hist_main(int argc, char *argv[]);
>  int osnoise_top_main(int argc, char **argv);
> diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
> index 4721f15f77cd..d9d15c8f27c7 100644
> --- a/tools/tracing/rtla/src/osnoise_hist.c
> +++ b/tools/tracing/rtla/src/osnoise_hist.c
> @@ -12,7 +12,6 @@
>  #include <errno.h>
>  #include <stdio.h>
>  #include <time.h>
> -#include <sched.h>
>  
>  #include "osnoise.h"
>  
> @@ -705,68 +704,9 @@ osnoise_hist_apply_config(struct osnoise_tool *tool, struct osnoise_params *para
>  {
>  	int retval;
>  
> -	if (!params->sleep_time)
> -		params->sleep_time = 1;
> -
> -	if (params->cpus) {
> -		retval = osnoise_set_cpus(tool->context, params->cpus);
> -		if (retval) {
> -			err_msg("Failed to apply CPUs config\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->runtime || params->period) {
> -		retval = osnoise_set_runtime_period(tool->context,
> -						    params->runtime,
> -						    params->period);
> -		if (retval) {
> -			err_msg("Failed to set runtime and/or period\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->stop_us) {
> -		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> -		if (retval) {
> -			err_msg("Failed to set stop us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->stop_total_us) {
> -		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> -		if (retval) {
> -			err_msg("Failed to set stop total us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->threshold) {
> -		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
> -		if (retval) {
> -			err_msg("Failed to set tracing_thresh\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->hk_cpus) {
> -		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
> -					   &params->hk_cpu_set);
> -		if (retval == -1) {
> -			err_msg("Failed to set rtla to the house keeping CPUs\n");
> -			goto out_err;
> -		}
> -	} else if (params->cpus) {
> -		/*
> -		 * Even if the user do not set a house-keeping CPU, try to
> -		 * move rtla to a CPU set different to the one where the user
> -		 * set the workload to run.
> -		 *
> -		 * No need to check results as this is an automatic attempt.
> -		 */
> -		auto_house_keeping(&params->monitored_cpus);
> -	}
> +	retval = osnoise_apply_config(tool, params);
> +	if (retval)
> +		goto out_err;
>  
>  	return 0;
>  
> diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
> index 7f393019bbf5..3455ee73e2e6 100644
> --- a/tools/tracing/rtla/src/osnoise_top.c
> +++ b/tools/tracing/rtla/src/osnoise_top.c
> @@ -11,7 +11,6 @@
>  #include <unistd.h>
>  #include <stdio.h>
>  #include <time.h>
> -#include <sched.h>
>  
>  #include "osnoise.h"
>  
> @@ -523,50 +522,9 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_params *param
>  {
>  	int retval;
>  
> -	if (!params->sleep_time)
> -		params->sleep_time = 1;
> -
> -	if (params->cpus) {
> -		retval = osnoise_set_cpus(tool->context, params->cpus);
> -		if (retval) {
> -			err_msg("Failed to apply CPUs config\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->runtime || params->period) {
> -		retval = osnoise_set_runtime_period(tool->context,
> -						    params->runtime,
> -						    params->period);
> -		if (retval) {
> -			err_msg("Failed to set runtime and/or period\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->stop_us) {
> -		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> -		if (retval) {
> -			err_msg("Failed to set stop us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->stop_total_us) {
> -		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> -		if (retval) {
> -			err_msg("Failed to set stop total us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->threshold) {
> -		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
> -		if (retval) {
> -			err_msg("Failed to set tracing_thresh\n");
> -			goto out_err;
> -		}
> -	}
> +	retval = osnoise_apply_config(tool, params);
> +	if (retval)
> +		goto out_err;
>  
>  	if (params->mode == MODE_HWNOISE) {
>  		retval = osnoise_set_irq_disable(tool->context, 1);
> @@ -576,24 +534,6 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_params *param
>  		}
>  	}
>  
> -	if (params->hk_cpus) {
> -		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
> -					   &params->hk_cpu_set);
> -		if (retval == -1) {
> -			err_msg("Failed to set rtla to the house keeping CPUs\n");
> -			goto out_err;
> -		}
> -	} else if (params->cpus) {
> -		/*
> -		 * Even if the user do not set a house-keeping CPU, try to
> -		 * move rtla to a CPU set different to the one where the user
> -		 * set the workload to run.
> -		 *
> -		 * No need to check results as this is an automatic attempt.
> -		 */
> -		auto_house_keeping(&params->monitored_cpus);
> -	}
> -
>  	if (isatty(STDOUT_FILENO) && !params->quiet)
>  		params->pretty_output = 1;
>  
> diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
> index 21cdcc5c4a29..448fb1f7d3a6 100644
> --- a/tools/tracing/rtla/src/timerlat.c
> +++ b/tools/tracing/rtla/src/timerlat.c
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright (C) 2021 Red Hat Inc, Daniel Bristot de Oliveira <bristot@kernel.org>
>   */
> +#define _GNU_SOURCE
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <pthread.h>
> @@ -11,9 +12,117 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <stdio.h>
> +#include <sched.h>
>  
>  #include "timerlat.h"
>  
> +/*
> + * timerlat_apply_config - apply common configs to the initialized tool
> + */
> +int
> +timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
> +{
> +	int retval, i;
> +
> +	if (!params->sleep_time)
> +		params->sleep_time = 1;
> +
> +	if (params->cpus) {
> +		retval = osnoise_set_cpus(tool->context, params->cpus);
> +		if (retval) {
> +			err_msg("Failed to apply CPUs config\n");
> +			goto out_err;
> +		}
> +	} else {
> +		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
> +			CPU_SET(i, &params->monitored_cpus);
> +	}
> +
> +	if (params->stop_us) {
> +		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> +		if (retval) {
> +			err_msg("Failed to set stop us\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->stop_total_us) {
> +		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> +		if (retval) {
> +			err_msg("Failed to set stop total us\n");
> +			goto out_err;
> +		}
> +	}
> +
> +
> +	if (params->timerlat_period_us) {
> +		retval = osnoise_set_timerlat_period_us(tool->context, params->timerlat_period_us);
> +		if (retval) {
> +			err_msg("Failed to set timerlat period\n");
> +			goto out_err;
> +		}
> +	}
> +
> +
> +	if (params->print_stack) {
> +		retval = osnoise_set_print_stack(tool->context, params->print_stack);
> +		if (retval) {
> +			err_msg("Failed to set print stack\n");
> +			goto out_err;
> +		}
> +	}
> +
> +	if (params->hk_cpus) {
> +		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
> +					   &params->hk_cpu_set);
> +		if (retval == -1) {
> +			err_msg("Failed to set rtla to the house keeping CPUs\n");
> +			goto out_err;
> +		}
> +	} else if (params->cpus) {
> +		/*
> +		 * Even if the user do not set a house-keeping CPU, try to
> +		 * move rtla to a CPU set different to the one where the user
> +		 * set the workload to run.
> +		 *
> +		 * No need to check results as this is an automatic attempt.
> +		 */
> +		auto_house_keeping(&params->monitored_cpus);
> +	}
> +
> +	/*
> +	 * If the user did not specify a type of thread, try user-threads first.
> +	 * Fall back to kernel threads otherwise.
> +	 */
> +	if (!params->kernel_workload && !params->user_data) {
> +		retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd");
> +		if (retval) {
> +			debug_msg("User-space interface detected, setting user-threads\n");
> +			params->user_workload = 1;
> +			params->user_data = 1;
> +		} else {
> +			debug_msg("User-space interface not detected, setting kernel-threads\n");
> +			params->kernel_workload = 1;
> +		}
> +	}
> +
> +	/*
> +	 * Set workload according to type of thread if the kernel supports it.
> +	 * On kernels without support, user threads will have already failed
> +	 * on missing timerlat_fd, and kernel threads do not need it.
> +	 */
> +	retval = osnoise_set_workload(tool->context, params->kernel_workload);
> +	if (retval < -1) {
> +		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
> +		goto out_err;
> +	}
> +
> +	return 0;
> +
> +out_err:
> +	return -1;
> +}
> +
>  static void timerlat_usage(int err)
>  {
>  	int i;
> diff --git a/tools/tracing/rtla/src/timerlat.h b/tools/tracing/rtla/src/timerlat.h
> index cadc613dc82e..73045aef23fa 100644
> --- a/tools/tracing/rtla/src/timerlat.h
> +++ b/tools/tracing/rtla/src/timerlat.h
> @@ -15,17 +15,15 @@ struct timerlat_params {
>  	int			sleep_time;
>  	int			output_divisor;
>  	int			duration;
> -	int			quiet;
>  	int			set_sched;
>  	int			dma_latency;
>  	int			no_aa;
> -	int			aa_only;
>  	int			dump_tasks;
>  	int			cgroup;
>  	int			hk_cpus;
>  	int			user_workload;
>  	int			kernel_workload;
> -	int			pretty_output;
> +	int			user_data;
>  	int			warmup;
>  	int			buffer_size;
>  	int			deepest_idle_state;
> @@ -35,11 +33,12 @@ struct timerlat_params {
>  	union {
>  		struct {
>  			/* top only */
> -			int			user_top;
> +			int			quiet;
> +			int			aa_only;
> +			int			pretty_output;
>  		};
>  		struct {
>  			/* hist only */
> -			int			user_hist;
>  			char			no_irq;
>  			char			no_thread;
>  			char			no_header;
> @@ -52,6 +51,8 @@ struct timerlat_params {
>  	};
>  };
>  
> +int timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params);
> +
>  int timerlat_hist_main(int argc, char *argv[]);
>  int timerlat_top_main(int argc, char *argv[]);
>  int timerlat_main(int argc, char *argv[]);
> diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
> index 822c068b4776..9d9efeedc4c2 100644
> --- a/tools/tracing/rtla/src/timerlat_hist.c
> +++ b/tools/tracing/rtla/src/timerlat_hist.c
> @@ -315,7 +315,7 @@ static void timerlat_hist_header(struct osnoise_tool *tool)
>  		if (!params->no_thread)
>  			trace_seq_printf(s, "   Thr-%03d", cpu);
>  
> -		if (params->user_hist)
> +		if (params->user_data)
>  			trace_seq_printf(s, "   Usr-%03d", cpu);
>  	}
>  	trace_seq_printf(s, "\n");
> @@ -371,7 +371,7 @@ timerlat_print_summary(struct timerlat_params *params,
>  			trace_seq_printf(trace->seq, "%9llu ",
>  					data->hist[cpu].thread_count);
>  
> -		if (params->user_hist)
> +		if (params->user_data)
>  			trace_seq_printf(trace->seq, "%9llu ",
>  					 data->hist[cpu].user_count);
>  	}
> @@ -399,7 +399,7 @@ timerlat_print_summary(struct timerlat_params *params,
>  					     data->hist[cpu].min_thread,
>  					     false);
>  
> -		if (params->user_hist)
> +		if (params->user_data)
>  			format_summary_value(trace->seq,
>  					     data->hist[cpu].user_count,
>  					     data->hist[cpu].min_user,
> @@ -429,7 +429,7 @@ timerlat_print_summary(struct timerlat_params *params,
>  					     data->hist[cpu].sum_thread,
>  					     true);
>  
> -		if (params->user_hist)
> +		if (params->user_data)
>  			format_summary_value(trace->seq,
>  					     data->hist[cpu].user_count,
>  					     data->hist[cpu].sum_user,
> @@ -459,7 +459,7 @@ timerlat_print_summary(struct timerlat_params *params,
>  					     data->hist[cpu].max_thread,
>  					     false);
>  
> -		if (params->user_hist)
> +		if (params->user_data)
>  			format_summary_value(trace->seq,
>  					     data->hist[cpu].user_count,
>  					     data->hist[cpu].max_user,
> @@ -521,7 +521,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
>  	if (!params->no_thread)
>  		trace_seq_printf(trace->seq, "       Thr");
>  
> -	if (params->user_hist)
> +	if (params->user_data)
>  		trace_seq_printf(trace->seq, "       Usr");
>  
>  	trace_seq_printf(trace->seq, "\n");
> @@ -537,7 +537,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
>  		trace_seq_printf(trace->seq, "%9llu ",
>  				 sum.thread_count);
>  
> -	if (params->user_hist)
> +	if (params->user_data)
>  		trace_seq_printf(trace->seq, "%9llu ",
>  				 sum.user_count);
>  
> @@ -558,7 +558,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
>  				     sum.min_thread,
>  				     false);
>  
> -	if (params->user_hist)
> +	if (params->user_data)
>  		format_summary_value(trace->seq,
>  				     sum.user_count,
>  				     sum.min_user,
> @@ -581,7 +581,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
>  				     sum.sum_thread,
>  				     true);
>  
> -	if (params->user_hist)
> +	if (params->user_data)
>  		format_summary_value(trace->seq,
>  				     sum.user_count,
>  				     sum.sum_user,
> @@ -604,7 +604,7 @@ timerlat_print_stats_all(struct timerlat_params *params,
>  				     sum.max_thread,
>  				     false);
>  
> -	if (params->user_hist)
> +	if (params->user_data)
>  		format_summary_value(trace->seq,
>  				     sum.user_count,
>  				     sum.max_user,
> @@ -654,7 +654,7 @@ timerlat_print_stats(struct timerlat_params *params, struct osnoise_tool *tool)
>  						data->hist[cpu].thread[bucket]);
>  			}
>  
> -			if (params->user_hist) {
> +			if (params->user_data) {
>  				total += data->hist[cpu].user[bucket];
>  				trace_seq_printf(trace->seq, "%9d ",
>  						data->hist[cpu].user[bucket]);
> @@ -690,7 +690,7 @@ timerlat_print_stats(struct timerlat_params *params, struct osnoise_tool *tool)
>  			trace_seq_printf(trace->seq, "%9d ",
>  					 data->hist[cpu].thread[data->entries]);
>  
> -		if (params->user_hist)
> +		if (params->user_data)
>  			trace_seq_printf(trace->seq, "%9d ",
>  					 data->hist[cpu].user[data->entries]);
>  	}
> @@ -965,7 +965,7 @@ static struct timerlat_params
>  			params->user_workload = 1;
>  			/* fallback: -u implies in -U */
>  		case 'U':
> -			params->user_hist = 1;
> +			params->user_data = 1;
>  			break;
>  		case '0': /* no irq */
>  			params->no_irq = 1;
> @@ -1063,98 +1063,11 @@ static struct timerlat_params
>  static int
>  timerlat_hist_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
>  {
> -	int retval, i;
> -
> -	if (!params->sleep_time)
> -		params->sleep_time = 1;
> -
> -	if (params->cpus) {
> -		retval = osnoise_set_cpus(tool->context, params->cpus);
> -		if (retval) {
> -			err_msg("Failed to apply CPUs config\n");
> -			goto out_err;
> -		}
> -	} else {
> -		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
> -			CPU_SET(i, &params->monitored_cpus);
> -	}
> -
> -	if (params->stop_us) {
> -		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> -		if (retval) {
> -			err_msg("Failed to set stop us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->stop_total_us) {
> -		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> -		if (retval) {
> -			err_msg("Failed to set stop total us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->timerlat_period_us) {
> -		retval = osnoise_set_timerlat_period_us(tool->context, params->timerlat_period_us);
> -		if (retval) {
> -			err_msg("Failed to set timerlat period\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->print_stack) {
> -		retval = osnoise_set_print_stack(tool->context, params->print_stack);
> -		if (retval) {
> -			err_msg("Failed to set print stack\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->hk_cpus) {
> -		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
> -					   &params->hk_cpu_set);
> -		if (retval == -1) {
> -			err_msg("Failed to set rtla to the house keeping CPUs\n");
> -			goto out_err;
> -		}
> -	} else if (params->cpus) {
> -		/*
> -		 * Even if the user do not set a house-keeping CPU, try to
> -		 * move rtla to a CPU set different to the one where the user
> -		 * set the workload to run.
> -		 *
> -		 * No need to check results as this is an automatic attempt.
> -		 */
> -		auto_house_keeping(&params->monitored_cpus);
> -	}
> -
> -	/*
> -	 * If the user did not specify a type of thread, try user-threads first.
> -	 * Fall back to kernel threads otherwise.
> -	 */
> -	if (!params->kernel_workload && !params->user_hist) {
> -		retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd");
> -		if (retval) {
> -			debug_msg("User-space interface detected, setting user-threads\n");
> -			params->user_workload = 1;
> -			params->user_hist = 1;
> -		} else {
> -			debug_msg("User-space interface not detected, setting kernel-threads\n");
> -			params->kernel_workload = 1;
> -		}
> -	}
> +	int retval;
>  
> -	/*
> -	* Set workload according to type of thread if the kernel supports it.
> -	* On kernels without support, user threads will have already failed
> -	* on missing timerlat_fd, and kernel threads do not need it.
> -	*/
> -	retval = osnoise_set_workload(tool->context, params->kernel_workload);
> -	if (retval < -1) {
> -		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
> +	retval = timerlat_apply_config(tool, params);
> +	if (retval)
>  		goto out_err;
> -	}
>  
>  	return 0;
>  
> diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
> index c3196a0bb585..79cb6f28967f 100644
> --- a/tools/tracing/rtla/src/timerlat_top.c
> +++ b/tools/tracing/rtla/src/timerlat_top.c
> @@ -266,7 +266,7 @@ static void timerlat_top_header(struct timerlat_params *params, struct osnoise_t
>  		trace_seq_printf(s, "\033[2;37;40m");
>  
>  	trace_seq_printf(s, "                                     Timer Latency                                              ");
> -	if (params->user_top)
> +	if (params->user_data)
>  		trace_seq_printf(s, "                                         ");
>  
>  	if (params->pretty_output)
> @@ -277,7 +277,7 @@ static void timerlat_top_header(struct timerlat_params *params, struct osnoise_t
>  			params->output_divisor == 1 ? "ns" : "us",
>  			params->output_divisor == 1 ? "ns" : "us");
>  
> -	if (params->user_top) {
> +	if (params->user_data) {
>  		trace_seq_printf(s, "      |    Ret user Timer Latency (%s)",
>  				params->output_divisor == 1 ? "ns" : "us");
>  	}
> @@ -287,7 +287,7 @@ static void timerlat_top_header(struct timerlat_params *params, struct osnoise_t
>  		trace_seq_printf(s, "\033[2;30;47m");
>  
>  	trace_seq_printf(s, "CPU COUNT      |      cur       min       avg       max |      cur       min       avg       max");
> -	if (params->user_top)
> +	if (params->user_data)
>  		trace_seq_printf(s, " |      cur       min       avg       max");
>  
>  	if (params->pretty_output)
> @@ -338,7 +338,7 @@ static void timerlat_top_print(struct osnoise_tool *top, int cpu)
>  		trace_seq_printf(s, "%9llu", cpu_data->max_thread);
>  	}
>  
> -	if (!params->user_top) {
> +	if (!params->user_data) {
>  		trace_seq_printf(s, "\n");
>  		return;
>  	}
> @@ -380,7 +380,7 @@ timerlat_top_print_sum(struct osnoise_tool *top, struct timerlat_top_cpu *summar
>  	}
>  
>  	trace_seq_printf(s, "%.*s|%.*s|%.*s", 15, split, 40, split, 39, split);
> -	if (params->user_top)
> +	if (params->user_data)
>  		trace_seq_printf(s, "-|%.*s", 39, split);
>  	trace_seq_printf(s, "\n");
>  
> @@ -405,7 +405,7 @@ timerlat_top_print_sum(struct osnoise_tool *top, struct timerlat_top_cpu *summar
>  		trace_seq_printf(s, "%9llu", summary->max_thread);
>  	}
>  
> -	if (!params->user_top) {
> +	if (!params->user_data) {
>  		trace_seq_printf(s, "\n");
>  		return;
>  	}
> @@ -722,7 +722,7 @@ static struct timerlat_params
>  			params->user_workload = true;
>  			/* fallback: -u implies -U */
>  		case 'U':
> -			params->user_top = true;
> +			params->user_data = true;
>  			break;
>  		case '0': /* trigger */
>  			if (params->events) {
> @@ -800,100 +800,10 @@ static int
>  timerlat_top_apply_config(struct osnoise_tool *top, struct timerlat_params *params)
>  {
>  	int retval;
> -	int i;
> -
> -	if (!params->sleep_time)
> -		params->sleep_time = 1;
> -
> -	if (params->cpus) {
> -		retval = osnoise_set_cpus(top->context, params->cpus);
> -		if (retval) {
> -			err_msg("Failed to apply CPUs config\n");
> -			goto out_err;
> -		}
> -	} else {
> -		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
> -			CPU_SET(i, &params->monitored_cpus);
> -	}
> -
> -	if (params->stop_us) {
> -		retval = osnoise_set_stop_us(top->context, params->stop_us);
> -		if (retval) {
> -			err_msg("Failed to set stop us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->stop_total_us) {
> -		retval = osnoise_set_stop_total_us(top->context, params->stop_total_us);
> -		if (retval) {
> -			err_msg("Failed to set stop total us\n");
> -			goto out_err;
> -		}
> -	}
> -
> -
> -	if (params->timerlat_period_us) {
> -		retval = osnoise_set_timerlat_period_us(top->context, params->timerlat_period_us);
> -		if (retval) {
> -			err_msg("Failed to set timerlat period\n");
> -			goto out_err;
> -		}
> -	}
>  
> -
> -	if (params->print_stack) {
> -		retval = osnoise_set_print_stack(top->context, params->print_stack);
> -		if (retval) {
> -			err_msg("Failed to set print stack\n");
> -			goto out_err;
> -		}
> -	}
> -
> -	if (params->hk_cpus) {
> -		retval = sched_setaffinity(getpid(), sizeof(params->hk_cpu_set),
> -					   &params->hk_cpu_set);
> -		if (retval == -1) {
> -			err_msg("Failed to set rtla to the house keeping CPUs\n");
> -			goto out_err;
> -		}
> -	} else if (params->cpus) {
> -		/*
> -		 * Even if the user do not set a house-keeping CPU, try to
> -		 * move rtla to a CPU set different to the one where the user
> -		 * set the workload to run.
> -		 *
> -		 * No need to check results as this is an automatic attempt.
> -		 */
> -		auto_house_keeping(&params->monitored_cpus);
> -	}
> -
> -	/*
> -	 * If the user did not specify a type of thread, try user-threads first.
> -	 * Fall back to kernel threads otherwise.
> -	 */
> -	if (!params->kernel_workload && !params->user_top) {
> -		retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd");
> -		if (retval) {
> -			debug_msg("User-space interface detected, setting user-threads\n");
> -			params->user_workload = 1;
> -			params->user_top = 1;
> -		} else {
> -			debug_msg("User-space interface not detected, setting kernel-threads\n");
> -			params->kernel_workload = 1;
> -		}
> -	}
> -
> -	/*
> -	* Set workload according to type of thread if the kernel supports it.
> -	* On kernels without support, user threads will have already failed
> -	* on missing timerlat_fd, and kernel threads do not need it.
> -	*/
> -	retval = osnoise_set_workload(top->context, params->kernel_workload);
> -	if (retval < -1) {
> -		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
> +	retval = timerlat_apply_config(top, params);
> +	if (retval)
>  		goto out_err;
> -	}
>  
>  	if (isatty(STDOUT_FILENO) && !params->quiet)
>  		params->pretty_output = 1;
> @@ -1142,7 +1052,7 @@ int timerlat_top_main(int argc, char *argv[])
>  		}
>  	}
>  
> -	if (params->cgroup && !params->user_top) {
> +	if (params->cgroup && !params->user_data) {
>  		retval = set_comm_cgroup("timerlat/", params->cgroup_name);
>  		if (!retval) {
>  			err_msg("Failed to move threads to cgroup\n");
> -- 
> 2.48.1
> 
> 
> 
Reviewed-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/6] rtla/osnoise: Set OSNOISE_WORKLOAD to true
  2025-03-20  9:24 ` [PATCH 3/6] rtla/osnoise: Set OSNOISE_WORKLOAD to true Tomas Glozar
@ 2025-03-20 19:40   ` John Kacur
  0 siblings, 0 replies; 14+ messages in thread
From: John Kacur @ 2025-03-20 19:40 UTC (permalink / raw)
  To: Tomas Glozar
  Cc: Steven Rostedt, linux-trace-kernel, linux-kernel, Luis Goncalves



On Thu, 20 Mar 2025, Tomas Glozar wrote:

> If running rtla osnoise with NO_OSNOISE_WORKLOAD, it reports no samples:
> 
> $ echo NO_OSNOISE_WORKLOAD > /sys/kernel/tracing/osnoise/options
> $ rtla osnoise hist -d 10s
> Index
> over: 0
> count: 0
> min: 0
> avg: 0
> max: 0
> 
> This situation can also happen when running rtla-osnoise after an
> improperly exited rtla-timerlat run.
> 
> Set OSNOISE_WORKLOAD in rtla-osnoise, too, similarly to what we
> already did for timerlat in commit 217f0b1e990e ("rtla/timerlat_top: Set
> OSNOISE_WORKLOAD for kernel threads") and commit d8d866171a41
> ("rtla/timerlat_hist: Set OSNOISE_WORKLOAD for kernel threads").
> 
> Note that there is no user workload mode for rtla-osnoise yet, so
> OSNOISE_WORKLOAD is always set to true.
> 
> Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
> Fixes: 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/src/osnoise.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
> index 1735a36466c4..a71618d876e9 100644
> --- a/tools/tracing/rtla/src/osnoise.c
> +++ b/tools/tracing/rtla/src/osnoise.c
> @@ -1187,6 +1187,12 @@ osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
>  		auto_house_keeping(&params->monitored_cpus);
>  	}
>  
> +	retval = osnoise_set_workload(tool->context, true);
> +	if (retval < -1) {
> +		err_msg("Failed to set OSNOISE_WORKLOAD option\n");
> +		goto out_err;
> +	}
> +
>  	return 0;
>  
>  out_err:
> -- 
> 2.48.1
> 
> 
> 
Reviewed-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/6] rtla: Always set all tracer options
  2025-03-20  9:24 ` [PATCH 4/6] rtla: Always set all tracer options Tomas Glozar
@ 2025-03-20 20:31   ` John Kacur
  0 siblings, 0 replies; 14+ messages in thread
From: John Kacur @ 2025-03-20 20:31 UTC (permalink / raw)
  To: Tomas Glozar
  Cc: Steven Rostedt, linux-trace-kernel, linux-kernel, Luis Goncalves



On Thu, 20 Mar 2025, Tomas Glozar wrote:

> rtla currently only sets tracer options that are explicitly set by the
> user, with the exception of OSNOISE_WORKLOAD.
> 
> This leads to improper behavior in case rtla is run with those options
> not set to the default value. rtla does reset them to the original
> value upon exiting, but that does not protect it from starting with
> non-default values set either by an improperly exited rtla or by another
> user of the tracers.
> 
> For example, after running this command:
> 
> $ echo 1 > /sys/kernel/tracing/osnoise/stop_tracing_us
> 
> all runs of rtla will stop at the 1us threshold, even if not requested
> by the user:
> 
> $ rtla osnoise hist
> Index   CPU-000   CPU-001
> 1             8         5
> 2             5         9
> 3             1         2
> 4             6         1
> 5             2         1
> 6             0         1
> 8             1         1
> 12            0         1
> 14            1         0
> 15            1         0
> over:         0         0
> count:       25        21
> min:          1         1
> avg:       3.68      3.05
> max:         15        12
> rtla osnoise hit stop tracing
> 
> Fix the problem by setting the default value for all tracer options if
> the user has not provided their own value.
> 
> For most of the options, it's enough to just drop the if clause checking
> for the value being set. For cpus, "all" is used as the default value,
> and for osnoise default period and runtime, default values of
> the osnoise_data variable in trace_osnoise.c are used.
> 
> Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
> Fixes: 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
> Fixes: a828cd18bc4a ("rtla: Add timerlat tool and timelart top mode")
> Fixes: 1eeb6328e8b3 ("rtla/timerlat: Add timerlat hist mode")
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/src/osnoise.c  | 56 ++++++++++++++---------------
>  tools/tracing/rtla/src/timerlat.c | 59 +++++++++++++++----------------
>  2 files changed, 56 insertions(+), 59 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
> index a71618d876e9..2dc3e4539e99 100644
> --- a/tools/tracing/rtla/src/osnoise.c
> +++ b/tools/tracing/rtla/src/osnoise.c
> @@ -17,6 +17,9 @@
>  
>  #include "osnoise.h"
>  
> +#define DEFAULT_SAMPLE_PERIOD	1000000			/* 1s */
> +#define DEFAULT_SAMPLE_RUNTIME	1000000			/* 1s */
> +
>  /*
>   * osnoise_get_cpus - return the original "osnoise/cpus" content
>   *
> @@ -1127,46 +1130,43 @@ osnoise_apply_config(struct osnoise_tool *tool, struct osnoise_params *params)
>  	if (!params->sleep_time)
>  		params->sleep_time = 1;
>  
> -	if (params->cpus) {
> -		retval = osnoise_set_cpus(tool->context, params->cpus);
> -		if (retval) {
> -			err_msg("Failed to apply CPUs config\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_cpus(tool->context, params->cpus ? params->cpus : "all");
> +	if (retval) {
> +		err_msg("Failed to apply CPUs config\n");
> +		goto out_err;
>  	}
>  
>  	if (params->runtime || params->period) {
>  		retval = osnoise_set_runtime_period(tool->context,
>  						    params->runtime,
>  						    params->period);
> -		if (retval) {
> -			err_msg("Failed to set runtime and/or period\n");
> -			goto out_err;
> -		}
> +	} else {
> +		retval = osnoise_set_runtime_period(tool->context,
> +						    DEFAULT_SAMPLE_PERIOD,
> +						    DEFAULT_SAMPLE_RUNTIME);
>  	}
>  
> -	if (params->stop_us) {
> -		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> -		if (retval) {
> -			err_msg("Failed to set stop us\n");
> -			goto out_err;
> -		}
> +	if (retval) {
> +		err_msg("Failed to set runtime and/or period\n");
> +		goto out_err;
>  	}
>  
> -	if (params->stop_total_us) {
> -		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> -		if (retval) {
> -			err_msg("Failed to set stop total us\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_stop_us(tool->context, params->stop_us);
> +	if (retval) {
> +		err_msg("Failed to set stop us\n");
> +		goto out_err;
>  	}
>  
> -	if (params->threshold) {
> -		retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
> -		if (retval) {
> -			err_msg("Failed to set tracing_thresh\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> +	if (retval) {
> +		err_msg("Failed to set stop total us\n");
> +		goto out_err;
> +	}
> +
> +	retval = osnoise_set_tracing_thresh(tool->context, params->threshold);
> +	if (retval) {
> +		err_msg("Failed to set tracing_thresh\n");
> +		goto out_err;
>  	}
>  
>  	if (params->hk_cpus) {
> diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
> index 448fb1f7d3a6..c29e2ba2d7d8 100644
> --- a/tools/tracing/rtla/src/timerlat.c
> +++ b/tools/tracing/rtla/src/timerlat.c
> @@ -16,6 +16,8 @@
>  
>  #include "timerlat.h"
>  
> +#define DEFAULT_TIMERLAT_PERIOD	1000			/* 1ms */
> +
>  /*
>   * timerlat_apply_config - apply common configs to the initialized tool
>   */
> @@ -27,49 +29,44 @@ timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
>  	if (!params->sleep_time)
>  		params->sleep_time = 1;
>  
> -	if (params->cpus) {
> -		retval = osnoise_set_cpus(tool->context, params->cpus);
> -		if (retval) {
> -			err_msg("Failed to apply CPUs config\n");
> -			goto out_err;
> -		}
> -	} else {
> +	retval = osnoise_set_cpus(tool->context, params->cpus ? params->cpus : "all");
> +	if (retval) {
> +		err_msg("Failed to apply CPUs config\n");
> +		goto out_err;
> +	}
> +
> +	if (!params->cpus) {
>  		for (i = 0; i < sysconf(_SC_NPROCESSORS_CONF); i++)
>  			CPU_SET(i, &params->monitored_cpus);
>  	}
>  
> -	if (params->stop_us) {
> -		retval = osnoise_set_stop_us(tool->context, params->stop_us);
> -		if (retval) {
> -			err_msg("Failed to set stop us\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_stop_us(tool->context, params->stop_us);
> +	if (retval) {
> +		err_msg("Failed to set stop us\n");
> +		goto out_err;
>  	}
>  
> -	if (params->stop_total_us) {
> -		retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> -		if (retval) {
> -			err_msg("Failed to set stop total us\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_stop_total_us(tool->context, params->stop_total_us);
> +	if (retval) {
> +		err_msg("Failed to set stop total us\n");
> +		goto out_err;
>  	}
>  
>  
> -	if (params->timerlat_period_us) {
> -		retval = osnoise_set_timerlat_period_us(tool->context, params->timerlat_period_us);
> -		if (retval) {
> -			err_msg("Failed to set timerlat period\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_timerlat_period_us(tool->context,
> +						params->timerlat_period_us ?
> +						params->timerlat_period_us :
> +						DEFAULT_TIMERLAT_PERIOD);
> +	if (retval) {
> +		err_msg("Failed to set timerlat period\n");
> +		goto out_err;
>  	}
>  
>  
> -	if (params->print_stack) {
> -		retval = osnoise_set_print_stack(tool->context, params->print_stack);
> -		if (retval) {
> -			err_msg("Failed to set print stack\n");
> -			goto out_err;
> -		}
> +	retval = osnoise_set_print_stack(tool->context, params->print_stack);
> +	if (retval) {
> +		err_msg("Failed to set print stack\n");
> +		goto out_err;
>  	}
>  
>  	if (params->hk_cpus) {
> -- 
> 2.48.1
> 
> 
> 
Reviewed-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/6] rtla/tests: Reset osnoise options before check
  2025-03-20  9:24 ` [PATCH 5/6] rtla/tests: Reset osnoise options before check Tomas Glozar
@ 2025-03-20 20:36   ` John Kacur
  0 siblings, 0 replies; 14+ messages in thread
From: John Kacur @ 2025-03-20 20:36 UTC (permalink / raw)
  To: Tomas Glozar
  Cc: Steven Rostedt, linux-trace-kernel, linux-kernel, Luis Goncalves



On Thu, 20 Mar 2025, Tomas Glozar wrote:

> Remove any dangling tracing instances from previous improperly exited
> runs of rtla, and reset osnoise options to default before running a test
> case.
> 
> This ensures that the test results are deterministic. Specific test
> cases checked that rtla behaves correctly even when the tracer state is
> not clean will be added later.
> 
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/tests/engine.sh | 40 ++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/tools/tracing/rtla/tests/engine.sh b/tools/tracing/rtla/tests/engine.sh
> index 64d0446dc28e..5db8aa4bc031 100644
> --- a/tools/tracing/rtla/tests/engine.sh
> +++ b/tools/tracing/rtla/tests/engine.sh
> @@ -8,12 +8,44 @@ test_begin() {
>  	[ -n "$TEST_COUNT" ] && echo "1..$TEST_COUNT"
>  }
>  
> +reset_osnoise() {
> +	# Reset osnoise options to default and remove any dangling instances created
> +	# by improperly exited rtla runs.
> +	pushd /sys/kernel/tracing || return 1
> +
> +	# Remove dangling instances created by previous rtla run
> +	echo 0 > tracing_thresh
> +	cd instances
> +	for i in osnoise_top osnoise_hist timerlat_top timerlat_hist
> +	do
> +		[ ! -d "$i" ] && continue
> +		rmdir "$i"
> +	done
> +
> +	# Reset options to default
> +	# Note: those are copied from the default values of osnoise_data
> +	# in kernel/trace/trace_osnoise.c
> +	cd ../osnoise
> +	echo all > cpus
> +	echo DEFAULTS > options
> +	echo 1000000 > period_us
> +	echo 0 > print_stack
> +	echo 1000000 > runtime_us
> +	echo 0 > stop_tracing_total_us
> +	echo 0 > stop_tracing_us
> +	echo 1000 > timerlat_period_us
> +
> +	popd
> +}
> +
>  check() {
>  	# Simple check: run rtla with given arguments and test exit code.
>  	# If TEST_COUNT is set, run the test. Otherwise, just count.
>  	ctr=$(($ctr + 1))
>  	if [ -n "$TEST_COUNT" ]
>  	then
> +		# Reset osnoise options before running test.
> +		[ "$NO_RESET_OSNOISE" == 1 ] || reset_osnoise
>  		# Run rtla; in case of failure, include its output as comment
>  		# in the test results.
>  		result=$(stdbuf -oL $TIMEOUT "$RTLA" $2 2>&1); exitcode=$?
> @@ -37,6 +69,14 @@ unset_timeout() {
>  	unset TIMEOUT
>  }
>  
> +set_no_reset_osnoise() {
> +	NO_RESET_OSNOISE=1
> +}
> +
> +unset_no_reset_osnoise() {
> +	unset NO_RESET_OSNOISE
> +}
> +
>  test_end() {
>  	# If running without TEST_COUNT, tests are not actually run, just
>  	# counted. In that case, re-run the test with the correct count.
> -- 
> 2.48.1
> 
> 
> 
Reviewed by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/6] rtla/tests: Test setting default options
  2025-03-20  9:25 ` [PATCH 6/6] rtla/tests: Test setting default options Tomas Glozar
@ 2025-03-20 20:42   ` John Kacur
  0 siblings, 0 replies; 14+ messages in thread
From: John Kacur @ 2025-03-20 20:42 UTC (permalink / raw)
  To: Tomas Glozar
  Cc: Steven Rostedt, linux-trace-kernel, linux-kernel, Luis Goncalves



On Thu, 20 Mar 2025, Tomas Glozar wrote:

> Add function to test engine to test with pre-set osnoise options, and
> use it to test whether osnoise period (as an example) is set correctly.
> 
> The test works by pre-setting a high period of 10 minutes and stop on
> threshold. Thus, it is easy to check whether rtla is properly resetting
> the period to default: if it is, the test will complete on time, since
> the first sample will overflow the threshold. If not, it will time out.
> 
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/tests/engine.sh | 26 ++++++++++++++++++++++++++
>  tools/tracing/rtla/tests/osnoise.t |  6 ++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/tools/tracing/rtla/tests/engine.sh b/tools/tracing/rtla/tests/engine.sh
> index 5db8aa4bc031..b1697b3e3f52 100644
> --- a/tools/tracing/rtla/tests/engine.sh
> +++ b/tools/tracing/rtla/tests/engine.sh
> @@ -61,6 +61,32 @@ check() {
>  	fi
>  }
>  
> +check_with_osnoise_options() {
> +	# Do the same as "check", but with pre-set osnoise options.
> +	# Note: rtla should reset the osnoise options, this is used to test
> +	# if it indeed does so.
> +	# Save original arguments
> +	arg1=$1
> +	arg2=$2
> +
> +	# Apply osnoise options (if not dry run)
> +	if [ -n "$TEST_COUNT" ]
> +	then
> +		[ "$NO_RESET_OSNOISE" == 1 ] || reset_osnoise
> +		shift
> +		while shift
> +		do
> +			[ "$1" == "" ] && continue
> +			option=$(echo $1 | cut -d '=' -f 1)
> +			value=$(echo $1 | cut -d '=' -f 2)
> +			echo "option: $option, value: $value"
> +			echo "$value" > "/sys/kernel/tracing/osnoise/$option" || return 1
> +		done
> +	fi
> +
> +	NO_RESET_OSNOISE=1 check "$arg1" "$arg2"
> +}
> +
>  set_timeout() {
>  	TIMEOUT="timeout -v -k 15s $1"
>  }
> diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
> index 86596e547893..e5995c03c790 100644
> --- a/tools/tracing/rtla/tests/osnoise.t
> +++ b/tools/tracing/rtla/tests/osnoise.t
> @@ -16,4 +16,10 @@ check "verify the  --trace param" \
>  check "verify the --entries/-E param" \
>  	"osnoise hist -P F:1 -c 0 -r 900000 -d 1M -b 10 -E 25"
>  
> +# Test setting default period by putting an absurdly high period
> +# and stopping on threshold.
> +# If default period is not set, this will time out.
> +check_with_osnoise_options "apply default period" \
> +	"osnoise hist -s 1" period_us=600000000
> +
>  test_end
> -- 
> 2.48.1
> 
> 
> 
looks correct
change "stop" to "stopping" in description
Reviewed-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/6] rtla/osnoise: Unify params struct
  2025-03-20 19:00   ` John Kacur
@ 2025-03-24 16:06     ` Steven Rostedt
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2025-03-24 16:06 UTC (permalink / raw)
  To: John Kacur; +Cc: Tomas Glozar, linux-trace-kernel, linux-kernel, Luis Goncalves

On Thu, 20 Mar 2025 15:00:08 -0400 (EDT)
John Kacur <jkacur@redhat.com> wrote:

> > -#include "utils.h"
> >  #include "osnoise.h"
> >  
> >  struct timerlat_params {
> > --   
> 
> Reviewed-by: John Kacur <jkacur@redhat.com>

Please trim your replies. I tend to stop scrolling if there's no comment
after three page downs. But it is very annoying to have to scroll down an
entire long patch to see a single reviewed-by tag :-/

-- Steve

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-03-24 16:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-20  9:24 [PATCH 0/6] rtla: Always set all tracer options Tomas Glozar
2025-03-20  9:24 ` [PATCH 1/6] rtla/osnoise: Unify params struct Tomas Glozar
2025-03-20 19:00   ` John Kacur
2025-03-24 16:06     ` Steven Rostedt
2025-03-20  9:24 ` [PATCH 2/6] rtla: Unify apply_config between top and hist Tomas Glozar
2025-03-20 19:27   ` John Kacur
2025-03-20  9:24 ` [PATCH 3/6] rtla/osnoise: Set OSNOISE_WORKLOAD to true Tomas Glozar
2025-03-20 19:40   ` John Kacur
2025-03-20  9:24 ` [PATCH 4/6] rtla: Always set all tracer options Tomas Glozar
2025-03-20 20:31   ` John Kacur
2025-03-20  9:24 ` [PATCH 5/6] rtla/tests: Reset osnoise options before check Tomas Glozar
2025-03-20 20:36   ` John Kacur
2025-03-20  9:25 ` [PATCH 6/6] rtla/tests: Test setting default options Tomas Glozar
2025-03-20 20:42   ` John Kacur

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).