[PATCH v2 3/3] rtla: fix -a overriding -t argument

Ivan Pravdin posted 3 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 3/3] rtla: fix -a overriding -t argument
Posted by Ivan Pravdin 1 month, 3 weeks ago
When running rtla as

    `rtla <timerlat|osnoise> <top|hist> -t custom_file.txt -a 100`

-a options override trace output filename specified by -t option.
Running the command above will create <timerlat|osnoise>_trace.txt file
instead of custom_file.txt. Fix this by making sure that -a option does
not override trace output filename even if it's passed after trace
output filename is specified.

Remove clear_terminal in timerlat top as it removes "Saving trace to <filename>"
line from the terminal and is not used in other modes.

Fixes: 173a3b014827 ("rtla/timerlat: Add the automatic trace option")
Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.com>
---
 tools/tracing/rtla/src/osnoise_hist.c  |  8 +++++---
 tools/tracing/rtla/src/osnoise_top.c   |  8 +++++---
 tools/tracing/rtla/src/timerlat_hist.c |  8 +++++---
 tools/tracing/rtla/src/timerlat_top.c  | 20 +++++---------------
 4 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index 298640d92434..53655bccbbad 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -543,7 +543,8 @@ static struct osnoise_params
 			params->threshold = 1;
 
 			/* set trace */
-			params->trace_output = "osnoise_trace.txt";
+			if (!params->trace_output)
+				params->trace_output = "osnoise_trace.txt";
 
 			break;
 		case 'b':
@@ -640,10 +641,11 @@ static struct osnoise_params
 					params->trace_output = &optarg[1];
 				else
 					params->trace_output = &optarg[0];
-			} else if (optind < argc && argv[optind][0] != '0')
+			} else if (optind < argc && argv[optind][0] != '0') {
 				params->trace_output = argv[optind];
-			else
+			} else {
 				params->trace_output = "osnoise_trace.txt";
+			}
 			break;
 		case '0': /* no header */
 			params->no_header = 1;
diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
index 70924242bcdd..db61a0c970e9 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -383,7 +383,8 @@ struct osnoise_params *osnoise_top_parse_args(int argc, char **argv)
 			params->threshold = 1;
 
 			/* set trace */
-			params->trace_output = "osnoise_trace.txt";
+			if (!params->trace_output)
+				params->trace_output = "osnoise_trace.txt";
 
 			break;
 		case 'c':
@@ -470,10 +471,11 @@ struct osnoise_params *osnoise_top_parse_args(int argc, char **argv)
 					params->trace_output = &optarg[1];
 				else
 					params->trace_output = &optarg[0];
-			} else if (optind < argc && argv[optind][0] != '-')
+			} else if (optind < argc && argv[optind][0] != '-') {
 				params->trace_output = argv[optind];
-			else
+			} else {
 				params->trace_output = "osnoise_trace.txt";
+			}
 			break;
 		case 'T':
 			params->threshold = get_llong_from_str(optarg);
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 2a48a076d941..123b7f89f80a 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -874,7 +874,8 @@ static struct timerlat_params
 			params->print_stack = auto_thresh;
 
 			/* set trace */
-			trace_output = "timerlat_trace.txt";
+			if (!trace_output)
+				trace_output = "timerlat_trace.txt";
 
 			break;
 		case 'c':
@@ -972,10 +973,11 @@ static struct timerlat_params
 					trace_output = &optarg[1];
 				else
 					trace_output = &optarg[0];
-			} else if (optind < argc && argv[optind][0] != '-')
+			} else if (optind < argc && argv[optind][0] != '-') {
 				trace_output = argv[optind];
-			else
+			} else {
 				trace_output = "timerlat_trace.txt";
+			}
 			break;
 		case 'u':
 			params->user_workload = 1;
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index e45d9978744c..4d7c3d74ac50 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -423,15 +423,6 @@ timerlat_top_print_sum(struct osnoise_tool *top, struct timerlat_top_cpu *summar
 	}
 }
 
-/*
- * clear_terminal - clears the output terminal
- */
-static void clear_terminal(struct trace_seq *seq)
-{
-	if (!config_debug)
-		trace_seq_printf(seq, "\033c");
-}
-
 /*
  * timerlat_print_stats - print data for all cpus
  */
