Commit 842fc5b8 authored by John Kacur's avatar John Kacur Committed by Daniel Bristot de Oliveira

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 <bristot@kernel.org>
Signed-off-by: default avatarJohn Kacur <jkacur@redhat.com>
Signed-off-by: default avatarDaniel Bristot de Oliveira <bristot@kernel.org>
parent 01b05fc0
...@@ -437,7 +437,7 @@ static void osnoise_hist_usage(char *usage) ...@@ -437,7 +437,7 @@ static void osnoise_hist_usage(char *usage)
static const char * const msg[] = { static const char * const msg[] = {
"", "",
" usage: rtla osnoise hist [-h] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\", " 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 <filter>] [--trigger <trigger>] \\", " [-T us] [-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\",
" [-c cpu-list] [-H cpu-list] [-P priority] [-b N] [-E N] [--no-header] [--no-summary] \\", " [-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]",
"", "",
...@@ -453,7 +453,7 @@ static void osnoise_hist_usage(char *usage) ...@@ -453,7 +453,7 @@ static void osnoise_hist_usage(char *usage)
" -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/--duration time[s|m|h|d]: duration of the session",
" -D/--debug: print debug info", " -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 <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed", " -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed",
" --filter <filter>: enable a trace event filter to the previous -e event", " --filter <filter>: enable a trace event filter to the previous -e event",
" --trigger <trigger>: enable a trace event trigger to the previous -e event", " --trigger <trigger>: enable a trace event trigger to the previous -e event",
...@@ -645,9 +645,13 @@ static struct osnoise_hist_params ...@@ -645,9 +645,13 @@ static struct osnoise_hist_params
params->threshold = get_llong_from_str(optarg); params->threshold = get_llong_from_str(optarg);
break; break;
case 't': case 't':
if (optarg) if (optarg) {
/* skip = */ if (optarg[0] == '=')
params->trace_output = &optarg[1]; params->trace_output = &optarg[1];
else
params->trace_output = &optarg[0];
} else if (optind < argc && argv[optind][0] != '0')
params->trace_output = argv[optind];
else else
params->trace_output = "osnoise_trace.txt"; params->trace_output = "osnoise_trace.txt";
break; break;
......
...@@ -283,7 +283,7 @@ static void osnoise_top_usage(struct osnoise_top_params *params, char *usage) ...@@ -283,7 +283,7 @@ static void osnoise_top_usage(struct osnoise_top_params *params, char *usage)
static const char * const msg[] = { static const char * const msg[] = {
" [-h] [-q] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\", " [-h] [-q] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\",
" [-T us] [-t[=file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\", " [-T us] [-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\",
" [-c cpu-list] [-H cpu-list] [-P priority] [-C[=cgroup_name]] [--warm-up s]", " [-c cpu-list] [-H cpu-list] [-P priority] [-C[=cgroup_name]] [--warm-up s]",
"", "",
" -h/--help: print this menu", " -h/--help: print this menu",
...@@ -298,7 +298,7 @@ static void osnoise_top_usage(struct osnoise_top_params *params, char *usage) ...@@ -298,7 +298,7 @@ static void osnoise_top_usage(struct osnoise_top_params *params, char *usage)
" -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/--duration time[s|m|h|d]: duration of the session",
" -D/--debug: print debug info", " -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 <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed", " -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed",
" --filter <filter>: enable a trace event filter to the previous -e event", " --filter <filter>: enable a trace event filter to the previous -e event",
" --trigger <trigger>: enable a trace event trigger to the previous -e event", " --trigger <trigger>: enable a trace event trigger to the previous -e event",
...@@ -486,9 +486,13 @@ struct osnoise_top_params *osnoise_top_parse_args(int argc, char **argv) ...@@ -486,9 +486,13 @@ struct osnoise_top_params *osnoise_top_parse_args(int argc, char **argv)
params->stop_total_us = get_llong_from_str(optarg); params->stop_total_us = get_llong_from_str(optarg);
break; break;
case 't': case 't':
if (optarg) if (optarg) {
/* skip = */ if (optarg[0] == '=')
params->trace_output = &optarg[1]; params->trace_output = &optarg[1];
else
params->trace_output = &optarg[0];
} else if (optind < argc && argv[optind][0] != '-')
params->trace_output = argv[optind];
else else
params->trace_output = "osnoise_trace.txt"; params->trace_output = "osnoise_trace.txt";
break; break;
......
...@@ -652,7 +652,7 @@ static void timerlat_hist_usage(char *usage) ...@@ -652,7 +652,7 @@ static void timerlat_hist_usage(char *usage)
char *msg[] = { char *msg[] = {
"", "",
" usage: [rtla] timerlat hist [-h] [-q] [-d s] [-D] [-n] [-a us] [-p us] [-i us] [-T us] [-s us] \\", " 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 <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\", " [-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\",
" [-P priority] [-E N] [-b N] [--no-irq] [--no-thread] [--no-header] [--no-summary] \\", " [-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]", " [--warm-up s]",
...@@ -669,7 +669,7 @@ static void timerlat_hist_usage(char *usage) ...@@ -669,7 +669,7 @@ static void timerlat_hist_usage(char *usage)
" -d/--duration time[m|h|d]: duration of the session in seconds", " -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)", " --dump-tasks: prints the task running on all CPUs if stop conditions are met (depends on !--no-aa)",
" -D/--debug: print debug info", " -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 <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed", " -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed",
" --filter <filter>: enable a trace event filter to the previous -e event", " --filter <filter>: enable a trace event filter to the previous -e event",
" --trigger <trigger>: enable a trace event trigger to the previous -e event", " --trigger <trigger>: enable a trace event trigger to the previous -e event",
...@@ -885,9 +885,13 @@ static struct timerlat_hist_params ...@@ -885,9 +885,13 @@ static struct timerlat_hist_params
params->stop_total_us = get_llong_from_str(optarg); params->stop_total_us = get_llong_from_str(optarg);
break; break;
case 't': case 't':
if (optarg) if (optarg) {
/* skip = */ if (optarg[0] == '=')
params->trace_output = &optarg[1]; params->trace_output = &optarg[1];
else
params->trace_output = &optarg[0];
} else if (optind < argc && argv[optind][0] != '-')
params->trace_output = argv[optind];
else else
params->trace_output = "timerlat_trace.txt"; params->trace_output = "timerlat_trace.txt";
break; break;
......
...@@ -446,7 +446,7 @@ static void timerlat_top_usage(char *usage) ...@@ -446,7 +446,7 @@ static void timerlat_top_usage(char *usage)
static const char *const msg[] = { 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] \\", " 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 <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\", " [[-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\",
" [-P priority] [--dma-latency us] [--aa-only us] [-C[=cgroup_name]] [-u|-k] [--warm-up s]", " [-P priority] [--dma-latency us] [--aa-only us] [-C[=cgroup_name]] [-u|-k] [--warm-up s]",
"", "",
" -h/--help: print this menu", " -h/--help: print this menu",
...@@ -462,7 +462,7 @@ static void timerlat_top_usage(char *usage) ...@@ -462,7 +462,7 @@ static void timerlat_top_usage(char *usage)
" -d/--duration time[m|h|d]: duration of the session in seconds", " -d/--duration time[m|h|d]: duration of the session in seconds",
" -D/--debug: print debug info", " -D/--debug: print debug info",
" --dump-tasks: prints the task running on all CPUs if stop conditions are met (depends on !--no-aa)", " --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 <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed", " -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed",
" --filter <command>: enable a trace event filter to the previous -e event", " --filter <command>: enable a trace event filter to the previous -e event",
" --trigger <command>: enable a trace event trigger to the previous -e event", " --trigger <command>: enable a trace event trigger to the previous -e event",
...@@ -668,9 +668,13 @@ static struct timerlat_top_params ...@@ -668,9 +668,13 @@ static struct timerlat_top_params
params->stop_total_us = get_llong_from_str(optarg); params->stop_total_us = get_llong_from_str(optarg);
break; break;
case 't': case 't':
if (optarg) if (optarg) {
/* skip = */ if (optarg[0] == '=')
params->trace_output = &optarg[1]; params->trace_output = &optarg[1];
else
params->trace_output = &optarg[0];
} else if (optind < argc && argv[optind][0] != '-')
params->trace_output = argv[optind];
else else
params->trace_output = "timerlat_trace.txt"; params->trace_output = "timerlat_trace.txt";
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment