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

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

[Tomas Glozar]

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@redhat.com>
Reviewed-by: Crystal Wood <crwood@redhat.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

[PATCH v2 2/3] rtla/tests: Extend action tests to 5s

[Tomas Glozar]

In non-BPF mode, it takes up to 1 second for RTLA to notice that tracing
has been stopped. That means that action tests cannot have a 1 second
duration, as the SIGALRM will be racing with the threshold overflow.

Previously, non-BPF mode actions were buggy and always executed
the action, even when stopping on duration or SIGINT, preventing
this issue from manifesting. Now that this has been fixed, the tests
have become flaky, and this has to be adjusted.

Fixes: 4e26f84abfb ("rtla/tests: Add tests for actions")
Fixes: 05b7e10687c ("tools/rtla: Add remaining support for osnoise actions")
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/tests/osnoise.t  | 4 ++--
 tools/tracing/rtla/tests/timerlat.t | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
index e3c89d45a6bb..08196443fef1 100644
--- a/tools/tracing/rtla/tests/osnoise.t
+++ b/tools/tracing/rtla/tests/osnoise.t
@@ -39,9 +39,9 @@ check "hist stop at failed action" \
 check "top stop at failed action" \
 	"timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
 check "hist with continue" \
-	"osnoise hist -S 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
+	"osnoise hist -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
 check "top with continue" \
-	"osnoise top -q -S 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
+	"osnoise top -q -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
 check "hist with trace output at end" \
 	"osnoise hist -d 1s --on-end trace" 0 "^  Saving trace to osnoise_trace.txt$"
 check "top with trace output at end" \
diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t
index b5d1e7260a9b..b550a6ae2445 100644
--- a/tools/tracing/rtla/tests/timerlat.t
+++ b/tools/tracing/rtla/tests/timerlat.t
@@ -60,9 +60,9 @@ check "hist stop at failed action" \
 check "top stop at failed action" \
 	"timerlat top -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1ALL"
 check "hist with continue" \
-	"timerlat hist -T 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
+	"timerlat hist -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
 check "top with continue" \
-	"timerlat top -q -T 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
+	"timerlat top -q -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
 check "hist with trace output at end" \
 	"timerlat hist -d 1s --on-end trace" 0 "^  Saving trace to timerlat_trace.txt$"
 check "top with trace output at end" \
-- 
2.51.0

[PATCH v2 3/3] rtla/tests: Fix osnoise test calling timerlat

[Tomas Glozar]

osnoise test "top stop at failed action" is calling timerlat instead of
osnoise by mistake.

Fix it so that it calls the correct RTLA subcommand.

Fixes: 05b7e10687c ("tools/rtla: Add remaining support for osnoise actions")
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/tracing/rtla/tests/osnoise.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
index 08196443fef1..396334608920 100644
--- a/tools/tracing/rtla/tests/osnoise.t
+++ b/tools/tracing/rtla/tests/osnoise.t
@@ -37,7 +37,7 @@ check "multiple actions" \
 check "hist stop at failed action" \
 	"osnoise hist -S 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1# RTLA osnoise histogram$"
 check "top stop at failed action" \
-	"timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
+	"osnoise top -S 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
 check "hist with continue" \
 	"osnoise hist -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
 check "top with continue" \
-- 
2.51.0

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

[Tomas Glozar]

Hi Steven,

út 7. 10. 2025 v 11:53 odesílatel Tomas Glozar <tglozar@redhat.com> napsal:
>
> 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@redhat.com>
> Reviewed-by: Crystal Wood <crwood@redhat.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(-)
>

It seems that this patchset containing fixes for recently merged RTLA
code consolidation [1] has no further comments. Could you take it?
(Unless you have any comments, of course.)

[1] https://lore.kernel.org/linux-trace-kernel/20250907022325.243930-1-crwood@redhat.com/T/

Thanks,
Tomas

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

[Steven Rostedt]

On Fri, 17 Oct 2025 09:38:04 +0200
Tomas Glozar <tglozar@redhat.com> wrote:
> 
> It seems that this patchset containing fixes for recently merged RTLA
> code consolidation [1] has no further comments. Could you take it?
> (Unless you have any comments, of course.)
> 
> [1] https://lore.kernel.org/linux-trace-kernel/20250907022325.243930-1-crwood@redhat.com/T/

I'll take a look at it next week. I added it to my "delegate" in Patchwork:

  https://patchwork.kernel.org/project/linux-trace-kernel/list/?series=1008975