@@ -449,9 +440,6 @@ timerlat_print_stats(struct timerlat_params *params, struct osnoise_tool *top)
 	if (nr_cpus == -1)
 		nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
 
-	if (!params->quiet)
-		clear_terminal(trace->seq);
-
 	timerlat_top_reset_sum(&summary);
 
 	timerlat_top_header(params, top);
@@ -625,7 +613,8 @@ static struct timerlat_params
 			params->print_stack = auto_thresh;
 
 			/* set trace */
-			trace_output = "timerlat_trace.txt";
+			if (!trace_output)
+				trace_output = "timerlat_trace.txt";
 
 			break;
 		case '5':
@@ -729,10 +718,11 @@ static struct timerlat_params
 					trace_output = &optarg[1];
 				else
 					trace_output = &optarg[0];
-			} else if (optind < argc && argv[optind][0] != '-')
+			} else if (optind < argc && argv[optind][0] != '-') {
 				trace_output = argv[optind];
-			else
+			} else {
 				trace_output = "timerlat_trace.txt";
+			}
 			break;
 		case 'u':
 			params->user_workload = true;
-- 
2.48.1
Re: [PATCH v2 3/3] rtla: fix -a overriding -t argument
Posted by Tomas Glozar 1 month ago
út 12. 8. 2025 v 19:22 odesílatel Ivan Pravdin
<ipravdin.official@gmail.com> napsal:
>
> When running rtla as
>
>     `rtla <timerlat|osnoise> <top|hist> -t custom_file.txt -a 100`
>
> -a options override trace output filename specified by -t option.
> Running the command above will create <timerlat|osnoise>_trace.txt file
> instead of custom_file.txt. Fix this by making sure that -a option does
> not override trace output filename even if it's passed after trace
> output filename is specified.
>

Yes, this should not be overridden, it's a bug in both the old
implementation and the new one using actions. Thank you for the fix.

> Remove clear_terminal in timerlat top as it removes "Saving trace to <filename>"
> line from the terminal and is not used in other modes.
>

This is not the correct fix for the issue. clear_terminal is needed in
timerlat top (and osnoise top) to clear the terminal before the
updated status is printed, the fix removes it without a replacement,
breaking this behavior. Also, this is a different issue that appeared
only when the actions patchset [1] was merged, and should go into a
separate patch with a different Fixes tag.

[1] https://lore.kernel.org/linux-trace-kernel/20250626123405.1496931-1-tglozar@redhat.com/T/#t

Tomas
Re: [PATCH v2 3/3] rtla: fix -a overriding -t argument
Posted by Ivan Pravdin 1 month ago
On Mon, Sep 01, 2025 at 04:23:55PM +0200, Tomas Glozar wrote:
> út 12. 8. 2025 v 19:22 odesílatel Ivan Pravdin
> <ipravdin.official@gmail.com> napsal:
> >
> > When running rtla as
> >
> >     `rtla <timerlat|osnoise> <top|hist> -t custom_file.txt -a 100`
> >
> > -a options override trace output filename specified by -t option.
> > Running the command above will create <timerlat|osnoise>_trace.txt file
> > instead of custom_file.txt. Fix this by making sure that -a option does
> > not override trace output filename even if it's passed after trace
> > output filename is specified.
> >
> 
> Yes, this should not be overridden, it's a bug in both the old
> implementation and the new one using actions. Thank you for the fix.
> 
> > Remove clear_terminal in timerlat top as it removes "Saving trace to <filename>"
> > line from the terminal and is not used in other modes.
> >
> 
> This is not the correct fix for the issue. clear_terminal is needed in
> timerlat top (and osnoise top) to clear the terminal before the
> updated status is printed, the fix removes it without a replacement,
> breaking this behavior. Also, this is a different issue that appeared
> only when the actions patchset [1] was merged, and should go into a
> separate patch with a different Fixes tag.
> 
> [1] https://lore.kernel.org/linux-trace-kernel/20250626123405.1496931-1-tglozar@redhat.com/T/#t

Makes sense. This problem indeed does not happend for osnoise. I will
update it in the next version of the patch.

> 
> Tomas
> 

	Ivan Pravdin