linux-kernel - Re: [PATCH v2 1/3] tools/rtla: Fix --on-threshold always triggering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <gwophnr3qcjvhe4wt2th5idkidjhejt2pei6swol7pwtsfpnhb@qjjwghig7wlt>
Date: Fri, 17 Oct 2025 09:21:05 -0300
From: Wander Lairson Costa <wander@...hat.com>
To: Tomas Glozar <tglozar@...hat.com>
Cc: Steven Rostedt <rostedt@...dmis.org>, 
	LKML <linux-kernel@...r.kernel.org>, Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>, 
	John Kacur <jkacur@...hat.com>, Luis Goncalves <lgoncalv@...hat.com>, 
	Costa Shulyupin <costa.shul@...hat.com>, Crystal Wood <crwood@...hat.com>
Subject: Re: [PATCH v2 1/3] tools/rtla: Fix --on-threshold always triggering

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 <tglozar@...hat.com>
> Reviewed-by: Crystal Wood <crwood@...hat.com>
> ---
> 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(&params->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(&params->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 <wander@...hat.com>