From 1462501c7a8d565f5949d3d5635b2111d889aaaa Mon Sep 17 00:00:00 2001 From: Daniel Bristot de Oliveira Date: Wed, 24 Apr 2024 16:36:54 +0200 Subject: rtla/timerlat: Add a summary for hist mode Like on rtla timerlat top, add an overall summary at the bottom of timerlat hist. For instance: # timerlat hist -c 0-1 -d 10s -E 20 # RTLA timerlat histogram # Time unit is microseconds (us) # Duration: 0 00:00:10 Index IRQ-000 Thr-000 IRQ-001 Thr-001 6 1 0 0 0 7 1 0 0 0 8 1 0 1 0 9 7 0 0 0 10 16 0 0 0 11 1 0 3 0 15 0 0 3 0 16 0 0 12 0 17 0 0 28 0 18 0 2 26 0 19 1 1 80 1 over: 9973 9998 9848 10000 count: 10001 10001 10001 10001 min: 6 18 8 19 avg: 185 204 95 113 max: 428 450 341 371 ALL: IRQ Thr count: 20002 20002 min: 6 18 avg: 140 159 max: 428 450 Link: https://lkml.kernel.org/r/a6bc06c798f72127edc57d1f99da8d57e1187cee.1713968967.git.bristot@kernel.org Cc: Jonathan Corbet Suggested-by: Juri Lelli Signed-off-by: Daniel Bristot de Oliveira --- tools/tracing/rtla/src/timerlat_hist.c | 130 ++++++++++++++++++++++++++++++++- 1 file changed, 129 insertions(+), 1 deletion(-) (limited to 'tools/tracing/rtla/src/timerlat_hist.c') diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 8bd51aab6513..b12c6b571dd4 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -401,8 +401,135 @@ timerlat_print_summary(struct timerlat_hist_params *params, trace_seq_reset(trace->seq); } +static void +timerlat_print_stats_all(struct timerlat_hist_params *params, + struct trace_instance *trace, + struct timerlat_hist_data *data) +{ + struct timerlat_hist_cpu *cpu_data; + struct timerlat_hist_cpu sum; + int cpu; + + if (params->no_summary) + return; + + memset(&sum, 0, sizeof(sum)); + sum.min_irq = ~0; + sum.min_thread = ~0; + sum.min_user = ~0; + + for (cpu = 0; cpu < data->nr_cpus; cpu++) { + if (params->cpus && !CPU_ISSET(cpu, ¶ms->monitored_cpus)) + continue; + + if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) + continue; + + cpu_data = &data->hist[cpu]; + + sum.irq_count += cpu_data->irq_count; + update_min(&sum.min_irq, &cpu_data->min_irq); + update_sum(&sum.sum_irq, &cpu_data->sum_irq); + update_max(&sum.max_irq, &cpu_data->max_irq); + + sum.thread_count += cpu_data->thread_count; + update_min(&sum.min_thread, &cpu_data->min_thread); + update_sum(&sum.sum_thread, &cpu_data->sum_thread); + update_max(&sum.max_thread, &cpu_data->max_thread); + + sum.user_count += cpu_data->user_count; + update_min(&sum.min_user, &cpu_data->min_user); + update_sum(&sum.sum_user, &cpu_data->sum_user); + update_max(&sum.max_user, &cpu_data->max_user); + } + + if (!params->no_index) + trace_seq_printf(trace->seq, "ALL: "); + + if (!params->no_irq) + trace_seq_printf(trace->seq, " IRQ"); + + if (!params->no_thread) + trace_seq_printf(trace->seq, " Thr"); + + if (params->user_hist) + trace_seq_printf(trace->seq, " Usr"); + + trace_seq_printf(trace->seq, "\n"); + + if (!params->no_index) + trace_seq_printf(trace->seq, "count:"); + + if (!params->no_irq) + trace_seq_printf(trace->seq, "%9d ", + sum.irq_count); + + if (!params->no_thread) + trace_seq_printf(trace->seq, "%9d ", + sum.thread_count); + + if (params->user_hist) + trace_seq_printf(trace->seq, "%9d ", + sum.user_count); + + trace_seq_printf(trace->seq, "\n"); + + if (!params->no_index) + trace_seq_printf(trace->seq, "min: "); + + if (!params->no_irq) + trace_seq_printf(trace->seq, "%9llu ", + sum.min_irq); + + if (!params->no_thread) + trace_seq_printf(trace->seq, "%9llu ", + sum.min_thread); + + if (params->user_hist) + trace_seq_printf(trace->seq, "%9llu ", + sum.min_user); + + trace_seq_printf(trace->seq, "\n"); + + if (!params->no_index) + trace_seq_printf(trace->seq, "avg: "); + + if (!params->no_irq) + trace_seq_printf(trace->seq, "%9llu ", + sum.sum_irq / sum.irq_count); + + if (!params->no_thread) + trace_seq_printf(trace->seq, "%9llu ", + sum.sum_thread / sum.thread_count); + + if (params->user_hist) + trace_seq_printf(trace->seq, "%9llu ", + sum.sum_user / sum.user_count); + + trace_seq_printf(trace->seq, "\n"); + + if (!params->no_index) + trace_seq_printf(trace->seq, "max: "); + + if (!params->no_irq) + trace_seq_printf(trace->seq, "%9llu ", + sum.max_irq); + + if (!params->no_thread) + trace_seq_printf(trace->seq, "%9llu ", + sum.max_thread); + + if (params->user_hist) + trace_seq_printf(trace->seq, "%9llu ", + sum.max_user); + + trace_seq_printf(trace->seq, "\n"); + trace_seq_do_printf(trace->seq); + trace_seq_reset(trace->seq); +} + /* - * timerlat_print_stats - print data for all CPUs + * timerlat_print_stats - print data for each CPUs */ static void timerlat_print_stats(struct timerlat_hist_params *params, struct osnoise_tool *tool) @@ -485,6 +612,7 @@ timerlat_print_stats(struct timerlat_hist_params *params, struct osnoise_tool *t trace_seq_reset(trace->seq); timerlat_print_summary(params, trace, data); + timerlat_print_stats_all(params, trace, data); } /* -- cgit v1.2.3 From cdbf71962bb07493d67fee34536a5724a8bb5886 Mon Sep 17 00:00:00 2001 From: Daniel Bristot de Oliveira Date: Wed, 24 Apr 2024 16:36:55 +0200 Subject: rtla: Add the --warm-up option On many cases, the results right after the startup are different from the rest of the execution, biasing the results. For example, on osnoise, the scheduler might take some time to adapt to the new busy-loop workload. Add the --warm-up option, adding a warm-up phase (in seconds) where the workload is set, but the results are discarded. Link: https://lkml.kernel.org/r/e682d5ce5af90f123bd13220f63d5c3d118a92be.1713968967.git.bristot@kernel.org Cc: Jonathan Corbet Cc: Juri Lelli Signed-off-by: Daniel Bristot de Oliveira --- tools/tracing/rtla/src/timerlat_hist.c | 49 ++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 17 deletions(-) (limited to 'tools/tracing/rtla/src/timerlat_hist.c') diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index b12c6b571dd4..da1c353bdac9 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -52,6 +52,7 @@ struct timerlat_hist_params { char with_zeros; int bucket_size; int entries; + int warmup; }; struct timerlat_hist_cpu { @@ -628,6 +629,7 @@ static void timerlat_hist_usage(char *usage) " [-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]", + " [--warm-up s]", "", " -h/--help: print this menu", " -a/--auto: set automatic trace mode, stopping the session if argument in us latency is hit", @@ -664,6 +666,7 @@ static void timerlat_hist_usage(char *usage) " in nanoseconds", " -u/--user-threads: use rtla user-space threads instead of in-kernel timerlat threads", " -U/--user-load: enable timerlat for user-defined user-space workload", + " --warm-up s: let the workload run for s seconds before collecting data", NULL, }; @@ -738,13 +741,14 @@ static struct timerlat_hist_params {"dma-latency", required_argument, 0, '8'}, {"no-aa", no_argument, 0, '9'}, {"dump-task", no_argument, 0, '\1'}, + {"warm-up", required_argument, 0, '\2'}, {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:np:P:s:t::T:uU0123456:7:8:9\1", + c = getopt_long(argc, argv, "a:c:C::b:d:e:E:DhH:i:np:P:s:t::T:uU0123456:7:8:9\1\2:", long_options, &option_index); /* detect the end of the options. */ @@ -913,6 +917,9 @@ static struct timerlat_hist_params case '\1': params->dump_tasks = 1; break; + case '\2': + params->warmup = get_llong_from_str(optarg); + break; default: timerlat_hist_usage("Invalid option"); } @@ -1167,22 +1174,6 @@ int timerlat_hist_main(int argc, char *argv[]) } } - /* - * Start the tracers here, after having set all instances. - * - * Let the trace instance start first for the case of hitting a stop - * tracing while enabling other instances. The trace instance is the - * one with most valuable information. - */ - if (params->trace_output) - trace_instance_start(&record->trace); - if (!params->no_aa) - trace_instance_start(&aa->trace); - trace_instance_start(trace); - - tool->start_time = time(NULL); - timerlat_hist_set_signals(params); - if (params->user_workload) { /* rtla asked to stop */ params_u.should_run = 1; @@ -1202,6 +1193,29 @@ int timerlat_hist_main(int argc, char *argv[]) err_msg("Error creating timerlat user-space threads\n"); } + if (params->warmup > 0) { + debug_msg("Warming up for %d seconds\n", params->warmup); + sleep(params->warmup); + if (stop_tracing) + goto out_hist; + } + + /* + * Start the tracers here, after having set all instances. + * + * Let the trace instance start first for the case of hitting a stop + * tracing while enabling other instances. The trace instance is the + * one with most valuable information. + */ + if (params->trace_output) + trace_instance_start(&record->trace); + if (!params->no_aa) + trace_instance_start(&aa->trace); + trace_instance_start(trace); + + tool->start_time = time(NULL); + timerlat_hist_set_signals(params); + while (!stop_tracing) { sleep(params->sleep_time); @@ -1227,6 +1241,7 @@ int timerlat_hist_main(int argc, char *argv[]) } } } + if (params->user_workload && !params_u.stopped_running) { params_u.should_run = 0; sleep(1); -- cgit v1.2.3 From fb9e90a67ee9a42779a8ea296a4cf7734258b27d Mon Sep 17 00:00:00 2001 From: Daniel Bristot de Oliveira Date: Wed, 24 Apr 2024 16:36:56 +0200 Subject: rtla/timerlat: Make user-space threads the default After ther -u addition, most of the known users are setting it. And it makes sense, as it adds more information, and inherits the default setup for the threads - e.g., cgroups configs. Thus, if the user-space interface is available, enable -u. Otherwise, use the in-kernel thread. Add the -k option to allow the user to request kernel-threads. Link: https://lkml.kernel.org/r/9241d3089de4091b124f780ed832a0e6646cadaa.1713968967.git.bristot@kernel.org Cc: Jonathan Corbet Cc: Juri Lelli Signed-off-by: Daniel Bristot de Oliveira --- tools/tracing/rtla/src/timerlat_hist.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) (limited to 'tools/tracing/rtla/src/timerlat_hist.c') diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index da1c353bdac9..6eb6e38d4a05 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -40,6 +40,7 @@ struct timerlat_hist_params { int no_aa; int dump_tasks; int user_workload; + int kernel_workload; int user_hist; cpu_set_t hk_cpu_set; struct sched_attr sched_param; @@ -628,7 +629,7 @@ static void timerlat_hist_usage(char *usage) " 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]\\", " [-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]", + " [--no-index] [--with-zeros] [--dma-latency us] [-C[=cgroup_name]] [--no-aa] [--dump-task] [-u|-k]", " [--warm-up s]", "", " -h/--help: print this menu", @@ -664,7 +665,8 @@ static void timerlat_hist_usage(char *usage) " 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", - " -u/--user-threads: use rtla user-space threads instead of in-kernel timerlat threads", + " -u/--user-threads: use rtla user-space threads instead of kernel-space timerlat threads", + " -k/--kernel-threads: use timerlat kernel-space threads instead of rtla user-space threads", " -U/--user-load: enable timerlat for user-defined user-space workload", " --warm-up s: let the workload run for s seconds before collecting data", NULL, @@ -728,6 +730,7 @@ static struct timerlat_hist_params {"thread", required_argument, 0, 'T'}, {"trace", optional_argument, 0, 't'}, {"user-threads", no_argument, 0, 'u'}, + {"kernel-threads", no_argument, 0, 'k'}, {"user-load", no_argument, 0, 'U'}, {"event", required_argument, 0, 'e'}, {"no-irq", no_argument, 0, '0'}, @@ -748,7 +751,7 @@ static struct timerlat_hist_params /* 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:np:P:s:t::T:uU0123456:7:8:9\1\2:", + 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:", long_options, &option_index); /* detect the end of the options. */ @@ -831,6 +834,9 @@ static struct timerlat_hist_params case 'i': params->stop_us = get_llong_from_str(optarg); break; + case 'k': + params->kernel_workload = 1; + break; case 'n': params->output_divisor = 1; break; @@ -942,6 +948,9 @@ static struct timerlat_hist_params if (!params->stop_us && !params->stop_total_us) params->no_aa = 1; + if (params->kernel_workload && params->user_workload) + timerlat_hist_usage("--kernel-threads and --user-threads are mutually exclusive!"); + return params; } @@ -1017,6 +1026,22 @@ timerlat_hist_apply_config(struct osnoise_tool *tool, struct timerlat_hist_param auto_house_keeping(¶ms->monitored_cpus); } + /* + * If the user did not specify a type of thread, try user-threads first. + * Fall back to kernel threads otherwise. + */ + if (!params->kernel_workload && !params->user_workload) { + retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd"); + if (retval) { + debug_msg("User-space interface detected, setting user-threads\n"); + params->user_workload = 1; + params->user_hist = 1; + } else { + debug_msg("User-space interface not detected, setting kernel-threads\n"); + params->kernel_workload = 1; + } + } + if (params->user_hist) { retval = osnoise_set_workload(tool->context, 0); if (retval) { -- cgit v1.2.3 From e9a4062e1527238c5649d0f4be794a8566fd77c9 Mon Sep 17 00:00:00 2001 From: Daniel Bristot de Oliveira Date: Thu, 16 May 2024 16:15:22 +0200 Subject: rtla: Add --trace-buffer-size option Add the option allow the users to set a different buffer size for the trace. For example, in large systems, the user might be interested on reducing the trace buffer to avoid large tracing files. The buffer size is specified in kB, and it is only affecting the tracing instance. The function trace_set_buffer_size() appears on libtracefs v1.6, so increase the minimum required version on Makefile.config. Link: https://lkml.kernel.org/r/e7c9ca5b3865f28e131a49ec3b984fadf2d056c6.1715860611.git.bristot@kernel.org Cc: Jonathan Corbet Cc: Juri Lelli Cc: John Kacur Signed-off-by: Daniel Bristot de Oliveira --- tools/tracing/rtla/src/timerlat_hist.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'tools/tracing/rtla/src/timerlat_hist.c') diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 6eb6e38d4a05..d4bab86ca1b9 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -54,6 +54,7 @@ struct timerlat_hist_params { int bucket_size; int entries; int warmup; + int buffer_size; }; struct timerlat_hist_cpu { @@ -669,6 +670,7 @@ static void timerlat_hist_usage(char *usage) " -k/--kernel-threads: use timerlat kernel-space threads instead of rtla user-space threads", " -U/--user-load: enable timerlat for user-defined user-space workload", " --warm-up s: let the workload run for s seconds before collecting data", + " --trace-buffer-size kB: set the per-cpu trace buffer size in kB", NULL, }; @@ -745,13 +747,14 @@ static struct timerlat_hist_params {"no-aa", no_argument, 0, '9'}, {"dump-task", no_argument, 0, '\1'}, {"warm-up", required_argument, 0, '\2'}, + {"trace-buffer-size", required_argument, 0, '\3'}, {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:", + 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); /* detect the end of the options. */ @@ -926,6 +929,9 @@ static struct timerlat_hist_params case '\2': params->warmup = get_llong_from_str(optarg); break; + case '\3': + params->buffer_size = get_llong_from_str(optarg); + break; default: timerlat_hist_usage("Invalid option"); } @@ -1179,6 +1185,12 @@ int timerlat_hist_main(int argc, char *argv[]) if (retval) goto out_hist; } + + if (params->buffer_size > 0) { + retval = trace_set_buffer_size(&record->trace, params->buffer_size); + if (retval) + goto out_hist; + } } if (!params->no_aa) { -- cgit v1.2.3 From 01b05fc0e5f3aec443a9a8ffa0022cbca2fd3608 Mon Sep 17 00:00:00 2001 From: John Kacur Date: Fri, 10 May 2024 15:03:18 -0400 Subject: rtla/timerlat: Fix histogram report when a cpu count is 0 On short runs it is possible to get no samples on a cpu, like this: # rtla timerlat hist -u -T50 Index IRQ-001 Thr-001 Usr-001 IRQ-002 Thr-002 Usr-002 2 1 0 0 0 0 0 33 0 1 0 0 0 0 36 0 0 1 0 0 0 49 0 0 0 1 0 0 52 0 0 0 0 1 0 over: 0 0 0 0 0 0 count: 1 1 1 1 1 0 min: 2 33 36 49 52 18446744073709551615 avg: 2 33 36 49 52 - max: 2 33 36 49 52 0 rtla timerlat hit stop tracing IRQ handler delay: (exit from idle) 48.21 us (91.09 %) IRQ latency: 49.11 us Timerlat IRQ duration: 2.17 us (4.09 %) Blocking thread: 1.01 us (1.90 %) swapper/2:0 1.01 us ------------------------------------------------------------------------ Thread latency: 52.93 us (100%) Max timerlat IRQ latency from idle: 49.11 us in cpu 2 Note, the value 18446744073709551615 is the same as ~0. Fix this by reporting no results for the min, avg and max if the count is 0. Link: https://lkml.kernel.org/r/20240510190318.44295-1-jkacur@redhat.com Cc: stable@vger.kernel.org Fixes: 1eeb6328e8b3 ("rtla/timerlat: Add timerlat hist mode") Suggested-by: Daniel Bristot de Oliveria Signed-off-by: John Kacur Signed-off-by: Daniel Bristot de Oliveira --- tools/tracing/rtla/src/timerlat_hist.c | 60 ++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 18 deletions(-) (limited to 'tools/tracing/rtla/src/timerlat_hist.c') diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index d4bab86ca1b9..fbe2c6549bf9 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -327,17 +327,29 @@ timerlat_print_summary(struct timerlat_hist_params *params, if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; - if (!params->no_irq) - trace_seq_printf(trace->seq, "%9llu ", - data->hist[cpu].min_irq); + if (!params->no_irq) { + if (data->hist[cpu].irq_count) + trace_seq_printf(trace->seq, "%9llu ", + data->hist[cpu].min_irq); + else + trace_seq_printf(trace->seq, " - "); + } - if (!params->no_thread) - trace_seq_printf(trace->seq, "%9llu ", - data->hist[cpu].min_thread); + if (!params->no_thread) { + if (data->hist[cpu].thread_count) + trace_seq_printf(trace->seq, "%9llu ", + data->hist[cpu].min_thread); + else + trace_seq_printf(trace->seq, " - "); + } - if (params->user_hist) - trace_seq_printf(trace->seq, "%9llu ", - data->hist[cpu].min_user); + if (params->user_hist) { + if (data->hist[cpu].user_count) + trace_seq_printf(trace->seq, "%9llu ", + data->hist[cpu].min_user); + else + trace_seq_printf(trace->seq, " - "); + } } trace_seq_printf(trace->seq, "\n"); @@ -387,17 +399,29 @@ timerlat_print_summary(struct timerlat_hist_params *params, if (!data->hist[cpu].irq_count && !data->hist[cpu].thread_count) continue; - if (!params->no_irq) - trace_seq_printf(trace->seq, "%9llu ", - data->hist[cpu].max_irq); + if (!params->no_irq) { + if (data->hist[cpu].irq_count) + trace_seq_printf(trace->seq, "%9llu ", + data->hist[cpu].max_irq); + else + trace_seq_printf(trace->seq, " - "); + } - if (!params->no_thread) - trace_seq_printf(trace->seq, "%9llu ", - data->hist[cpu].max_thread); + if (!params->no_thread) { + if (data->hist[cpu].thread_count) + trace_seq_printf(trace->seq, "%9llu ", + data->hist[cpu].max_thread); + else + trace_seq_printf(trace->seq, " - "); + } - if (params->user_hist) - trace_seq_printf(trace->seq, "%9llu ", - data->hist[cpu].max_user); + if (params->user_hist) { + if (data->hist[cpu].user_count) + trace_seq_printf(trace->seq, "%9llu ", + data->hist[cpu].max_user); + else + trace_seq_printf(trace->seq, " - "); + } } trace_seq_printf(trace->seq, "\n"); trace_seq_do_printf(trace->seq); -- cgit v1.2.3 From 842fc5b87a5058d475b9411dbb94ed49f8d6bce3 Mon Sep 17 00:00:00 2001 From: John Kacur Date: Wed, 15 May 2024 14:30:23 -0400 Subject: rtla: Fix -t\--trace[=file] The -t option has an optional argument. The usual case is for a short option to be specified without an '=' and for the long version to be specified with an '=' Various forms of this do not work as expected. For example: rtla timerlat hist -T50 -tfile.txt will result in a truncated file name of "ile.txt" Another example is that the long form without the '=' will result in the default file name instead of the requested file name. This patch properly parses the optional argument with and without '=' and with and without spaces for the short form. This patch was also tested using -t and --trace without providing a file name both as the last requested option and with a following long and short option. For example: rtla timerlat hist -T50 -t -u rtla timerlat hist -T50 --trace -u This fix is applied to both timerlat top and hist and to osnoise top and hist. Here is the full testing for rtla timerlat hist. Before applying the patch rtla timerlat hist -T50 -t=file.txt Works as expected, "file.txt" rtla timerlat hist -T50 -tfile.txt Truncated file name "ile.txt" rtla timerlat hist -T50 -t file.txt Default file name instead of file.txt rtla timerlat hist -T50 --trace=file.txt Truncated file name "ile.txt" rtla timerlat hist -T50 --trace file.txt Default file name "timerlat_trace.txt" instead of "file.txt" After applying the patch: rtla timerlat hist -T50 -t=file.txt Works as expected, "file.txt" rtla timerlat hist -T50 -tfile.txt Works as expected, "file.txt" rtla timerlat hist -T50 -t file.txt Works as expected, "file.txt" rtla timerlat hist -T50 --trace=file.txt Works as expected, "file.txt" rtla timerlat hist -T50 --trace file.txt Works as expected, "file.txt" In addition the following tests were performed to make sure that the default file name worked as expected including with trailing options. rtla timerlat hist -T50 -t Works as expected "timerlat_trace.txt" rtla timerlat hist -T50 --trace Works as expected "timerlat_trace.txt" rtla timerlat hist -T50 -t -u Works as expected "timerlat_trace.txt" rtla timerlat hist -T50 --trace -u Works as expected "timerlat_trace.txt" Link: https://lkml.kernel.org/r/20240515183024.59985-1-jkacur@redhat.com Cc: Daniel Bristot de Oliveria Signed-off-by: John Kacur Signed-off-by: Daniel Bristot de Oliveira --- tools/tracing/rtla/src/timerlat_hist.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'tools/tracing/rtla/src/timerlat_hist.c') diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index fbe2c6549bf9..a3907c390d67 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -652,7 +652,7 @@ static void timerlat_hist_usage(char *usage) 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]", " [--warm-up s]", @@ -669,7 +669,7 @@ static void timerlat_hist_usage(char *usage) " -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", @@ -885,9 +885,13 @@ static struct timerlat_hist_params params->stop_total_us = get_llong_from_str(optarg); break; case 't': - if (optarg) - /* skip = */ - params->trace_output = &optarg[1]; + if (optarg) { + if (optarg[0] == '=') + params->trace_output = &optarg[1]; + else + params->trace_output = &optarg[0]; + } else if (optind < argc && argv[optind][0] != '-') + params->trace_output = argv[optind]; else params->trace_output = "timerlat_trace.txt"; break; -- cgit v1.2.3