From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Tomas Glozar <tglozar@redhat.com>, John Kacur <jkacur@redhat.com>,
Luis Goncalves <lgoncalv@redhat.com>
Subject: [for-next][PATCH 6/9] rtla: Always set all tracer options
Date: Wed, 26 Mar 2025 10:55:55 -0400 [thread overview]
Message-ID: <20250326145604.654551325@goodmis.org> (raw)
In-Reply-To: 20250326145549.978154551@goodmis.org
From: Tomas Glozar <tglozar@redhat.com>
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.
Cc: Luis Goncalves <lgoncalv@redhat.com>
Link: https://lore.kernel.org/20250320092500.101385-5-tglozar@redhat.com
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>
Reviewed-by: John Kacur <jkacur@redhat.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
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, ¶ms->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.47.2
next prev parent reply other threads:[~2025-03-26 14:55 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-26 14:55 [for-next][PATCH 0/9] rtla: Updates for 6.15 Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 1/9] tools/build: Use SYSTEM_BPFTOOL for system bpftool Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 2/9] rtla: Fix segfault in save_trace_to_file call Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 3/9] rtla/osnoise: Unify params struct Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 4/9] rtla: Unify apply_config between top and hist Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 5/9] rtla/osnoise: Set OSNOISE_WORKLOAD to true Steven Rostedt
2025-03-26 14:55 ` Steven Rostedt [this message]
2025-03-26 14:55 ` [for-next][PATCH 7/9] rtla/tests: Reset osnoise options before check Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 8/9] rtla/tests: Test setting default options Steven Rostedt
2025-03-26 14:55 ` [for-next][PATCH 9/9] rtla: Add the ability to create ctags and etags Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250326145604.654551325@goodmis.org \
--to=rostedt@goodmis.org \
--cc=jkacur@redhat.com \
--cc=lgoncalv@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tglozar@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.