| Age | Commit message (Collapse) | Author |
|---|
|
Multiple runtime tests in RTLA rely on the get_workload_pids() shell
helper function to get the PIDs of both kernel and user workloads.
On some systems (e.g. Fedora 43), pgrep matches kernel thread names
including square brackets: "[osnoise/0]"; on other systems (e.g.
RHEL 9.8), brackets are not included: "osnoise/0".
Accept both as valid workload PIDs rather that just the non-bracket form
to make the tests work on all systems.
Fixes: a98dad63cda3 ("rtla/tests: Add runtime test for -k and -u options")
Reported-by: Crystal Wood <crwood@redhat.com>
Link: https://lore.kernel.org/r/20260604140547.3616495-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
.gitignore includes several entries prone to unwanted matches in
subdirectories. One of them, the recently added "lib/", matches the
recently added directory "tests/scripts/lib/" in addition to the
intended top-level "lib/", which contains object files built from
sources in tools/lib.
Add "/" to all .gitignore entries that are intended to only match
top-level files or directories: rtla, rtla_static, unit_tests,
libsubcmd/.
Remove .gitignore entries that are not needed at all:
- lib/ (contains only object files, ignored by top-level .gitignore
already).
- .txt rtla output files added to .gitignore in commit 02689ae385c5
("rtla: Add generated output files to gitignore"). Since commit
ad5b50a0959f ("rtla/tests: Run runtime tests in temporary directory"),
those are created in a temporary directory, not in tools/tracing/rtla.
Keeping libsubcmd/ as that contains other generated files (headers,
archives, etc.).
Fixes: 48209d763c22 ("rtla: Add libsubcmd dependency")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202605291919.eszupseg-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202605300436.PqQ0Bc8q-lkp@intel.com/
Link: https://lore.kernel.org/r/20260601091835.3118094-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
RTLA tests are not documented anywhere. Mention both runtime and unit
tests in the README, with instructions on how to run them and a list of
dependencies and required system configuration.
Link: https://lore.kernel.org/r/20260514073038.204428-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add both parse_args() and opt_* tests for the newly added -A/--aligned
option.
Assisted-by: Claude:claude-4.5-opus-high-thinking
Link: https://lore.kernel.org/r/20260527144928.2944472-2-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>
|
|
In addition to testing all tool_parse_args() functions, test also all
callbacks used for parsing custom option formats.
The callbacks represent a middle layer between the parsing functions
and utility functions dedicated to checking specific argument formats,
for example, scheduling class and duration. Callback tests are run
before parsing functions to make sure any issue in the former is
reported before it is encountered through the latter.
Tests verify both successful parsing and proper rejection of invalid
inputs (via exit tests). To enable testing static callbacks, a pragma
once guard is added to timerlat.h for safe inclusion by cli_p.h.
Add dependency of UNIT_TESTS_IN on LIBSUBCMD_INCLUDES, as the new test
file tests/unit/cli_opt_callback.c includes cli_p.h which includes
subcmd/parse-options.h.
Link: https://lore.kernel.org/r/20260528103254.2990068-7-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add a test suite for the _parse_args() function of each tool that checks
the params structures (struct common_params, struct osnoise_params,
struct timerlat_params) returned by them for correctness.
One test case is added per option, as well as a few special cases for
tricky combinations of options. Test cases are ordered the same as the
option arrays and help message to allow easy checking of whether all
options are covered.
This should help clarify what the proper command line behavior of RTLA
is in case there are holes in the documentation and verify that the
intended behavior is implemented correctly.
A few necessary changes to the unit tests were done as part of this
commit:
- Unit tests now also link to libsubcmd and its dependencies.
- A new global variable in_unit_test is added to RTLA's CLI interface,
causing it to skip check for root if running in unit tests. This
allows the CLI unit tests to run as non-root, like existing unit
tests.
There is quite a lot of duplication, some of it is mitigated with macros,
but partially it is intentional so that future changes in behavior are
tracked across tools.
Link: https://lore.kernel.org/r/20260528103254.2990068-6-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>
|
|
In preparation for migrating RTLA to libsubcmd, build libsubcmd from the
appropriate directory next to the RTLA build proper, and link the
resulting object to RTLA.
libsubcmd uses str_error_r() and strlcpy() at several places. To support
these, also link the respective libraries from tools/lib.
For completeness, also add tools/include to include path. This will
allow other userspace functions and macros shipped with the kernel to be
used in RTLA; perf and bpftool, two other users of libsubcmd, already do
that.
To prevent a name conflict, rename RTLA's run_command() function to
run_tool_command(), and replace RTLA's own container_of implementation
with the one in tools/include/linux/container_of.h.
Assisted-by: Composer:composer-1
Link: https://lore.kernel.org/r/20260528103254.2990068-2-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
In case an action preceding the continue action fails, not only
the continue flag should not be set, it should be unset if it was set
from a previous run of actions_perform().
Add a runtime test to both osnoise and timerlat tools that checks that
this works properly by creating a temporary file.
Link: https://lore.kernel.org/r/20260526102523.2662391-4-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Create a temporary directory before each test case to serve as working
directory during the duration of the test.
This prevents littering of the original working directory as well as
allows tests to use it to avoid path conflicts.
In order not to break already existing tests, also add a new "testdir"
variable containing the directory where the test file is located. This
is then used to locate artifacts used during testing like BPF programs
and scripts for checking the tracer threads.
Link: https://lore.kernel.org/r/20260526102523.2662391-3-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
In case an action preceding the continue action fails, not only
the continue flag should not be set, it should be unset if it was set
from a previous run of actions_perform().
Add a unit test to check if this is implemented correctly.
Link: https://lore.kernel.org/r/20260526102523.2662391-2-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Currently, actions_perform() only ever sets the continue flag (when
performing the continue action), but never resets it. That leads to
RTLA continuing tracing even if the continue action was not performed in
the current iteration.
For example, the following command:
$ rtla timerlat hist -T 100 --on-threshold shell,command='
echo Spike!
if [ -f /tmp/a ]
then
exit 1
else
touch /tmp/a
fi' --on-threshold continue
should print Spike! at most once, because after hitting the threshold
for the first time, /tmp/a exists, the shell action will fail, and the
continue action is not performed. However, unless /tmp/a exists before
the measurement, it will print Spike! until stopped, as the continue
flag stays set.
Set the continue flag to false in the beginning of actions_perform() to
make RTLA continue only if the action was actually performed.
Fixes: 8d933d5c89e8 ("rtla/timerlat: Add continue action")
Link: https://lore.kernel.org/r/20260526102523.2662391-1-tglozar@redhat.com
[ correct Fixes tag to include 12 characters of hash ]
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Before, when rtla got a signal, it stopped the main trace but not the
record trace. With "--on-end trace", this can lead to
save_trace_to_file() failing to keep up, especially on a debug kernel.
Plus, it adds post-stoppage noise to the trace file.
Signed-off-by: Crystal Wood <crwood@redhat.com>
Fixes: c73cab9dbed0 ("rtla/timerlat_hist: Stop timerlat tracer on signal")
Fixes: a4dfce7559d7 ("rtla/timerlat_top: Stop timerlat tracer on signal")
Fixes: 3aadb65db5d6 ("rtla/timerlat: Add action on end feature")
Link: https://lore.kernel.org/r/20260512173731.2151841-1-crwood@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add unit tests covering all functions in the actions module, including
both valid and invalid inputs and all action types, except for
actions_perform(), where only shell and continue actions are tested.
To support testing multiple modules, the unit test build was modified so
that it links the entire rtla-in.o file. For this to work, the main()
function in rtla.c was declared weak, so that the unit test main is able
to override it.
Other included minor changes to unit tests are:
- Make unit test output verbose to show which tests are being run, now
that we have more than 3 tests.
- Add unit_tests file to .gitignore.
- Split unit test sources to one file per test suite, and keep only
main() function in unit_tests.c.
- Fix Makefile dependencies so that "make unit-tests" will rebuild the
binary with the changes in the commit.
Also with the linking the entire rtla-in.o file, it now has rtla's
nr_cpus symbol, so the declaration in utils unit tests is made extern.
Assisted-by: Composer:composer-2-fast
Link: https://lore.kernel.org/r/20260424140244.958495-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add a new script check-cgroup-match.sh that retrieves the cgroup of the
main rtla process and compares it to the cgroup of the rtla workload
threads.
Add a new test based on this script, for both osnoise and timerlat
tools, testing the variant of -C without argument (which sets the cgroup
of the workload to the cgroup of the rtla main process).
Note that this has to be tested in kernel mode to be significant for
timerlat tool, as user workloads inherit the parent rtla process cgroup
even without the option.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-10-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add runtime test for rtla-timerlat's -k/--kernel-threads and
-u/--user-threads options using get_workload_pids.sh to check whether
the appropriate threads are being created.
The tests are implemented for both top and hist. Additionally, all tests
related to timerlat threads are moved to a separate section in the test
files. The latter is also done for rtla-osnoise tests.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-9-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add a runtime test for -H/--house-keeping option for both osnoise and
timerlat tools, with affinity checking similar to what is done for
-c/--cpus.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-8-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Cover all options regarding histogram formatting for both
rtla-osnoise-hist and rtla-timerlat-hist tools. All options also have
output checking using positive or negative match, except for
-b/--bucket-size and -E/--entries, which cannot be tested in isolated
due to the output depending on the actual data collected.
Old -E/--entries test for rtla-osnoise was replaced with a new one
equivalent to the timerlat one.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-7-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
rtla-timerlat-top's --aa-only option is currently only tested for return
value.
Extend the tests to also check that only auto-analysis is being done via
a negative match for the "Timer Latency" text in the top header, and
further split the test case into two:
- one test case for --aa-only stopping on threshold
- one test case for --aa-only exiting without threshold being hit
For both cases, the expected output ("analyzing it" or "Max latency was"
respectively) is checked against in addition to the negative match.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-6-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
For testing the -a/--auto option in timerlat tool, the string "analyzing
it" is matched against to make sure auto-analysis was triggered.
Use the same string as a negative match for --aa-only option test.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-5-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
RTLA runtime tests verify the -c/--cpus options, but do not check
whether the correct affinity is actually applied.
Add a script named check-cpus.sh that retrieves the affinity of all
workload threads and use it to check the -c/--cpus option for both
osnoise and timerlat tools.
Also add missing -c/--cpus test for osnoise.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-4-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
RTLA runtime tests that check workload processes (currently the test
case "verify -P/--priority" of timerlat.t and "verify the --priority/-P
param" of osnoise.t) use "pgrep timerlatu/" or "pgrep osnoise/"
respectively to identify the workload.
Make them more robust by adding a get_workload_pids() helper that
finds the main rtla process and returns the PIDs of all siblings other
than the test script itself, plus all child processes of kthreadd that
have the osnoise/timerlat kthread pattern comm.
This filters out any spurious processes not related to the running test
that happen to have "timerlatu/" or "osnoise/" in their command, for
example, a user grepping the same names at the time of the running of
the test.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-3-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
RTLA runtime tests currently do not cover both tool variants for osnoise
and timerlat properly. Many tests applicable to both tools are only
tested for one tool, selected randomly.
Introduce two new shell functions, check_top_hist() and
check_top_q_hist(). The functions use the same syntax as check() and run
check() on the arguments twice: once replacing the "TOOL" string in the
command with "top" (or "top -q"), once replacing it with "hist". The top
-q variant is used for tests relying on messages printed after aborting
the RTLA main loop with a starting new line, which only happens for top
tools in quiet mode; without -q, the top output is printed on the same
line and the matches would fail.
Tests that are applicable to both top and hist tools were modified to
the run for both; additionally, tests that were already done for both
tools were migrated to the new shell functions, unless the test command
or matches differ between the tools. Additional tests were added to test
tool-specific help messages.
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260423130558.882022-2-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Fix --dump-task to --dump-tasks in timerlat_hist usage string
and getopt_long table for consistency with timerlat_top.
Add missing --dump-tasks to timerlat_top usage synopsis.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
Fixes: 2091336b9a8b ("rtla/timerlat_hist: Add auto-analysis support")
Link: https://lore.kernel.org/r/20260414185223.65353-1-costa.shul@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
Pull RTLA updates from Steven Rostedt:
- Simplify option parsing
Auto-generate getopt_long() optstring for short options from long
options array, avoiding the need to specify it manually and reducing
the surface for mistakes.
- Add unit tests
Implement unit tests (make unit-tests) using libcheck, next to
existing runtime tests (make check). Currently, three functions from
utils.c are tested.
- Add --stack-format option
In addition to stopping stack pointer decoding (with -s/--stack
option) on first unresolvable pointer, allow also skipping
unresolvable pointers and displaying everything, configurable with a
new option.
- Unify number of CPUs into one global variable
Use one global variable, nr_cpus, to store the number of CPUs instead
of retrieving it and passing it at multiple places.
- Fix behavior in various corner cases
Make RTLA behave correctly in several corner cases: memory allocation
failure, invalid value read from kernel side, thread creation
failure, malformed time value input, and read/write failure or
interruption by signal.
- Improve string handling
Simplify several places in the code that handle strings, including
parsing of action arguments. A few new helper functions and variables
are added for that purpose.
- Get rid of magic numbers
Few places handling paths use a magic number of 1024. Replace it with
MAX_PATH and ARRAY_SIZE() macro.
- Unify threshold handling
Code that handles response to latency threshold is duplicated between
tools, which has led to bugs in the past. Unify it into a new helper
as much as possible.
- Fix segfault on SIGINT during cleanup
The SIGINT handler touches dynamically allocated memory. Detach it
before freeing it during cleanup to prevent segmentation fault and
discarding of output buffers. Also, properly document SIGINT handling
while at it.
* tag 'trace-rtla-v7.1' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace: (28 commits)
Documentation/rtla: Document SIGINT behavior
rtla: Fix segfault on multiple SIGINTs
rtla/utils: Fix loop condition in PID validation
rtla/utils: Fix resource leak in set_comm_sched_attr()
rtla/trace: Fix I/O handling in save_trace_to_file()
rtla/trace: Fix write loop in trace_event_save_hist()
rtla/timerlat: Simplify RTLA_NO_BPF environment variable check
rtla: Use str_has_prefix() for option prefix check
rtla: Enforce exact match for time unit suffixes
rtla: Use str_has_prefix() for prefix checks
rtla: Add str_has_prefix() helper function
rtla: Handle pthread_create() failure properly
rtla/timerlat: Add bounds check for softirq vector
rtla: Simplify code by caching string lengths
rtla: Replace magic number with MAX_PATH
rtla: Introduce common_threshold_handler() helper
rtla/actions: Simplify argument parsing
rtla: Use strdup() to simplify code
rtla: Exit on memory allocation failures during initialization
tools/rtla: Remove unneeded nr_cpus from for_each_monitored_cpu
...
|
|
rtla supports building without libbpf. However, BPF actions
patchset [1] adds an include of bpf/libbpf.h into timerlat_bpf.h,
which breaks build on systems that don't have libbpf headers
installed.
This is a leftover from a draft version of the patchset where
timerlat_bpf_set_action() (which takes a struct bpf_program * argument)
was defined in the header. timerlat_bpf.c already includes bpf/libbpf.h
via timerlat.skel.h when libbpf is present.
Remove the redundant include to fix build on systems without libbpf
headers.
[1] https://lore.kernel.org/linux-trace-kernel/20251126144205.331954-1-tglozar@redhat.com/T/
Cc: John Kacur <jkacur@redhat.com>
Cc: Luis Goncalves <lgoncalv@redhat.com>
Cc: Crystal Wood <crwood@redhat.com>
Cc: Costa Shulyupin <costa.shul@redhat.com>
Link: https://patch.msgid.link/20260330091207.16184-1-tglozar@redhat.com
Reported-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Closes: https://lore.kernel.org/linux-trace-kernel/20260329122202.65a8b575@robin/
Fixes: 8cd0f08ac72e ("rtla/timerlat: Support tail call from BPF program")
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
|
|
Detach stop_trace() from SIGINT/SIGALRM on tool clean-up to prevent it
from crashing RTLA by accessing freed memory.
This prevents a crash when multiple SIGINTs are received.
Fixes: d6899e560366 ("rtla/timerlat_hist: Abort event processing on second signal")
Fixes: 80967b354a76 ("rtla/timerlat_top: Abort event processing on second signal")
Reviewed-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260310160725.144443-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The procfs_is_workload_pid() function iterates through a directory
entry name to validate if it represents a process ID. The loop
condition checks if the pointer t_name is non-NULL, but since
incrementing a pointer never makes it NULL, this condition is always
true within the loop's context. Although the inner isdigit() check
catches the NUL terminator and breaks out of the loop, the condition
is semantically misleading and not idiomatic for C string processing.
Correct the loop condition from checking the pointer (t_name) to
checking the character it points to (*t_name). This ensures the loop
terminates when the NUL terminator is reached, aligning with standard
C string iteration practices. While the original code functioned
correctly due to the existing character validation, this change
improves code clarity and maintainability.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-19-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The set_comm_sched_attr() function opens the /proc directory via
opendir() but fails to call closedir() on its successful exit path.
If the function iterates through all processes without error, it
returns 0 directly, leaking the DIR stream pointer.
Fix this by refactoring the function to use a single exit path. A
retval variable is introduced to track the success or failure status.
All exit points now jump to a unified out label that calls closedir()
before the function returns, ensuring the resource is always freed.
Fixes: dada03db9bb19 ("rtla: Remove procps-ng dependency")
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-18-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The read/write loop in save_trace_to_file() does not correctly handle
errors from the read() and write() system calls. If either call is
interrupted by a signal, it returns -1 with errno set to EINTR, but
the code treats this as a fatal error and aborts the save operation.
Additionally, write() may perform a partial write, returning fewer
bytes than requested, which the code does not handle.
Fix the I/O loop by introducing proper error handling. The return
value of read() is now stored in a ssize_t variable and checked for
errors, with EINTR causing a retry. For write(), an inner loop ensures
all bytes are written, handling both EINTR and partial writes. Error
messages now include strerror() output for better debugging.
This follows the same pattern established in the previous commit that
fixed trace_event_save_hist(), ensuring consistent and robust I/O
handling throughout the trace saving code.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-17-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The write loop in trace_event_save_hist() does not correctly handle
errors from the write() system call. If write() returns -1, this value
is added to the loop index, leading to an incorrect memory access on
the next iteration and potentially an infinite loop. The loop also
fails to handle EINTR.
Fix the write loop by introducing proper error handling. The return
value of write() is now stored in a ssize_t variable and checked for
errors. The loop retries the call if interrupted by a signal and breaks
on any other error after logging it with strerror().
Additionally, change the index variable type from int to size_t to
match the type used for buffer sizes and by strlen(), improving type
safety.
Fixes: 761916fd02c2 ("rtla/trace: Save event histogram output to a file")
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-16-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The code that checks the RTLA_NO_BPF environment variable calls
getenv() twice and uses strncmp() with a length of 2 to compare
against the single-character string "1". This is inefficient and
the comparison length is unnecessarily long.
Store the result of getenv() in a local variable to avoid the
redundant call, and replace strncmp() with strncmp_static() for
the exact match comparison. This follows the same pattern
established in recent commits that improved string comparison
consistency throughout the rtla codebase.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-15-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The argument parsing code in timerlat_main() and osnoise_main() uses
strncmp() with a length of 1 to check if the first argument starts
with a dash, indicating an option flag was passed.
Replace this pattern with str_has_prefix() for consistency with the
rest of the codebase. While character comparison would be slightly
more efficient, using str_has_prefix() provides better readability
and maintains a uniform coding style throughout the rtla tool.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-14-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The parse_ns_duration() function currently uses prefix matching for
detecting time units. This approach is problematic as it silently
accepts malformed strings such as "100nsx" or "100us_invalid" by
ignoring the trailing characters, leading to potential configuration
errors.
Introduce a match_time_unit() helper that checks the suffix matches
exactly and is followed by either end-of-string or a ':' delimiter.
The ':' is needed because parse_ns_duration() is also called from
get_long_ns_after_colon() when parsing SCHED_DEADLINE priority
specifications in the format "d:runtime:period" (e.g., "d:10ms:100ms").
A plain strcmp() would reject valid deadline strings because the suffix
"ms" is followed by ":100ms", not end-of-string. Similarly,
strncmp_static() would fail because ARRAY_SIZE() includes the NUL
terminator, making it equivalent to strcmp() for this comparison.
The match_time_unit() helper solves both problems: it rejects malformed
input like "100msx" while correctly handling the colon-delimited
deadline format.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-13-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The code currently uses strncmp() combined with strlen() to check if a
string starts with a specific prefix. This pattern is verbose and prone
to errors if the length does not match the prefix string.
Replace this pattern with the str_has_prefix() helper function in both
trace.c and utils.c. This improves code readability and safety by
handling the prefix length calculation automatically.
In addition, remove the unused retval variable from
trace_event_save_hist() in trace.c to clean up the function and
silence potential compiler warnings.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-12-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add a str_has_prefix() helper function that tests whether a string
starts with a given prefix. This function provides a cleaner interface
for prefix matching compared to using strncmp() with strlen() directly.
The function returns a boolean value indicating whether the string
starts with the specified prefix. This helper will be used in
subsequent changes to simplify prefix matching code throughout rtla.
Also add the missing string.h include which is needed for the strlen()
and strncmp() functions used by str_has_prefix() and the existing
strncmp_static() macro.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-11-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add proper error handling when pthread_create() fails to create the
timerlat user-space dispatcher thread. Previously, the code only logged
an error message but continued execution, which could lead to undefined
behavior when the tool later expects the thread to be running.
When pthread_create() returns an error, the function now jumps to the
out_trace error path to properly clean up resources and exit. This
ensures consistent error handling and prevents the tool from running
in an invalid state without the required user-space thread.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-10-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Add bounds checking when accessing the softirq_name array using the
vector value from kernel trace data. The vector field from the
osnoise:softirq_noise event is used directly as an array index without
validation, which could cause an out-of-bounds read if the kernel
provides an unexpected vector value.
The softirq_name array contains 10 elements corresponding to the
standard Linux softirq vectors. While the kernel should only provide
valid vector values in the range 0-9, defensive programming requires
validating untrusted input before using it as an array index. If an
out-of-range vector is encountered, display the word UNKNOWN instead
of attempting to read beyond the array bounds.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-9-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Simplify trace_event_save_hist() and set_comm_cgroup() by computing
string lengths once and storing them in local variables, rather than
calling strlen() multiple times on the same unchanged strings. This
makes the code clearer by eliminating redundant function calls and
improving readability.
In trace_event_save_hist(), the write loop previously called strlen()
on the hist buffer twice per iteration for both the size calculation
and loop condition. Store the length in hist_len before entering the
loop. In set_comm_cgroup(), strlen() was called on cgroup_path up to
three times in succession. Store the result in cg_path_len to use in
both the offset calculation and size parameter for subsequent append
operations.
This simplification makes the code easier to read and maintain without
changing program behavior.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-7-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The trace functions use a buffer to manipulate strings that will be
written to tracefs files. These buffers are defined with a magic number
of 1024, which is a common source of vulnerabilities.
Replace the magic number 1024 with the MAX_PATH macro to make the code
safer and more readable. While at it, replace other instances of the
magic number with ARRAY_SIZE() when the buffer is locally defined.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-6-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Several functions duplicate the logic for handling threshold actions.
When a threshold is reached, these functions stop the trace, perform
configured actions, and restart the trace if --on-threshold continue
is set.
Create common_threshold_handler() to centralize this shared logic and
avoid code duplication. The function executes the configured threshold
actions and restarts the necessary trace instances when appropriate.
Also add should_continue_tracing() helper to encapsulate the check
for whether tracing should continue after a threshold event, improving
code readability at call sites.
In timerlat_top_bpf_main_loop(), use common_params directly instead
of casting through timerlat_params when only common fields are needed.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-5-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The actions_parse() function uses open-coded logic to extract arguments
from a string. This includes manual length checks and strncmp() calls,
which can be verbose and error-prone.
To simplify and improve the robustness of argument parsing, introduce a
new extract_arg() helper macro. This macro extracts the value from a
"key=value" pair, making the code more concise and readable.
Also, introduce STRING_LENGTH() and strncmp_static() macros to
perform compile-time calculations of string lengths and safer string
comparisons.
Refactor actions_parse() to use these new helpers, resulting in
cleaner and more maintainable code.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-4-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
The actions_add_trace_output() and actions_add_shell() functions were
using calloc() followed by strcpy() to allocate and copy a string.
This can be simplified by using strdup(), which allocates memory and
copies the string in a single step.
Replace the calloc() and strcpy() calls with strdup(), making the
code more concise and readable.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-3-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
Most memory allocations in rtla happen during early initialization
before any resources are acquired that would require cleanup. In these
cases, propagating allocation errors just adds complexity without any
benefit. There's nothing to clean up, and the program must exit anyway.
This patch introduces fatal allocation wrappers (calloc_fatal,
reallocarray_fatal, strdup_fatal) that call fatal() on allocation
failure. These wrappers simplify the code by eliminating unnecessary
error propagation paths.
The patch converts early allocations to use these wrappers in
actions_init() and related action functions, osnoise_context_alloc()
and osnoise_init_tool(), trace_instance_init() and trace event
functions, and parameter structure allocations in main functions.
This simplifies the code while maintaining the same behavior: immediate
exit on allocation failure during initialization. Allocations that
require cleanup, such as those in histogram allocation functions with
goto cleanup paths, are left unchanged and continue to return errors.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Link: https://lore.kernel.org/r/20260309195040.1019085-2-wander@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
nr_cpus does not change at runtime, so passing it through the macro
argument is unnecessary.
Remove the argument and use the global nr_cpus instead.
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
Link: https://lore.kernel.org/r/20260306194953.2511960-5-costa.shul@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
nr_cpus does not change at runtime, so keeping it in struct members is
unnecessary.
Use the global nr_cpus instead of struct members.
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
Link: https://lore.kernel.org/r/20260306194953.2511960-4-costa.shul@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
nr_cpus does not change at runtime, so passing it through function
arguments is unnecessary.
Use the global nr_cpus instead of propagating it via parameters.
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
Link: https://lore.kernel.org/r/20260306194953.2511960-3-costa.shul@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
sysconf(_SC_NPROCESSORS_CONF) (via get_nprocs_conf) reflects
cpu_possible_mask, which is fixed at boot time, so querying it
repeatedly is unnecessary.
Replace multiple calls to sysconf(_SC_NPROCESSORS_CONF) with a single
global nr_cpus variable initialized once at startup.
`#pragma once` in timerlat_u.h is needed for pre-C23 compilers to avoid
redefinition errors.
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
Link: https://lore.kernel.org/r/20260306194953.2511960-2-costa.shul@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
|
In the current implementation, the auto-analysis code for printing the
stack captured in the tracefs buffer of the aa instance stops at the
first encountered address that cannot be resolved into a function
symbol.
This is not always the desired behavior on all platforms; sometimes,
there might be resolvable entries after unresolvable ones, and
sometimes, the user might want to inspect the raw pointers for the
unresolvable entries.
Add a new option, --stack-format, with three values:
- truncate: stop at first unresolvable entry. This is the current
behavior, and is kept as the default.
- skip: skip unresolvable entries, but do not stop on them.
- full: print all entries, including unresolvable ones.
To make this work, the "size" field of the stack entry is now also read
and used as the maximum number of entries to print, capped at 64, since
that is the fixed length of the "caller" field.
Link: https://lore.kernel.org/r/20260119115222.744150-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
|
