The timerlat hist command fails to parse short options with attached
numeric arguments (e.g., -p100) due to conflicts between digit characters
used as option values and numeric arguments to other options.
This issue was discovered when testing rtla 7.1.0-rc6 with rteval,
which passes arguments in the compact -p100 format. The rteval tests
failed with the confusing error "no-irq and no-thread set, there is
nothing to do here" even though neither option was specified.
The root cause is two-fold:
1. Digit characters ('0'-'9') were used as short option values for
long-only options like --no-irq, --no-thread, etc. This caused
getopt_auto() to generate an option string like 'a:b:...:u0123456:7:8:9'
which made getopt treat digits as valid option characters.
2. The two-phase option parsing approach (alternating calls between
common_parse_options() and local option parsing) confused getopt's
internal state when encountering arguments like -p100.
When a user passed -p100, getopt would incorrectly parse it as three
separate options: -p, -1, -0, and -0, silently setting no_irq and
no_thread flags instead of recognizing "100" as the period argument.
The two-phase parsing was introduced in commit 850cd24cb6d6 ("tools/rtla:
Add common_parse_options()") which first appeared in v7.0-rc1. Prior to
that commit, -p100 worked correctly. The digit characters as option
values existed since the original timerlat implementation, but only
became problematic when combined with the two-phase parsing approach.
Fix this by:
1. Eliminating digit characters from the option string by filtering them
out in getopt_auto(). This prevents conflicts with numeric arguments.
2. Refactoring timerlat_hist_parse_args() to use single-pass option
parsing. Instead of alternating between common_parse_options() and
local parsing, merge all options (common and local) into a single
option table and parse them in one pass. This matches the approach
used by cyclictest and other tools.
With these changes, all argument formats work correctly:
-p 100 (short with space)
-p100 (short without space)
--period=100 (long with =)
--period 100 (long with space)
This maintains compatibility with existing usage while enabling the
compact -p100 format that users expect from similar tools.
Assisted-by: Claude:claude-sonnet-4-5
Signed-off-by: John Kacur <[email protected]>
---
tools/tracing/rtla/src/common.c | 4 ++
tools/tracing/rtla/src/timerlat_hist.c | 55 ++++++++++++++++++++++++--
2 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index 35e3d3aa922e..c2fd051c562c 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -65,6 +65,10 @@ int getopt_auto(int argc, char **argv, const struct option
*long_opts)
if (long_opts[i].val < 32 || long_opts[i].val > 127)
continue;
+ /* Skip digit characters to avoid conflicts with numeric
arguments */
+ if (long_opts[i].val >= '0' && long_opts[i].val <= '9')
+ continue;
+
if (n + 4 >= sizeof(opts))
fatal("optstring buffer overflow");
diff --git a/tools/tracing/rtla/src/timerlat_hist.c
b/tools/tracing/rtla/src/timerlat_hist.c
index 79142af4f566..c0b6d7c30114 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -787,11 +787,24 @@ static struct common_params
static struct option long_options[] = {
{"auto", required_argument, 0, 'a'},
{"bucket-size", required_argument, 0, 'b'},
+ /* Common options */
+ {"cpus", required_argument, 0, 'c'},
+ {"cgroup", optional_argument, 0, 'C'},
+ {"debug", no_argument, 0, 'D'},
+ {"duration", required_argument, 0, 'd'},
+ {"event", required_argument, 0, 'e'},
+ /* End common options */
{"entries", required_argument, 0, 'E'},
{"help", no_argument, 0, 'h'},
+ /* Common option */
+ {"house-keeping", required_argument, 0, 'H'},
+ /* End common option */
{"irq", required_argument, 0, 'i'},
{"nano", no_argument, 0, 'n'},
{"period", required_argument, 0, 'p'},
+ /* Common option */
+ {"priority", required_argument, 0, 'P'},
+ /* End common option */
{"stack", required_argument, 0, 's'},
{"thread", required_argument, 0, 'T'},
{"trace", optional_argument, 0, 't'},
@@ -819,9 +832,6 @@ static struct common_params
{0, 0, 0, 0}
};
- if (common_parse_options(argc, argv, ¶ms->common))
- continue;
-
c = getopt_auto(argc, argv, long_options);
/* detect the end of the options. */
@@ -850,6 +860,35 @@ static struct common_params
params->common.hist.bucket_size >= 1000000)
fatal("Bucket size needs to be > 0 and <=
1000000");
break;
+ case 'c':
+ if (parse_cpu_set(optarg,
¶ms->common.monitored_cpus))
+ fatal("Invalid -c cpu list");
+ params->common.cpus = optarg;
+ break;
+ case 'C':
+ params->common.cgroup = 1;
+ params->common.cgroup_name = parse_optional_arg(argc,
argv);
+ break;
+ case 'D':
+ config_debug = 1;
+ break;
+ case 'd':
+ params->common.duration =
parse_seconds_duration(optarg);
+ if (!params->common.duration)
+ fatal("Invalid -d duration");
+ break;
+ case 'e':
+ {
+ struct trace_events *tevent;
+ tevent = trace_event_alloc(optarg);
+ if (!tevent)
+ fatal("Error alloc trace event");
+
+ if (params->common.events)
+ tevent->next = params->common.events;
+ params->common.events = tevent;
+ }
+ break;
case 'E':
params->common.hist.entries =
get_llong_from_str(optarg);
if (params->common.hist.entries < 10 ||
@@ -860,6 +899,11 @@ static struct common_params
case '?':
timerlat_hist_usage();
break;
+ case 'H':
+ params->common.hk_cpus = 1;
+ if (parse_cpu_set(optarg, ¶ms->common.hk_cpu_set))
+ fatal("Error parsing house keeping CPUs");
+ break;
case 'i':
params->common.stop_us = get_llong_from_str(optarg);
break;
@@ -874,6 +918,11 @@ static struct common_params
if (params->timerlat_period_us > 1000000)
fatal("Period longer than 1 s");
break;
+ case 'P':
+ if (parse_prio(optarg, ¶ms->common.sched_param) ==
-1)
+ fatal("Invalid -P priority");
+ params->common.set_sched = 1;
+ break;
case 's':
params->print_stack = get_llong_from_str(optarg);
break;
--
2.54.0
