On Tue, Oct 07, 2025 at 11:53:39AM +0200, Tomas Glozar wrote:
> Commit 8d933d5c89e8 ("rtla/timerlat: Add continue action") moved the
> code performing on-threshold actions (enabled through --on-threshold
> option) to inside the RTLA main loop.
>
> The condition in the loop does not check whether the threshold was
> actually exceeded or if stop tracing was requested by the user through
> SIGINT or duration. This leads to a bug where on-threshold actions are
> always performed, even when the threshold was not hit.
>
> (BPF mode is not affected, since it uses a different condition in the
> while loop.)
>
> Add a condition that checks for !stop_tracing before executing the
> actions. Also, fix incorrect brackets in hist_main_loop to match the
> semantics of top_main_loop.
>
> Fixes: 8d933d5c89e8 ("rtla/timerlat: Add continue action")
> Fixes: 2f3172f9dd58 ("tools/rtla: Consolidate code between osnoise/timerlat
> and hist/top")
> Signed-off-by: Tomas Glozar <[email protected]>
> Reviewed-by: Crystal Wood <[email protected]>
> ---
> v2:
> - add one more patch fixing a bug noticed by Crystal in context of
> the following one (doing v2 just to avoid conflicts, the first two
> patches were not changed)
>
> tools/tracing/rtla/src/common.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
> index 2e6e3dac1897..b197037fc58b 100644
> --- a/tools/tracing/rtla/src/common.c
> +++ b/tools/tracing/rtla/src/common.c
> @@ -268,6 +268,10 @@ int top_main_loop(struct osnoise_tool *tool)
> tool->ops->print_stats(tool);
>
> if (osnoise_trace_is_off(tool, record)) {
> + if (stop_tracing)
> + /* stop tracing requested, do not perform
> actions */
> + return 0;
> +
> actions_perform(¶ms->threshold_actions);
>
> if (!params->threshold_actions.continue_flag)
> @@ -315,20 +319,22 @@ int hist_main_loop(struct osnoise_tool *tool)
> }
>
> if (osnoise_trace_is_off(tool, tool->record)) {
> + if (stop_tracing)
> + /* stop tracing requested, do not perform
> actions */
> + break;
> +
> actions_perform(¶ms->threshold_actions);
>
> - if (!params->threshold_actions.continue_flag) {
> + if (!params->threshold_actions.continue_flag)
> /* continue flag not set, break */
> break;
>
> - /* continue action reached, re-enable tracing */
> - if (tool->record)
> -
> trace_instance_start(&tool->record->trace);
> - if (tool->aa)
> - trace_instance_start(&tool->aa->trace);
> - trace_instance_start(&tool->trace);
> - }
> - break;
> + /* continue action reached, re-enable tracing */
> + if (tool->record)
> + trace_instance_start(&tool->record->trace);
> + if (tool->aa)
> + trace_instance_start(&tool->aa->trace);
> + trace_instance_start(&tool->trace);
> }
>
> /* is there still any user-threads ? */
> --
> 2.51.0
>
Reviewed-by: Wander Lairson Costa <[email protected]>
