From 0d5077c73aceb51ef10d096160dd62a11db2f3e4 Mon Sep 17 00:00:00 2001 From: Steven Rostedt Date: Wed, 12 Nov 2025 11:35:56 -0500 Subject: [PATCH 01/17] MAINTAINERS: Add Tomas Glozar as a maintainer to RTLA tool Tomas will start taking over managing the changes to the Real-time Linux Analysis (RTLA) tool. Make him officially one of the maintainers. Also update the RTLA entry to include the linux-kernel mailing list as well as list the patchwork and git repository that the patches will go through. Cc: Tomas Glozar Cc: Gabriele Monaco Cc: John Kacur Cc: Crystal Wood Cc: Linus Torvalds Link: https://patch.msgid.link/20251112113556.47ec9d12@gandalf.local.home Signed-off-by: Steven Rostedt (Google) --- MAINTAINERS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3da2c26a796b..ff65fb76c86d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -21630,8 +21630,12 @@ F: tools/testing/selftests/rtc/ Real-time Linux Analysis (RTLA) tools M: Steven Rostedt +M: Tomas Glozar L: linux-trace-kernel@vger.kernel.org +L: linux-kernel@vger.kernel.org S: Maintained +Q: https://patchwork.kernel.org/project/linux-trace-kernel/list/ +T: git git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git F: Documentation/tools/rtla/ F: tools/tracing/rtla/ From 04fa6bf3736d727bb800dddb23c9a513969e565d Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Thu, 2 Oct 2025 15:35:38 +0300 Subject: [PATCH 02/17] tools/rtla: Add for_each_monitored_cpu() helper The rtla tools have many instances of iterating over CPUs while checking if they are monitored. Add a for_each_monitored_cpu() helper macro to make the code more readable and reduce code duplication. Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251002123553.389467-1-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/common.h | 4 ++++ tools/tracing/rtla/src/osnoise_hist.c | 28 ++++++---------------- tools/tracing/rtla/src/osnoise_top.c | 4 +--- tools/tracing/rtla/src/timerlat.c | 9 ++------ tools/tracing/rtla/src/timerlat_hist.c | 32 +++++++------------------- tools/tracing/rtla/src/timerlat_top.c | 4 +--- 6 files changed, 23 insertions(+), 58 deletions(-) diff --git a/tools/tracing/rtla/src/common.h b/tools/tracing/rtla/src/common.h index 355f113a14a3..9ec2b7632c37 100644 --- a/tools/tracing/rtla/src/common.h +++ b/tools/tracing/rtla/src/common.h @@ -107,6 +107,10 @@ struct common_params { struct timerlat_u_params user; }; +#define for_each_monitored_cpu(cpu, nr_cpus, common) \ + for (cpu = 0; cpu < nr_cpus; cpu++) \ + if (!(common)->cpus || CPU_ISSET(cpu, &(common)->monitored_cpus)) + struct tool_ops; /* diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index dffb6d0a98d7..844f0468953c 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -247,9 +247,7 @@ static void osnoise_hist_header(struct osnoise_tool *tool) if (!params->common.hist.no_index) trace_seq_printf(s, "Index"); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; @@ -278,9 +276,7 @@ osnoise_print_summary(struct osnoise_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "count:"); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; @@ -292,9 +288,7 @@ osnoise_print_summary(struct osnoise_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "min: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; @@ -307,9 +301,7 @@ osnoise_print_summary(struct osnoise_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "avg: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; @@ -325,9 +317,7 @@ osnoise_print_summary(struct osnoise_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "max: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; @@ -362,9 +352,7 @@ osnoise_print_stats(struct osnoise_tool *tool) trace_seq_printf(trace->seq, "%-6d", bucket * data->bucket_size); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; @@ -400,9 +388,7 @@ osnoise_print_stats(struct osnoise_tool *tool) if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "over: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].count) continue; diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index 95418f7ecc96..defa1eb63bee 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -243,9 +243,7 @@ osnoise_print_stats(struct osnoise_tool *top) osnoise_top_header(top); - for (i = 0; i < nr_cpus; i++) { - if (params->common.cpus && !CPU_ISSET(i, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(i, nr_cpus, ¶ms->common) { osnoise_top_print(top, i); } diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c index b69212874127..28ea4f6710c1 100644 --- a/tools/tracing/rtla/src/timerlat.c +++ b/tools/tracing/rtla/src/timerlat.c @@ -126,9 +126,7 @@ int timerlat_enable(struct osnoise_tool *tool) nr_cpus = sysconf(_SC_NPROCESSORS_CONF); - for (i = 0; i < nr_cpus; i++) { - if (params->common.cpus && !CPU_ISSET(i, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(i, nr_cpus, ¶ms->common) { if (save_cpu_idle_disable_state(i) < 0) { err_msg("Could not save cpu idle state.\n"); return -1; @@ -221,10 +219,7 @@ void timerlat_free(struct osnoise_tool *tool) if (dma_latency_fd >= 0) close(dma_latency_fd); if (params->deepest_idle_state >= -1) { - for (i = 0; i < nr_cpus; i++) { - if (params->common.cpus && - !CPU_ISSET(i, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(i, nr_cpus, ¶ms->common) { restore_cpu_idle_disable_state(i); } } diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 606c1688057b..02d3ffd2cf4b 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -305,9 +305,7 @@ static void timerlat_hist_header(struct osnoise_tool *tool) if (!params->common.hist.no_index) trace_seq_printf(s, "Index"); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -359,9 +357,7 @@ timerlat_print_summary(struct timerlat_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "count:"); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -383,9 +379,7 @@ timerlat_print_summary(struct timerlat_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "min: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -413,9 +407,7 @@ timerlat_print_summary(struct timerlat_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "avg: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -443,9 +435,7 @@ timerlat_print_summary(struct timerlat_params *params, if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "max: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -490,9 +480,7 @@ timerlat_print_stats_all(struct timerlat_params *params, sum.min_thread = ~0; sum.min_user = ~0; - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -639,9 +627,7 @@ timerlat_print_stats(struct osnoise_tool *tool) trace_seq_printf(trace->seq, "%-6d", bucket * data->bucket_size); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; @@ -679,9 +665,7 @@ timerlat_print_stats(struct osnoise_tool *tool) if (!params->common.hist.no_index) trace_seq_printf(trace->seq, "over: "); - for (cpu = 0; cpu < data->nr_cpus; cpu++) { - if (params->common.cpus && !CPU_ISSET(cpu, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(cpu, data->nr_cpus, ¶ms->common) { if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index fc479a0dcb59..607b57f2f231 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -459,9 +459,7 @@ timerlat_print_stats(struct osnoise_tool *top) timerlat_top_header(params, top); - for (i = 0; i < nr_cpus; i++) { - if (params->common.cpus && !CPU_ISSET(i, ¶ms->common.monitored_cpus)) - continue; + for_each_monitored_cpu(i, nr_cpus, ¶ms->common) { timerlat_top_print(top, i); timerlat_top_update_sum(top, i, &summary); } From 671314fce1c60913012e5643ffecdaa4578cfcb3 Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Thu, 2 Oct 2025 15:35:39 +0300 Subject: [PATCH 03/17] tools/rtla: Remove unused optional option_index The longindex argument of getopt_long() is optional and tied to the unused local variable option_index. Remove it to shorten the four longest functions and make the code neater. Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251002123553.389467-2-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/osnoise_hist.c | 5 +---- tools/tracing/rtla/src/osnoise_top.c | 5 +---- tools/tracing/rtla/src/timerlat_hist.c | 5 +---- tools/tracing/rtla/src/timerlat_top.c | 5 +---- 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index 844f0468953c..df0657b78980 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -524,11 +524,8 @@ static struct common_params {0, 0, 0, 0} }; - /* getopt_long stores the option index here. */ - int option_index = 0; - c = getopt_long(argc, argv, "a:c:C::b:d:e:E:DhH:p:P:r:s:S:t::T:01234:5:6:7:", - long_options, &option_index); + long_options, NULL); /* detect the end of the options. */ if (c == -1) diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index defa1eb63bee..1b5181e66b17 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -376,11 +376,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) {0, 0, 0, 0} }; - /* getopt_long stores the option index here. */ - int option_index = 0; - c = getopt_long(argc, argv, "a:c:C::d:De:hH:p:P:qr:s:S:t::T:0:1:2:3:", - long_options, &option_index); + long_options, NULL); /* Detect the end of the options. */ if (c == -1) diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 02d3ffd2cf4b..c432ef5f59e7 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -840,11 +840,8 @@ static struct common_params {0, 0, 0, 0} }; - /* getopt_long stores the option index here. */ - int option_index = 0; - c = getopt_long(argc, argv, "a:c:C::b:d:e:E:DhH:i:knp:P:s:t::T:uU0123456:7:8:9\1\2:\3:", - long_options, &option_index); + long_options, NULL); /* detect the end of the options. */ if (c == -1) diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 607b57f2f231..82e227d27af7 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -604,11 +604,8 @@ static struct common_params {0, 0, 0, 0} }; - /* getopt_long stores the option index here. */ - int option_index = 0; - c = getopt_long(argc, argv, "a:c:C::d:De:hH:i:knp:P:qs:t::T:uU0:1:2:345:6:7:", - long_options, &option_index); + long_options, NULL); /* detect the end of the options. */ if (c == -1) From b4275b23010df719ec6508ddbc84951dcd24adce Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Thu, 2 Oct 2025 20:08:45 +0300 Subject: [PATCH 04/17] tools/rtla: Fix unassigned nr_cpus In recently introduced timerlat_free(), the variable 'nr_cpus' is not assigned. Assign it with sysconf(_SC_NPROCESSORS_CONF) as done elsewhere. Remove the culprit: -Wno-maybe-uninitialized. The rest of the code is clean. Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Fixes: 2f3172f9dd58 ("tools/rtla: Consolidate code between osnoise/timerlat and hist/top") Link: https://lore.kernel.org/r/20251002170846.437888-1-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/Makefile.rtla | 2 +- tools/tracing/rtla/src/timerlat.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/tracing/rtla/Makefile.rtla b/tools/tracing/rtla/Makefile.rtla index 08c1b40883d3..1743d91829d4 100644 --- a/tools/tracing/rtla/Makefile.rtla +++ b/tools/tracing/rtla/Makefile.rtla @@ -18,7 +18,7 @@ export CC AR STRIP PKG_CONFIG LD_SO_CONF_PATH LDCONFIG FOPTS := -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong \ -fasynchronous-unwind-tables -fstack-clash-protection WOPTS := -O -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 \ - -Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized + -Wp,-D_GLIBCXX_ASSERTIONS ifeq ($(CC),clang) FOPTS := $(filter-out -flto=auto -ffat-lto-objects, $(FOPTS)) diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c index 28ea4f6710c1..df4f9bfe3433 100644 --- a/tools/tracing/rtla/src/timerlat.c +++ b/tools/tracing/rtla/src/timerlat.c @@ -213,7 +213,8 @@ void timerlat_analyze(struct osnoise_tool *tool, bool stopped) void timerlat_free(struct osnoise_tool *tool) { struct timerlat_params *params = to_timerlat_params(tool->params); - int nr_cpus, i; + int nr_cpus = sysconf(_SC_NPROCESSORS_CONF); + int i; timerlat_aa_destroy(); if (dma_latency_fd >= 0) From e4240db9336c25826a2d6634adcca86d5ee01bde Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Mon, 6 Oct 2025 16:31:00 +0200 Subject: [PATCH 05/17] rtla/timerlat_bpf: Stop tracing on user latency rtla-timerlat allows a *thread* latency threshold to be set via the -T/--thread option. However, the timerlat tracer calls this *total* latency (stop_tracing_total_us), and stops tracing also when the return-to-user latency is over the threshold. Change the behavior of the timerlat BPF program to reflect what the timerlat tracer is doing, to avoid discrepancy between stopping collecting data in the BPF program and stopping tracing in the timerlat tracer. Cc: stable@vger.kernel.org Fixes: e34293ddcebd ("rtla/timerlat: Add BPF skeleton to collect samples") Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20251006143100.137255-1-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/timerlat.bpf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/tracing/rtla/src/timerlat.bpf.c b/tools/tracing/rtla/src/timerlat.bpf.c index 084cd10c21fc..e2265b5d6491 100644 --- a/tools/tracing/rtla/src/timerlat.bpf.c +++ b/tools/tracing/rtla/src/timerlat.bpf.c @@ -148,6 +148,9 @@ int handle_timerlat_sample(struct trace_event_raw_timerlat_sample *tp_args) } else { update_main_hist(&hist_user, bucket); update_summary(&summary_user, latency, bucket); + + if (thread_threshold != 0 && latency_us >= thread_threshold) + set_stop_tracing(); } return 0; From 417bd0d502f90a2e785e7299dae4f248b5ac0292 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Tue, 7 Oct 2025 11:53:39 +0200 Subject: [PATCH 06/17] tools/rtla: Fix --on-threshold always triggering Commit 8d933d5c89e8 ("rtla/timerlat: Add continue action") moved the code performing on-threshold actions (enabled through --on-threshold option) to inside the RTLA main loop. The condition in the loop does not check whether the threshold was actually exceeded or if stop tracing was requested by the user through SIGINT or duration. This leads to a bug where on-threshold actions are always performed, even when the threshold was not hit. (BPF mode is not affected, since it uses a different condition in the while loop.) Add a condition that checks for !stop_tracing before executing the actions. Also, fix incorrect brackets in hist_main_loop to match the semantics of top_main_loop. Fixes: 8d933d5c89e8 ("rtla/timerlat: Add continue action") Fixes: 2f3172f9dd58 ("tools/rtla: Consolidate code between osnoise/timerlat and hist/top") Reviewed-by: Crystal Wood Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20251007095341.186923-1-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/common.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c index 2e6e3dac1897..b197037fc58b 100644 --- a/tools/tracing/rtla/src/common.c +++ b/tools/tracing/rtla/src/common.c @@ -268,6 +268,10 @@ int top_main_loop(struct osnoise_tool *tool) tool->ops->print_stats(tool); if (osnoise_trace_is_off(tool, record)) { + if (stop_tracing) + /* stop tracing requested, do not perform actions */ + return 0; + actions_perform(¶ms->threshold_actions); if (!params->threshold_actions.continue_flag) @@ -315,20 +319,22 @@ int hist_main_loop(struct osnoise_tool *tool) } if (osnoise_trace_is_off(tool, tool->record)) { + if (stop_tracing) + /* stop tracing requested, do not perform actions */ + break; + actions_perform(¶ms->threshold_actions); - if (!params->threshold_actions.continue_flag) { + if (!params->threshold_actions.continue_flag) /* continue flag not set, break */ break; - /* continue action reached, re-enable tracing */ - if (tool->record) - trace_instance_start(&tool->record->trace); - if (tool->aa) - trace_instance_start(&tool->aa->trace); - trace_instance_start(&tool->trace); - } - break; + /* continue action reached, re-enable tracing */ + if (tool->record) + trace_instance_start(&tool->record->trace); + if (tool->aa) + trace_instance_start(&tool->aa->trace); + trace_instance_start(&tool->trace); } /* is there still any user-threads ? */ From d649e9f04cb0224817dac8190461ef1674e32b37 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Tue, 7 Oct 2025 11:53:40 +0200 Subject: [PATCH 07/17] rtla/tests: Extend action tests to 5s In non-BPF mode, it takes up to 1 second for RTLA to notice that tracing has been stopped. That means that action tests cannot have a 1 second duration, as the SIGALRM will be racing with the threshold overflow. Previously, non-BPF mode actions were buggy and always executed the action, even when stopping on duration or SIGINT, preventing this issue from manifesting. Now that this has been fixed, the tests have become flaky, and this has to be adjusted. Fixes: 4e26f84abfbb ("rtla/tests: Add tests for actions") Fixes: 05b7e10687c6 ("tools/rtla: Add remaining support for osnoise actions") Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20251007095341.186923-2-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/tests/osnoise.t | 4 ++-- tools/tracing/rtla/tests/timerlat.t | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t index e3c89d45a6bb..08196443fef1 100644 --- a/tools/tracing/rtla/tests/osnoise.t +++ b/tools/tracing/rtla/tests/osnoise.t @@ -39,9 +39,9 @@ check "hist stop at failed action" \ check "top stop at failed action" \ "timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh" check "hist with continue" \ - "osnoise hist -S 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" + "osnoise hist -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" check "top with continue" \ - "osnoise top -q -S 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" + "osnoise top -q -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" check "hist with trace output at end" \ "osnoise hist -d 1s --on-end trace" 0 "^ Saving trace to osnoise_trace.txt$" check "top with trace output at end" \ diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t index b5d1e7260a9b..b550a6ae2445 100644 --- a/tools/tracing/rtla/tests/timerlat.t +++ b/tools/tracing/rtla/tests/timerlat.t @@ -60,9 +60,9 @@ check "hist stop at failed action" \ check "top stop at failed action" \ "timerlat top -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1ALL" check "hist with continue" \ - "timerlat hist -T 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" + "timerlat hist -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" check "top with continue" \ - "timerlat top -q -T 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" + "timerlat top -q -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" check "hist with trace output at end" \ "timerlat hist -d 1s --on-end trace" 0 "^ Saving trace to timerlat_trace.txt$" check "top with trace output at end" \ From 34c170ae5c3036ef879567a37409a2859e327342 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Tue, 7 Oct 2025 11:53:41 +0200 Subject: [PATCH 08/17] rtla/tests: Fix osnoise test calling timerlat osnoise test "top stop at failed action" is calling timerlat instead of osnoise by mistake. Fix it so that it calls the correct RTLA subcommand. Fixes: 05b7e10687c6 ("tools/rtla: Add remaining support for osnoise actions") Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20251007095341.186923-3-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/tests/osnoise.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t index 08196443fef1..396334608920 100644 --- a/tools/tracing/rtla/tests/osnoise.t +++ b/tools/tracing/rtla/tests/osnoise.t @@ -37,7 +37,7 @@ check "multiple actions" \ check "hist stop at failed action" \ "osnoise hist -S 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1# RTLA osnoise histogram$" check "top stop at failed action" \ - "timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh" + "osnoise top -S 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh" check "hist with continue" \ "osnoise hist -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" check "top with continue" \ From 8cbb25db81544f0bfc05c037ad61d3e70d031f88 Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Sat, 11 Oct 2025 11:27:34 +0300 Subject: [PATCH 09/17] tools/rtla: Add fatal() and replace error handling pattern The code contains some technical debt in error handling, which complicates the consolidation of duplicated code. Introduce an fatal() function to replace the common pattern of err_msg() followed by exit(EXIT_FAILURE), reducing the length of an already long function. Further patches using fatal() follow. Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251011082738.173670-2-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/osnoise_hist.c | 42 ++++++++-------------- tools/tracing/rtla/src/osnoise_top.c | 42 ++++++++-------------- tools/tracing/rtla/src/timerlat_hist.c | 50 +++++++++----------------- tools/tracing/rtla/src/timerlat_top.c | 48 +++++++++---------------- tools/tracing/rtla/src/timerlat_u.c | 12 +++---- tools/tracing/rtla/src/utils.c | 15 ++++++++ tools/tracing/rtla/src/utils.h | 1 + 7 files changed, 81 insertions(+), 129 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index df0657b78980..d30a9a03b764 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -575,10 +575,8 @@ static struct common_params break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -598,10 +596,8 @@ static struct common_params case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs"); break; case 'p': params->period = get_llong_from_str(optarg); @@ -654,10 +650,8 @@ static struct common_params case '4': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s", optarg); } else { osnoise_hist_usage("--trigger requires a previous -e\n"); } @@ -665,10 +659,8 @@ static struct common_params case '5': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s", optarg); } else { osnoise_hist_usage("--filter requires a previous -e\n"); } @@ -682,18 +674,14 @@ static struct common_params case '8': retval = actions_parse(¶ms->common.threshold_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; case '9': retval = actions_parse(¶ms->common.end_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; default: osnoise_hist_usage("Invalid option"); @@ -703,10 +691,8 @@ static struct common_params if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("rtla needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("rtla needs root permission"); if (params->common.hist.no_index && !params->common.hist.with_zeros) osnoise_hist_usage("no-index set and with-zeros not set - it does not make sense"); diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index 1b5181e66b17..487daac8908c 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -421,10 +421,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -438,10 +436,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs"); break; case 'p': params->period = get_llong_from_str(optarg); @@ -485,10 +481,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case '0': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s", optarg); } else { osnoise_top_usage(params, "--trigger requires a previous -e\n"); } @@ -496,10 +490,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case '1': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s", optarg); } else { osnoise_top_usage(params, "--filter requires a previous -e\n"); } @@ -513,18 +505,14 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case '4': retval = actions_parse(¶ms->common.threshold_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; case '5': retval = actions_parse(¶ms->common.end_actions, optarg, "osnoise_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; default: osnoise_top_usage(params, "Invalid option"); @@ -534,10 +522,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("osnoise needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("osnoise needs root permission"); return ¶ms->common; } diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index c432ef5f59e7..f98deb16b452 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -894,10 +894,8 @@ static struct common_params break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -917,10 +915,8 @@ static struct common_params case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs"); break; case 'i': params->common.stop_us = get_llong_from_str(optarg); @@ -986,10 +982,8 @@ static struct common_params case '6': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s", optarg); } else { timerlat_hist_usage("--trigger requires a previous -e\n"); } @@ -997,20 +991,16 @@ static struct common_params case '7': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s", optarg); } else { timerlat_hist_usage("--filter requires a previous -e\n"); } break; case '8': params->dma_latency = get_llong_from_str(optarg); - if (params->dma_latency < 0 || params->dma_latency > 10000) { - err_msg("--dma-latency needs to be >= 0 and < 10000"); - exit(EXIT_FAILURE); - } + if (params->dma_latency < 0 || params->dma_latency > 10000) + fatal("--dma-latency needs to be >= 0 and < 10000"); break; case '9': params->no_aa = 1; @@ -1030,31 +1020,25 @@ static struct common_params case '\5': retval = actions_parse(¶ms->common.threshold_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; case '\6': retval = actions_parse(¶ms->common.end_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; default: - timerlat_hist_usage("Invalid option"); + fatal("Invalid option"); } } if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("rtla needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("rtla needs root permission"); if (params->common.hist.no_irq && params->common.hist.no_thread) timerlat_hist_usage("no-irq and no-thread set, there is nothing to do here"); diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 82e227d27af7..9664b8af727e 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -666,10 +666,8 @@ static struct common_params break; case 'e': tevent = trace_event_alloc(optarg); - if (!tevent) { - err_msg("Error alloc trace event"); - exit(EXIT_FAILURE); - } + if (!tevent) + fatal("Error alloc trace event"); if (params->common.events) tevent->next = params->common.events; @@ -682,10 +680,8 @@ static struct common_params case 'H': params->common.hk_cpus = 1; retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set); - if (retval) { - err_msg("Error parsing house keeping CPUs\n"); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error parsing house keeping CPUs"); break; case 'i': params->common.stop_us = get_llong_from_str(optarg); @@ -736,10 +732,8 @@ static struct common_params case '0': /* trigger */ if (params->common.events) { retval = trace_event_add_trigger(params->common.events, optarg); - if (retval) { - err_msg("Error adding trigger %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding trigger %s", optarg); } else { timerlat_top_usage("--trigger requires a previous -e\n"); } @@ -747,20 +741,16 @@ static struct common_params case '1': /* filter */ if (params->common.events) { retval = trace_event_add_filter(params->common.events, optarg); - if (retval) { - err_msg("Error adding filter %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Error adding filter %s", optarg); } else { timerlat_top_usage("--filter requires a previous -e\n"); } break; case '2': /* dma-latency */ params->dma_latency = get_llong_from_str(optarg); - if (params->dma_latency < 0 || params->dma_latency > 10000) { - err_msg("--dma-latency needs to be >= 0 and < 10000"); - exit(EXIT_FAILURE); - } + if (params->dma_latency < 0 || params->dma_latency > 10000) + fatal("--dma-latency needs to be >= 0 and < 10000"); break; case '3': /* no-aa */ params->no_aa = 1; @@ -780,18 +770,14 @@ static struct common_params case '9': retval = actions_parse(¶ms->common.threshold_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; case '\1': retval = actions_parse(¶ms->common.end_actions, optarg, "timerlat_trace.txt"); - if (retval) { - err_msg("Invalid action %s\n", optarg); - exit(EXIT_FAILURE); - } + if (retval) + fatal("Invalid action %s", optarg); break; default: timerlat_top_usage("Invalid option"); @@ -801,10 +787,8 @@ static struct common_params if (trace_output) actions_add_trace_output(¶ms->common.threshold_actions, trace_output); - if (geteuid()) { - err_msg("rtla needs root permission\n"); - exit(EXIT_FAILURE); - } + if (geteuid()) + fatal("rtla needs root permission"); /* * Auto analysis only happens if stop tracing, thus: diff --git a/tools/tracing/rtla/src/timerlat_u.c b/tools/tracing/rtla/src/timerlat_u.c index 01dbf9a6b5a5..ce68e39d25fd 100644 --- a/tools/tracing/rtla/src/timerlat_u.c +++ b/tools/tracing/rtla/src/timerlat_u.c @@ -51,10 +51,8 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params) if (!params->sched_param) { retval = sched_setscheduler(0, SCHED_FIFO, &sp); - if (retval < 0) { - err_msg("Error setting timerlat u default priority: %s\n", strerror(errno)); - exit(1); - } + if (retval < 0) + fatal("Error setting timerlat u default priority: %s", strerror(errno)); } else { retval = __set_sched_attr(getpid(), params->sched_param); if (retval) { @@ -78,10 +76,8 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params) snprintf(buffer, sizeof(buffer), "osnoise/per_cpu/cpu%d/timerlat_fd", cpu); timerlat_fd = tracefs_instance_file_open(NULL, buffer, O_RDONLY); - if (timerlat_fd < 0) { - err_msg("Error opening %s:%s\n", buffer, strerror(errno)); - exit(1); - } + if (timerlat_fd < 0) + fatal("Error opening %s:%s", buffer, strerror(errno)); debug_msg("User-space timerlat pid %d on cpu %d\n", gettid(), cpu); diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c index d6ab15dcb490..54334c676a22 100644 --- a/tools/tracing/rtla/src/utils.c +++ b/tools/tracing/rtla/src/utils.c @@ -56,6 +56,21 @@ void debug_msg(const char *fmt, ...) fprintf(stderr, "%s", message); } +/* + * fatal - print an error message and EOL to stderr and exit with ERROR + */ +void fatal(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + fprintf(stderr, "\n"); + + exit(ERROR); +} + /* * get_llong_from_str - get a long long int from a string */ diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h index a2a6f89f342d..1be095f9a7e6 100644 --- a/tools/tracing/rtla/src/utils.h +++ b/tools/tracing/rtla/src/utils.h @@ -19,6 +19,7 @@ extern int config_debug; void debug_msg(const char *fmt, ...); void err_msg(const char *fmt, ...); +void fatal(const char *fmt, ...); long parse_seconds_duration(char *val); void get_duration(time_t start_time, char *output, int output_size); From 4e5e7210f9721796a21a4b2d646fa043a7d88fef Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Sat, 11 Oct 2025 11:27:35 +0300 Subject: [PATCH 10/17] tools/rtla: Replace timerlat_top_usage("...") with fatal("...") A long time ago, when the usage help was short, it was a favor to the user to show it on error. Now that the usage help has become very long, it is too noisy to dump the complete help text for each typo after the error message itself. Replace timerlat_top_usage("...\n") with fatal("...") on errors. Remove the already unused 'usage' argument from timerlat_top_usage(). Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251011082738.173670-3-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/timerlat_top.c | 28 +++++++++++---------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 9664b8af727e..67a5b6ab78a6 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -474,7 +474,7 @@ timerlat_print_stats(struct osnoise_tool *top) /* * timerlat_top_usage - prints timerlat top usage message */ -static void timerlat_top_usage(char *usage) +static void timerlat_top_usage(void) { int i; @@ -522,18 +522,12 @@ static void timerlat_top_usage(char *usage) NULL, }; - if (usage) - fprintf(stderr, "%s\n", usage); - fprintf(stderr, "rtla timerlat top: a per-cpu summary of the timer latency (version %s)\n", VERSION); for (i = 0; msg[i]; i++) fprintf(stderr, "%s\n", msg[i]); - if (usage) - exit(EXIT_FAILURE); - exit(EXIT_SUCCESS); } @@ -643,7 +637,7 @@ static struct common_params case 'c': retval = parse_cpu_set(optarg, ¶ms->common.monitored_cpus); if (retval) - timerlat_top_usage("\nInvalid -c cpu list\n"); + fatal("Invalid -c cpu list"); params->common.cpus = optarg; break; case 'C': @@ -662,7 +656,7 @@ static struct common_params case 'd': params->common.duration = parse_seconds_duration(optarg); if (!params->common.duration) - timerlat_top_usage("Invalid -d duration\n"); + fatal("Invalid -d duration"); break; case 'e': tevent = trace_event_alloc(optarg); @@ -675,7 +669,7 @@ static struct common_params break; case 'h': case '?': - timerlat_top_usage(NULL); + timerlat_top_usage(); break; case 'H': params->common.hk_cpus = 1; @@ -695,12 +689,12 @@ static struct common_params case 'p': params->timerlat_period_us = get_llong_from_str(optarg); if (params->timerlat_period_us > 1000000) - timerlat_top_usage("Period longer than 1 s\n"); + fatal("Period longer than 1 s"); break; case 'P': retval = parse_prio(optarg, ¶ms->common.sched_param); if (retval == -1) - timerlat_top_usage("Invalid -P priority"); + fatal("Invalid -P priority"); params->common.set_sched = 1; break; case 'q': @@ -735,7 +729,7 @@ static struct common_params if (retval) fatal("Error adding trigger %s", optarg); } else { - timerlat_top_usage("--trigger requires a previous -e\n"); + fatal("--trigger requires a previous -e"); } break; case '1': /* filter */ @@ -744,7 +738,7 @@ static struct common_params if (retval) fatal("Error adding filter %s", optarg); } else { - timerlat_top_usage("--filter requires a previous -e\n"); + fatal("--filter requires a previous -e"); } break; case '2': /* dma-latency */ @@ -780,7 +774,7 @@ static struct common_params fatal("Invalid action %s", optarg); break; default: - timerlat_top_usage("Invalid option"); + fatal("Invalid option"); } } @@ -797,10 +791,10 @@ static struct common_params params->no_aa = 1; if (params->no_aa && params->common.aa_only) - timerlat_top_usage("--no-aa and --aa-only are mutually exclusive!"); + fatal("--no-aa and --aa-only are mutually exclusive!"); if (params->common.kernel_workload && params->common.user_workload) - timerlat_top_usage("--kernel-threads and --user-threads are mutually exclusive!"); + fatal("--kernel-threads and --user-threads are mutually exclusive!"); /* * If auto-analysis or trace output is enabled, switch from BPF mode to From 8f4264e046ef75d35cfffeb7aadfc5d84717df3e Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Sat, 11 Oct 2025 11:27:36 +0300 Subject: [PATCH 11/17] tools/rtla: Replace timerlat_hist_usage("...") with fatal("...") A long time ago, when the usage help was short, it was a favor to the user to show it on error. Now that the usage help has become very long, it is too noisy to dump the complete help text for each typo after the error message itself. Replace timerlat_hist_usage("...\n") with fatal("...") on errors. Remove the already unused 'usage' argument from timerlat_hist_usage(). Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251011082738.173670-4-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/timerlat_hist.c | 32 +++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index f98deb16b452..1b505531ad3b 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -694,7 +694,7 @@ timerlat_print_stats(struct osnoise_tool *tool) /* * timerlat_hist_usage - prints timerlat top usage message */ -static void timerlat_hist_usage(char *usage) +static void timerlat_hist_usage(void) { int i; @@ -750,18 +750,12 @@ static void timerlat_hist_usage(char *usage) NULL, }; - if (usage) - fprintf(stderr, "%s\n", usage); - fprintf(stderr, "rtla timerlat hist: a per-cpu histogram of the timer latency (version %s)\n", VERSION); for (i = 0; msg[i]; i++) fprintf(stderr, "%s\n", msg[i]); - if (usage) - exit(EXIT_FAILURE); - exit(EXIT_SUCCESS); } @@ -865,7 +859,7 @@ static struct common_params case 'c': retval = parse_cpu_set(optarg, ¶ms->common.monitored_cpus); if (retval) - timerlat_hist_usage("\nInvalid -c cpu list\n"); + fatal("Invalid -c cpu list"); params->common.cpus = optarg; break; case 'C': @@ -882,7 +876,7 @@ static struct common_params params->common.hist.bucket_size = get_llong_from_str(optarg); if (params->common.hist.bucket_size == 0 || params->common.hist.bucket_size >= 1000000) - timerlat_hist_usage("Bucket size needs to be > 0 and <= 1000000\n"); + fatal("Bucket size needs to be > 0 and <= 1000000"); break; case 'D': config_debug = 1; @@ -890,7 +884,7 @@ static struct common_params case 'd': params->common.duration = parse_seconds_duration(optarg); if (!params->common.duration) - timerlat_hist_usage("Invalid -D duration\n"); + fatal("Invalid -D duration"); break; case 'e': tevent = trace_event_alloc(optarg); @@ -906,11 +900,11 @@ static struct common_params params->common.hist.entries = get_llong_from_str(optarg); if (params->common.hist.entries < 10 || params->common.hist.entries > 9999999) - timerlat_hist_usage("Entries must be > 10 and < 9999999\n"); + fatal("Entries must be > 10 and < 9999999"); break; case 'h': case '?': - timerlat_hist_usage(NULL); + timerlat_hist_usage(); break; case 'H': params->common.hk_cpus = 1; @@ -930,12 +924,12 @@ static struct common_params case 'p': params->timerlat_period_us = get_llong_from_str(optarg); if (params->timerlat_period_us > 1000000) - timerlat_hist_usage("Period longer than 1 s\n"); + fatal("Period longer than 1 s"); break; case 'P': retval = parse_prio(optarg, ¶ms->common.sched_param); if (retval == -1) - timerlat_hist_usage("Invalid -P priority"); + fatal("Invalid -P priority"); params->common.set_sched = 1; break; case 's': @@ -985,7 +979,7 @@ static struct common_params if (retval) fatal("Error adding trigger %s", optarg); } else { - timerlat_hist_usage("--trigger requires a previous -e\n"); + fatal("--trigger requires a previous -e"); } break; case '7': /* filter */ @@ -994,7 +988,7 @@ static struct common_params if (retval) fatal("Error adding filter %s", optarg); } else { - timerlat_hist_usage("--filter requires a previous -e\n"); + fatal("--filter requires a previous -e"); } break; case '8': @@ -1041,10 +1035,10 @@ static struct common_params fatal("rtla needs root permission"); if (params->common.hist.no_irq && params->common.hist.no_thread) - timerlat_hist_usage("no-irq and no-thread set, there is nothing to do here"); + fatal("no-irq and no-thread set, there is nothing to do here"); if (params->common.hist.no_index && !params->common.hist.with_zeros) - timerlat_hist_usage("no-index set with with-zeros is not set - it does not make sense"); + fatal("no-index set with with-zeros is not set - it does not make sense"); /* * Auto analysis only happens if stop tracing, thus: @@ -1053,7 +1047,7 @@ static struct common_params params->no_aa = 1; if (params->common.kernel_workload && params->common.user_workload) - timerlat_hist_usage("--kernel-threads and --user-threads are mutually exclusive!"); + fatal("--kernel-threads and --user-threads are mutually exclusive!"); /* * If auto-analysis or trace output is enabled, switch from BPF mode to From 92b5b55e5e8eeed9094dc68cdbb1afe31dd0ff37 Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Sat, 11 Oct 2025 11:27:37 +0300 Subject: [PATCH 12/17] tools/rtla: Replace osnoise_top_usage("...") with fatal("...") A long time ago, when the usage help was short, it was a favor to the user to show it on error. Now that the usage help has become very long, it is too noisy to dump the complete help text for each typo after the error message itself. Replace osnoise_top_usage("...") with fatal("...") on errors. Remove the already unused 'usage' argument from osnoise_top_usage(). Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251011082738.173670-5-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/osnoise_top.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index 487daac8908c..de8e26d7c68b 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -255,7 +255,7 @@ osnoise_print_stats(struct osnoise_tool *top) /* * osnoise_top_usage - prints osnoise top usage message */ -static void osnoise_top_usage(struct osnoise_params *params, char *usage) +static void osnoise_top_usage(struct osnoise_params *params) { int i; @@ -294,9 +294,6 @@ static void osnoise_top_usage(struct osnoise_params *params, char *usage) NULL, }; - if (usage) - fprintf(stderr, "%s\n", usage); - if (params->mode == MODE_OSNOISE) { fprintf(stderr, "rtla osnoise top: a per-cpu summary of the OS noise (version %s)\n", @@ -316,9 +313,6 @@ static void osnoise_top_usage(struct osnoise_params *params, char *usage) for (i = 0; msg[i]; i++) fprintf(stderr, "%s\n", msg[i]); - if (usage) - exit(EXIT_FAILURE); - exit(EXIT_SUCCESS); } @@ -398,7 +392,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case 'c': retval = parse_cpu_set(optarg, ¶ms->common.monitored_cpus); if (retval) - osnoise_top_usage(params, "\nInvalid -c cpu list\n"); + fatal("Invalid -c cpu list"); params->common.cpus = optarg; break; case 'C': @@ -417,7 +411,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case 'd': params->common.duration = parse_seconds_duration(optarg); if (!params->common.duration) - osnoise_top_usage(params, "Invalid -d duration\n"); + fatal("Invalid -d duration"); break; case 'e': tevent = trace_event_alloc(optarg); @@ -431,7 +425,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) break; case 'h': case '?': - osnoise_top_usage(params, NULL); + osnoise_top_usage(params); break; case 'H': params->common.hk_cpus = 1; @@ -442,12 +436,12 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case 'p': params->period = get_llong_from_str(optarg); if (params->period > 10000000) - osnoise_top_usage(params, "Period longer than 10 s\n"); + fatal("Period longer than 10 s"); break; case 'P': retval = parse_prio(optarg, ¶ms->common.sched_param); if (retval == -1) - osnoise_top_usage(params, "Invalid -P priority"); + fatal("Invalid -P priority"); params->common.set_sched = 1; break; case 'q': @@ -456,7 +450,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) case 'r': params->runtime = get_llong_from_str(optarg); if (params->runtime < 100) - osnoise_top_usage(params, "Runtime shorter than 100 us\n"); + fatal("Runtime shorter than 100 us"); break; case 's': params->common.stop_us = get_llong_from_str(optarg); @@ -484,7 +478,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) if (retval) fatal("Error adding trigger %s", optarg); } else { - osnoise_top_usage(params, "--trigger requires a previous -e\n"); + fatal("--trigger requires a previous -e"); } break; case '1': /* filter */ @@ -493,7 +487,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) if (retval) fatal("Error adding filter %s", optarg); } else { - osnoise_top_usage(params, "--filter requires a previous -e\n"); + fatal("--filter requires a previous -e"); } break; case '2': @@ -515,7 +509,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) fatal("Invalid action %s", optarg); break; default: - osnoise_top_usage(params, "Invalid option"); + fatal("Invalid option"); } } From 49c15794198ff03a4fa844f894f7e5d4bdbffcfc Mon Sep 17 00:00:00 2001 From: Costa Shulyupin Date: Sat, 11 Oct 2025 11:27:38 +0300 Subject: [PATCH 13/17] tools/rtla: Replace osnoise_hist_usage("...") with fatal("...") A long time ago, when the usage help was short, it was a favor to the user to show it on error. Now that the usage help has become very long, it is too noisy to dump the complete help text for each typo after the error message itself. Replace osnoise_hist_usage("...") with fatal("...") on errors. Remove the already unused 'usage' argument from osnoise_hist_usage(). Signed-off-by: Costa Shulyupin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/20251011082738.173670-6-costa.shul@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/osnoise_hist.c | 32 +++++++++++---------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index d30a9a03b764..1d841a3056bc 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -407,7 +407,7 @@ osnoise_print_stats(struct osnoise_tool *tool) /* * osnoise_hist_usage - prints osnoise hist usage message */ -static void osnoise_hist_usage(char *usage) +static void osnoise_hist_usage(void) { int i; @@ -453,18 +453,12 @@ static void osnoise_hist_usage(char *usage) NULL, }; - if (usage) - fprintf(stderr, "%s\n", usage); - fprintf(stderr, "rtla osnoise hist: a per-cpu histogram of the OS noise (version %s)\n", VERSION); for (i = 0; msg[i]; i++) fprintf(stderr, "%s\n", msg[i]); - if (usage) - exit(EXIT_FAILURE); - exit(EXIT_SUCCESS); } @@ -547,12 +541,12 @@ static struct common_params params->common.hist.bucket_size = get_llong_from_str(optarg); if (params->common.hist.bucket_size == 0 || params->common.hist.bucket_size >= 1000000) - osnoise_hist_usage("Bucket size needs to be > 0 and <= 1000000\n"); + fatal("Bucket size needs to be > 0 and <= 1000000"); break; case 'c': retval = parse_cpu_set(optarg, ¶ms->common.monitored_cpus); if (retval) - osnoise_hist_usage("\nInvalid -c cpu list\n"); + fatal("Invalid -c cpu list"); params->common.cpus = optarg; break; case 'C': @@ -571,7 +565,7 @@ static struct common_params case 'd': params->common.duration = parse_seconds_duration(optarg); if (!params->common.duration) - osnoise_hist_usage("Invalid -D duration\n"); + fatal("Invalid -D duration"); break; case 'e': tevent = trace_event_alloc(optarg); @@ -587,11 +581,11 @@ static struct common_params params->common.hist.entries = get_llong_from_str(optarg); if (params->common.hist.entries < 10 || params->common.hist.entries > 9999999) - osnoise_hist_usage("Entries must be > 10 and < 9999999\n"); + fatal("Entries must be > 10 and < 9999999"); break; case 'h': case '?': - osnoise_hist_usage(NULL); + osnoise_hist_usage(); break; case 'H': params->common.hk_cpus = 1; @@ -602,18 +596,18 @@ static struct common_params case 'p': params->period = get_llong_from_str(optarg); if (params->period > 10000000) - osnoise_hist_usage("Period longer than 10 s\n"); + fatal("Period longer than 10 s"); break; case 'P': retval = parse_prio(optarg, ¶ms->common.sched_param); if (retval == -1) - osnoise_hist_usage("Invalid -P priority"); + fatal("Invalid -P priority"); params->common.set_sched = 1; break; case 'r': params->runtime = get_llong_from_str(optarg); if (params->runtime < 100) - osnoise_hist_usage("Runtime shorter than 100 us\n"); + fatal("Runtime shorter than 100 us"); break; case 's': params->common.stop_us = get_llong_from_str(optarg); @@ -653,7 +647,7 @@ static struct common_params if (retval) fatal("Error adding trigger %s", optarg); } else { - osnoise_hist_usage("--trigger requires a previous -e\n"); + fatal("--trigger requires a previous -e"); } break; case '5': /* filter */ @@ -662,7 +656,7 @@ static struct common_params if (retval) fatal("Error adding filter %s", optarg); } else { - osnoise_hist_usage("--filter requires a previous -e\n"); + fatal("--filter requires a previous -e"); } break; case '6': @@ -684,7 +678,7 @@ static struct common_params fatal("Invalid action %s", optarg); break; default: - osnoise_hist_usage("Invalid option"); + fatal("Invalid option"); } } @@ -695,7 +689,7 @@ static struct common_params fatal("rtla needs root permission"); if (params->common.hist.no_index && !params->common.hist.with_zeros) - osnoise_hist_usage("no-index set and with-zeros not set - it does not make sense"); + fatal("no-index set and with-zeros not set - it does not make sense"); return ¶ms->common; } From 7b71f3a6986c93defbb72bb6c143e04122720cb1 Mon Sep 17 00:00:00 2001 From: Ivan Pravdin Date: Mon, 3 Nov 2025 11:19:06 -0500 Subject: [PATCH 14/17] rtla: Fix -C/--cgroup interface Currently, user can only specify cgroup to the tracer's thread the following ways: `-C[cgroup]` `-C[=cgroup]` `--cgroup[=cgroup]` If user tries to specify cgroup as `-C [cgroup]` or `--cgroup [cgroup]`, the parser silently fails and rtla's cgroup is used for the tracer threads. To make interface more user-friendly, allow user to specify cgroup in the aforementioned way, i.e. `-C [cgroup]` and `--cgroup [cgroup]`. Refactor identical logic between -t/--trace and -C/--cgroup into a common function. Change documentation to reflect this user interface change. Fixes: a957cbc02531 ("rtla: Add -C cgroup support") Signed-off-by: Ivan Pravdin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/16132f1565cf5142b5fbd179975be370b529ced7.1762186418.git.ipravdin.official@gmail.com [ use capital letter in subject, as required by tracing subsystem ] Signed-off-by: Tomas Glozar --- Documentation/tools/rtla/common_options.rst | 2 +- tools/tracing/rtla/src/osnoise_hist.c | 26 ++++++--------------- tools/tracing/rtla/src/osnoise_top.c | 26 ++++++--------------- tools/tracing/rtla/src/timerlat_hist.c | 26 ++++++--------------- tools/tracing/rtla/src/timerlat_top.c | 26 ++++++--------------- tools/tracing/rtla/src/utils.c | 26 +++++++++++++++++++++ tools/tracing/rtla/src/utils.h | 1 + 7 files changed, 56 insertions(+), 77 deletions(-) diff --git a/Documentation/tools/rtla/common_options.rst b/Documentation/tools/rtla/common_options.rst index 77ef35d3f831..edc8e850f5d0 100644 --- a/Documentation/tools/rtla/common_options.rst +++ b/Documentation/tools/rtla/common_options.rst @@ -42,7 +42,7 @@ - *f:prio* - use SCHED_FIFO with *prio*; - *d:runtime[us|ms|s]:period[us|ms|s]* - use SCHED_DEADLINE with *runtime* and *period* in nanoseconds. -**-C**, **--cgroup**\[*=cgroup*] +**-C**, **--cgroup** \[*cgroup*] Set a *cgroup* to the tracer's threads. If the **-C** option is passed without arguments, the tracer's thread will inherit **rtla**'s *cgroup*. Otherwise, the threads will be placed on the *cgroup* passed to the option. diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index 1d841a3056bc..1d06ea47b663 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -414,9 +414,9 @@ static void osnoise_hist_usage(void) static const char * const msg[] = { "", " usage: rtla osnoise hist [-h] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\", - " [-T us] [-t[file]] [-e sys[:event]] [--filter ] [--trigger ] \\", + " [-T us] [-t [file]] [-e sys[:event]] [--filter ] [--trigger ] \\", " [-c cpu-list] [-H cpu-list] [-P priority] [-b N] [-E N] [--no-header] [--no-summary] \\", - " [--no-index] [--with-zeros] [-C[=cgroup_name]] [--warm-up]", + " [--no-index] [--with-zeros] [-C [cgroup_name]] [--warm-up]", "", " -h/--help: print this menu", " -a/--auto: set automatic trace mode, stopping the session if argument in us sample is hit", @@ -427,10 +427,10 @@ static void osnoise_hist_usage(void) " -T/--threshold us: the minimum delta to be considered a noise", " -c/--cpus cpu-list: list of cpus to run osnoise threads", " -H/--house-keeping cpus: run rtla control threads only on the given cpus", - " -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", + " -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", " -d/--duration time[s|m|h|d]: duration of the session", " -D/--debug: print debug info", - " -t/--trace[file]: save the stopped trace to [file|osnoise_trace.txt]", + " -t/--trace [file]: save the stopped trace to [file|osnoise_trace.txt]", " -e/--event : enable the in the trace instance, multiple -e are allowed", " --filter : enable a trace event filter to the previous -e event", " --trigger : enable a trace event trigger to the previous -e event", @@ -551,13 +551,7 @@ static struct common_params break; case 'C': params->common.cgroup = 1; - if (!optarg) { - /* will inherit this cgroup */ - params->common.cgroup_name = NULL; - } else if (*optarg == '=') { - /* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name = parse_optional_arg(argc, argv); break; case 'D': config_debug = 1; @@ -619,14 +613,8 @@ static struct common_params params->threshold = get_llong_from_str(optarg); break; case 't': - if (optarg) { - if (optarg[0] == '=') - trace_output = &optarg[1]; - else - trace_output = &optarg[0]; - } else if (optind < argc && argv[optind][0] != '0') - trace_output = argv[optind]; - else + trace_output = parse_optional_arg(argc, argv); + if (!trace_output) trace_output = "osnoise_trace.txt"; break; case '0': /* no header */ diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index de8e26d7c68b..cac60e4c267e 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -261,8 +261,8 @@ static void osnoise_top_usage(struct osnoise_params *params) static const char * const msg[] = { " [-h] [-q] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\", - " [-T us] [-t[file]] [-e sys[:event]] [--filter ] [--trigger ] \\", - " [-c cpu-list] [-H cpu-list] [-P priority] [-C[=cgroup_name]] [--warm-up s]", + " [-T us] [-t [file]] [-e sys[:event]] [--filter ] [--trigger ] \\", + " [-c cpu-list] [-H cpu-list] [-P priority] [-C [cgroup_name]] [--warm-up s]", "", " -h/--help: print this menu", " -a/--auto: set automatic trace mode, stopping the session if argument in us sample is hit", @@ -273,10 +273,10 @@ static void osnoise_top_usage(struct osnoise_params *params) " -T/--threshold us: the minimum delta to be considered a noise", " -c/--cpus cpu-list: list of cpus to run osnoise threads", " -H/--house-keeping cpus: run rtla control threads only on the given cpus", - " -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", + " -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", " -d/--duration time[s|m|h|d]: duration of the session", " -D/--debug: print debug info", - " -t/--trace[file]: save the stopped trace to [file|osnoise_trace.txt]", + " -t/--trace [file]: save the stopped trace to [file|osnoise_trace.txt]", " -e/--event : enable the in the trace instance, multiple -e are allowed", " --filter : enable a trace event filter to the previous -e event", " --trigger : enable a trace event trigger to the previous -e event", @@ -397,13 +397,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) break; case 'C': params->common.cgroup = 1; - if (!optarg) { - /* will inherit this cgroup */ - params->common.cgroup_name = NULL; - } else if (*optarg == '=') { - /* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name = parse_optional_arg(argc, argv); break; case 'D': config_debug = 1; @@ -459,14 +453,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) params->common.stop_total_us = get_llong_from_str(optarg); break; case 't': - if (optarg) { - if (optarg[0] == '=') - trace_output = &optarg[1]; - else - trace_output = &optarg[0]; - } else if (optind < argc && argv[optind][0] != '-') - trace_output = argv[optind]; - else + trace_output = parse_optional_arg(argc, argv); + if (!trace_output) trace_output = "osnoise_trace.txt"; break; case 'T': diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 1b505531ad3b..961aa969c003 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -701,9 +701,9 @@ static void timerlat_hist_usage(void) char *msg[] = { "", " usage: [rtla] timerlat hist [-h] [-q] [-d s] [-D] [-n] [-a us] [-p us] [-i us] [-T us] [-s us] \\", - " [-t[file]] [-e sys[:event]] [--filter ] [--trigger ] [-c cpu-list] [-H cpu-list]\\", + " [-t [file]] [-e sys[:event]] [--filter ] [--trigger ] [-c cpu-list] [-H cpu-list]\\", " [-P priority] [-E N] [-b N] [--no-irq] [--no-thread] [--no-header] [--no-summary] \\", - " [--no-index] [--with-zeros] [--dma-latency us] [-C[=cgroup_name]] [--no-aa] [--dump-task] [-u|-k]", + " [--no-index] [--with-zeros] [--dma-latency us] [-C [cgroup_name]] [--no-aa] [--dump-task] [-u|-k]", " [--warm-up s] [--deepest-idle-state n]", "", " -h/--help: print this menu", @@ -714,11 +714,11 @@ static void timerlat_hist_usage(void) " -s/--stack us: save the stack trace at the IRQ if a thread latency is higher than the argument in us", " -c/--cpus cpus: run the tracer only on the given cpus", " -H/--house-keeping cpus: run rtla control threads only on the given cpus", - " -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", + " -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", " -d/--duration time[m|h|d]: duration of the session in seconds", " --dump-tasks: prints the task running on all CPUs if stop conditions are met (depends on !--no-aa)", " -D/--debug: print debug info", - " -t/--trace[file]: save the stopped trace to [file|timerlat_trace.txt]", + " -t/--trace [file]: save the stopped trace to [file|timerlat_trace.txt]", " -e/--event : enable the in the trace instance, multiple -e are allowed", " --filter : enable a trace event filter to the previous -e event", " --trigger : enable a trace event trigger to the previous -e event", @@ -864,13 +864,7 @@ static struct common_params break; case 'C': params->common.cgroup = 1; - if (!optarg) { - /* will inherit this cgroup */ - params->common.cgroup_name = NULL; - } else if (*optarg == '=') { - /* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name = parse_optional_arg(argc, argv); break; case 'b': params->common.hist.bucket_size = get_llong_from_str(optarg); @@ -939,14 +933,8 @@ static struct common_params params->common.stop_total_us = get_llong_from_str(optarg); break; case 't': - if (optarg) { - if (optarg[0] == '=') - trace_output = &optarg[1]; - else - trace_output = &optarg[0]; - } else if (optind < argc && argv[optind][0] != '-') - trace_output = argv[optind]; - else + trace_output = parse_optional_arg(argc, argv); + if (!trace_output) trace_output = "timerlat_trace.txt"; break; case 'u': diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 67a5b6ab78a6..213356a5be52 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -481,8 +481,8 @@ static void timerlat_top_usage(void) static const char *const msg[] = { "", " usage: rtla timerlat [top] [-h] [-q] [-a us] [-d s] [-D] [-n] [-p us] [-i us] [-T us] [-s us] \\", - " [[-t[file]] [-e sys[:event]] [--filter ] [--trigger ] [-c cpu-list] [-H cpu-list]\\", - " [-P priority] [--dma-latency us] [--aa-only us] [-C[=cgroup_name]] [-u|-k] [--warm-up s] [--deepest-idle-state n]", + " [[-t [file]] [-e sys[:event]] [--filter ] [--trigger ] [-c cpu-list] [-H cpu-list]\\", + " [-P priority] [--dma-latency us] [--aa-only us] [-C [cgroup_name]] [-u|-k] [--warm-up s] [--deepest-idle-state n]", "", " -h/--help: print this menu", " -a/--auto: set automatic trace mode, stopping the session if argument in us latency is hit", @@ -493,11 +493,11 @@ static void timerlat_top_usage(void) " -s/--stack us: save the stack trace at the IRQ if a thread latency is higher than the argument in us", " -c/--cpus cpus: run the tracer only on the given cpus", " -H/--house-keeping cpus: run rtla control threads only on the given cpus", - " -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", + " -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited", " -d/--duration time[s|m|h|d]: duration of the session", " -D/--debug: print debug info", " --dump-tasks: prints the task running on all CPUs if stop conditions are met (depends on !--no-aa)", - " -t/--trace[file]: save the stopped trace to [file|timerlat_trace.txt]", + " -t/--trace [file]: save the stopped trace to [file|timerlat_trace.txt]", " -e/--event : enable the in the trace instance, multiple -e are allowed", " --filter : enable a trace event filter to the previous -e event", " --trigger : enable a trace event trigger to the previous -e event", @@ -642,13 +642,7 @@ static struct common_params break; case 'C': params->common.cgroup = 1; - if (!optarg) { - /* will inherit this cgroup */ - params->common.cgroup_name = NULL; - } else if (*optarg == '=') { - /* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name = optarg; break; case 'D': config_debug = 1; @@ -707,14 +701,8 @@ static struct common_params params->common.stop_total_us = get_llong_from_str(optarg); break; case 't': - if (optarg) { - if (optarg[0] == '=') - trace_output = &optarg[1]; - else - trace_output = &optarg[0]; - } else if (optind < argc && argv[optind][0] != '-') - trace_output = argv[optind]; - else + trace_output = parse_optional_arg(argc, argv); + if (!trace_output) trace_output = "timerlat_trace.txt"; break; case 'u': diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c index 54334c676a22..9cf5a0098e9a 100644 --- a/tools/tracing/rtla/src/utils.c +++ b/tools/tracing/rtla/src/utils.c @@ -974,3 +974,29 @@ int auto_house_keeping(cpu_set_t *monitored_cpus) return 1; } + +/** + * parse_optional_arg - Parse optional argument value + * + * Parse optional argument value, which can be in the form of: + * -sarg, -s/--long=arg, -s/--long arg + * + * Returns arg value if found, NULL otherwise. + */ +char *parse_optional_arg(int argc, char **argv) +{ + if (optarg) { + if (optarg[0] == '=') { + /* skip the = */ + return &optarg[1]; + } else { + return optarg; + } + /* parse argument of form -s [arg] and --long [arg]*/ + } else if (optind < argc && argv[optind][0] != '-') { + /* consume optind */ + return argv[optind++]; + } else { + return NULL; + } +} diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h index 1be095f9a7e6..091df4ba4587 100644 --- a/tools/tracing/rtla/src/utils.h +++ b/tools/tracing/rtla/src/utils.h @@ -25,6 +25,7 @@ long parse_seconds_duration(char *val); void get_duration(time_t start_time, char *output, int output_size); int parse_cpu_list(char *cpu_list, char **monitored_cpus); +char *parse_optional_arg(int argc, char **argv); long long get_llong_from_str(char *start); static inline void From ddb6e42494e5c48c17e64f29b7674b9add486a19 Mon Sep 17 00:00:00 2001 From: Ivan Pravdin Date: Mon, 3 Nov 2025 11:19:08 -0500 Subject: [PATCH 15/17] rtla: Fix -a overriding -t argument When running rtla as `rtla -t custom_file.txt -a 100` -a options override trace output filename specified by -t option. Running the command above will create _trace.txt file instead of custom_file.txt. Fix this by making sure that -a option does not override trace output filename even if it's passed after trace output filename is specified. Fixes: 173a3b014827 ("rtla/timerlat: Add the automatic trace option") Signed-off-by: Ivan Pravdin Reviewed-by: Tomas Glozar Link: https://lore.kernel.org/r/b6ae60424050b2c1c8709e18759adead6012b971.1762186418.git.ipravdin.official@gmail.com [ use capital letter in subject, as required by tracing subsystem ] Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/osnoise_hist.c | 3 ++- tools/tracing/rtla/src/osnoise_top.c | 3 ++- tools/tracing/rtla/src/timerlat_hist.c | 3 ++- tools/tracing/rtla/src/timerlat_top.c | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index 1d06ea47b663..ff8c231e47c4 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -534,7 +534,8 @@ static struct common_params params->threshold = 1; /* set trace */ - trace_output = "osnoise_trace.txt"; + if (!trace_output) + trace_output = "osnoise_trace.txt"; break; case 'b': diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index cac60e4c267e..04c699bdd736 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -386,7 +386,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) params->threshold = 1; /* set trace */ - trace_output = "osnoise_trace.txt"; + if (!trace_output) + trace_output = "osnoise_trace.txt"; break; case 'c': diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 961aa969c003..1fb471a787b7 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -853,7 +853,8 @@ static struct common_params params->print_stack = auto_thresh; /* set trace */ - trace_output = "timerlat_trace.txt"; + if (!trace_output) + trace_output = "timerlat_trace.txt"; break; case 'c': diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 213356a5be52..fec113ce370e 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -617,7 +617,8 @@ static struct common_params params->print_stack = auto_thresh; /* set trace */ - trace_output = "timerlat_trace.txt"; + if (!trace_output) + trace_output = "timerlat_trace.txt"; break; case '5': From 61f1fd5d69fde27cfc277d1f68a1e6e4f7265b4b Mon Sep 17 00:00:00 2001 From: Crystal Wood Date: Wed, 12 Nov 2025 09:25:25 -0600 Subject: [PATCH 16/17] rtla/tests: Don't rely on matching ^1ALL The timerlat "top stop at failed action" test was relying on "ALL" being printed immediately after the "1" from the threshold action. Besides being fragile, this depends on stdbuf behavior, which is easy to miss when recreating the test outside of the framework for debugging purposes. Instead, use the expected/unexpected text mechanism from the corresponding osnoise test. Signed-off-by: Crystal Wood Link: https://lore.kernel.org/r/20251112152529.956778-2-crwood@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/tests/timerlat.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t index b550a6ae2445..bbaa1897d8a8 100644 --- a/tools/tracing/rtla/tests/timerlat.t +++ b/tools/tracing/rtla/tests/timerlat.t @@ -58,7 +58,7 @@ check "multiple actions" \ check "hist stop at failed action" \ "timerlat hist -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1# RTLA timerlat histogram$" check "top stop at failed action" \ - "timerlat top -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1ALL" + "timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh" check "hist with continue" \ "timerlat hist -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$" check "top with continue" \ From 3138df6f0cd04a75f8efa5b5270ba56d00a84ae6 Mon Sep 17 00:00:00 2001 From: Crystal Wood Date: Wed, 12 Nov 2025 09:25:28 -0600 Subject: [PATCH 17/17] rtla/timerlat: Exit top main loop on any non-zero wait_retval Comparing to exactly 1 will fail if more than one ring buffer event was seen since the last call to timerlat_bpf_wait(), which can happen in some race scenarios. Signed-off-by: Crystal Wood Link: https://lore.kernel.org/r/20251112152529.956778-5-crwood@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/timerlat_top.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index fec113ce370e..29c2c1f717ed 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -878,7 +878,7 @@ timerlat_top_bpf_main_loop(struct osnoise_tool *tool) if (!params->common.quiet) timerlat_print_stats(tool); - if (wait_retval == 1) { + if (wait_retval != 0) { /* Stopping requested by tracer */ actions_perform(¶ms->common.threshold_actions);