| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8e41f0ee-7534-eedb-0a67-184ac195baf4@redhat.com>
Date: Thu, 20 Mar 2025 16:31:42 -0400 (EDT)
From: John Kacur <jkacur@...hat.com>
To: Tomas Glozar <tglozar@...hat.com>
cc: Steven Rostedt <rostedt@...dmis.org>, linux-trace-kernel@...r.kernel.org,
linux-kernel@...r.kernel.org, Luis Goncalves <lgoncalv@...hat.com>
Subject: Re: [PATCH 4/6] rtla: Always set all tracer options
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@...hat.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, ¶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.48.1
>
>
>
Reviewed-by: John Kacur <jkacur@...hat.com>
Powered by blists - more mailing lists