So hopefully, I don't forget.

We need to start making you the maintainer too ;-)

-- Steve

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

[Wander Lairson Costa]

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@redhat.com>
> Reviewed-by: Crystal Wood <crwood@redhat.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@redhat.com>

Re: [PATCH v2 3/3] rtla/tests: Fix osnoise test calling timerlat

[Wander Lairson Costa]

On Tue, Oct 07, 2025 at 11:53:41AM +0200, Tomas Glozar wrote:
> osnoise test "top stop at failed action" is calling timerlat instead of
> osnoise by mistake.
> 
> Fix it so that it calls the correct RTLA subcommand.
> 
> Fixes: 05b7e10687c ("tools/rtla: Add remaining support for osnoise actions")
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/tests/osnoise.t | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
> index 08196443fef1..396334608920 100644
> --- a/tools/tracing/rtla/tests/osnoise.t
> +++ b/tools/tracing/rtla/tests/osnoise.t
> @@ -37,7 +37,7 @@ check "multiple actions" \
>  check "hist stop at failed action" \
>  	"osnoise hist -S 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1# RTLA osnoise histogram$"
>  check "top stop at failed action" \
> -	"timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
> +	"osnoise top -S 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
>  check "hist with continue" \
>  	"osnoise hist -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
>  check "top with continue" \
> -- 
> 2.51.0
> 

Reviewed-by: Wander Lairson Costa <wander@redhat.com>

Re: [PATCH v2 2/3] rtla/tests: Extend action tests to 5s

[Wander Lairson Costa]

On Tue, Oct 07, 2025 at 11:53:40AM +0200, Tomas Glozar wrote:
> In non-BPF mode, it takes up to 1 second for RTLA to notice that tracing
> has been stopped. That means that action tests cannot have a 1 second
> duration, as the SIGALRM will be racing with the threshold overflow.
> 
> Previously, non-BPF mode actions were buggy and always executed
> the action, even when stopping on duration or SIGINT, preventing
> this issue from manifesting. Now that this has been fixed, the tests
> have become flaky, and this has to be adjusted.
> 
> Fixes: 4e26f84abfb ("rtla/tests: Add tests for actions")
> Fixes: 05b7e10687c ("tools/rtla: Add remaining support for osnoise actions")
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/tracing/rtla/tests/osnoise.t  | 4 ++--
>  tools/tracing/rtla/tests/timerlat.t | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
> index e3c89d45a6bb..08196443fef1 100644
> --- a/tools/tracing/rtla/tests/osnoise.t
> +++ b/tools/tracing/rtla/tests/osnoise.t
> @@ -39,9 +39,9 @@ check "hist stop at failed action" \
>  check "top stop at failed action" \
>  	"timerlat top -T 2 --on-threshold shell,command='echo -n abc; false' --on-threshold shell,command='echo -n defgh'" 2 "^abc" "defgh"
>  check "hist with continue" \
> -	"osnoise hist -S 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
> +	"osnoise hist -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
>  check "top with continue" \
> -	"osnoise top -q -S 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
> +	"osnoise top -q -S 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
>  check "hist with trace output at end" \
>  	"osnoise hist -d 1s --on-end trace" 0 "^  Saving trace to osnoise_trace.txt$"
>  check "top with trace output at end" \
> diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t
> index b5d1e7260a9b..b550a6ae2445 100644
> --- a/tools/tracing/rtla/tests/timerlat.t
> +++ b/tools/tracing/rtla/tests/timerlat.t
> @@ -60,9 +60,9 @@ check "hist stop at failed action" \
>  check "top stop at failed action" \
>  	"timerlat top -T 2 --on-threshold shell,command='echo -n 1; false' --on-threshold shell,command='echo -n 2'" 2 "^1ALL"
>  check "hist with continue" \
> -	"timerlat hist -T 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
> +	"timerlat hist -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
>  check "top with continue" \
> -	"timerlat top -q -T 2 -d 1s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
> +	"timerlat top -q -T 2 -d 5s --on-threshold shell,command='echo TestOutput' --on-threshold continue" 0 "^TestOutput$"
>  check "hist with trace output at end" \
>  	"timerlat hist -d 1s --on-end trace" 0 "^  Saving trace to timerlat_trace.txt$"
>  check "top with trace output at end" \
> -- 
> 2.51.0
> 

In general, my pupils dilate when I see time based synchronization.
However, in this case, it seems harmless.

Reviewed-by: Wander Lairson Costa <wander@redhat.com>