| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The current handling of invalid command line option arguments is
inconsistent:
- opt_llong_callback() treats non-numerical input the same as "-1",
which might or might not be rejected later.
- opt_int_callback() returns -1 on non-numerical input without an error
message, which makes parsing fail silently (libsubcmd will
automatically print the usage of the option only, no error message).
- custom callbacks abort command line parsing using fatal(), which
displays an error message and exits, without libsubcmd printing the
usage.
Unify this such that all invalid options, regardless of the format,
print an error message similar to the out of range case:
Error: --opt: 'value' is not a valid XY
followed by the usage of the option, e.g.:
$ rtla timerlat hist --period=1us
Error: --period: '1us' is not a valid number
Usage: rtla timerlat hist [<options>] [-h|--help]
-p, --period <us> timerlat period in us
As this is a libsubcmd help path, all option parsing failures now return
the exit code of 129 (help).
The unified handling is implemented using a new error message helper,
opt_err(), which is called from two new CLI-specific parsing functions,
strtoll_safe() and strtoi_safe(), as well as from custom helpers.
Option callback tests are updated to cover the new behavior.
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260716144901.1187474-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Several RTLA options do range validation inside the CLI parser layer
(e.g. -p/--period). When RTLA migrated CLI parsing to libsubcmd, this
logic was moved unchanged inside opt_*() callbacks.
Unify range validation so that all options use two newly added
functions, check_llong_range() and check_int_range(), to validate the
range.
The new range validation returns -1 from opt_*() callbacks rather than
hard-exit with fatal(), allowing the help message for the specific
option to be automatically displayed by libsubcmd logic.
Many options no longer need a custom callback, as they use the unified
range validation of opt_llong_callback() and opt_int_callback().
Validation for several other options is improved:
- timerlat -p/--period: lower bound raised from 1 to 100 us to match
the kernel's timerlat_min_period in trace_osnoise.c.
- timerlat -A/--aligned: reject negative values.
- timerlat --deepest-idle-state: add range [-1, INT_MAX]; previously,
values <= -2 were read as "option not set".
- timerlat -p/--period, -A/--aligned, -b/--bucket-size: properly reject
negative values instead of passing them to the tracer.
Remove unit tests for removed callbacks and test the new range
validation functionality of opt_llong_callback() and opt_int_callback().
Update runtime tests for histogram options to account for the new error
messages and exit value.
Assisted-by: Claude:claude-opus-4-6
Link: https://lore.kernel.org/r/20260710131554.338335-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
libsubcmd implicitly allows the user to unset already set options using
a "no-" prefix for long options. For example, if I set the period like
this:
$ rtla timerlat -D
Loading BPF program
reading osnoise/timerlat_period_us returned 1000
setting osnoise/timerlat_period_us to 1000
reading osnoise/print_stack returned 0
setting osnoise/print_stack to 0
...
<timerlat top>
it can be unset by a subsequent --no-debug:
$ rtla timerlat -D --no-debug
...
<timerlat top>
Currently, this works only for boolean options. Extend the feature for
all options by implementing handling of the "unset" argument in opt_*()
callbacks defined in cli_p.h, except for list options, i.e. options that
can be passed multiple times (--event, --filter, --trigger,
--on-threshold, --on-end).
This allows, for example, unsetting of int/long long options, e.g. "-p":
$ rtla timerlat -D -p100 --no-period
...
setting osnoise/timerlat_period_us to 1000
...
By default, options in params struct are reset to zero. A constant is
added for every parameter with a different default value, which is then
used both in <tool>_hist_args() while setting the initial value and in
opt_*() when unsetting the option. This refactoring ensures there is no
duplicate "magic number".
The default value for opt_llong_callback() and opt_int_callback() is
passed in struct option's defval field; new macros
RTLA_OPT_{LLONG,INT}{,_DEFVAL} are added to define the field
conveniently. The default value for other callbacks is hardcoded inside
each callback's unset logic.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260629083654.1548925-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add a new option, -A/--aligned, that enables timerlat thread alignment
implemented on the kernel-side in commit 4245bf4dc58f ("tracing/osnoise:
Add option to align tlat threads"). The option takes an argument,
representing alignment between timerlat threads in microseconds.
The feature is modeled after the option of the same name in the
cyclictest tool.
Link: https://lore.kernel.org/r/20260527144928.2944472-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Instead of using getopt_long() directly to parse the command line
arguments given to an RTLA tool, use libsubcmd's parse_options().
Utilizing libsubcmd for parsing command line arguments has several
benefits:
- A help message is automatically generated by libsubcmd from the
specification, removing the need of writing it by hand.
- Options are sorted into groups based on which part of tracing (CPU,
thread, auto-analysis, tuning, histogram) they relate to.
- Common parsing patterns for numerical and boolean values now share
code, with the target variable being stored in the option array.
To avoid duplication of the option parsing logic, RTLA-specific
macros defining struct option values are created:
- RTLA_OPT_* for options common to all tools
- OSNOISE_OPT_* and TIMERLAT_OPT_* for options specific to
osnoise/timerlat tools
- HIST_OPT_* macros for options specific to histogram-based tools.
Individual *_parse_args() functions then construct an array out of
these macros that is then passed to libsubcmd's parse_options().
All code specific to command line options parsing is moved out of the
individual tool files into a new file, cli.c, which also contains the
contents of the rtla.c file. A private header, cli_p.h, is added
alongside the public header cli.h, so that unit tests are able to test
statically declared option callbacks.
Minor changes:
- The return value of tool-level help option changes to 129, as this is
the value set by libsubcmd; this is reflected in affected test cases.
The implementation of help for command-level and tracer-level help
is set to 129 as well for consistency, and the change is reflected in
exit value documentation.
- Related to the above, {rtla,osnoise,timerlat}_usage() are marked
__noreturn and exit() is removed from after they are called for
cleaner code.
- The error messages for invalid argument for options --dma-latency and
-E/--entries were corrected, fixing off-by-one in the limits.
Note that unsetting options (using --no-<opt> syntax) is currently not
implemented for options that use custom callbacks. For --irq and
--thread, it will never be implemented, as they conflict with already
existing --no-irq and --no-thread with a different meaning.
Assisted-by: Composer:composer-1.5
Link: https://lore.kernel.org/r/20260528103254.2990068-5-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
Tomas Glozar