[PATCH 1/8] rtla: Follow kernel version

[PATCH 1/8] rtla: Follow kernel version

[Daniel Bristot de Oliveira]

To avoid having commits with new version, it is just easier to follow
kernel verison.

Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
index 2d52ff0bff7d..3dd51d0b1c3b 100644
--- a/tools/tracing/rtla/Makefile
+++ b/tools/tracing/rtla/Makefile
@@ -1,5 +1,6 @@
 NAME	:=	rtla
-VERSION	:=	0.5
+# Follow the kernel version
+VERSION :=	$(shell cat VERSION 2> /dev/null || make -sC ../../.. kernelversion)
 
 # From libtracefs:
 # Makefiles suck: This macro sets a default value of $(2) for the
@@ -85,6 +86,7 @@ clean: doc_clean
 
 tarball: clean
 	rm -rf $(NAME)-$(VERSION) && mkdir $(NAME)-$(VERSION)
+	echo $(VERSION) > $(NAME)-$(VERSION)/VERSION
 	cp -r $(DIRS) $(FILES) $(NAME)-$(VERSION)
 	mkdir $(NAME)-$(VERSION)/Documentation/
 	cp -rp $(SRCTREE)/../../../Documentation/tools/rtla/* $(NAME)-$(VERSION)/Documentation/
-- 
2.34.1

[PATCH 2/8] rtla/utils: Fix session duration parsing

[Daniel Bristot de Oliveira]

Use gmtime to format the duration time. This avoids problems when the
system uses local time different of Pisa's Local Time.

Fixes: b1696371d865 ("rtla: Helper functions for rtla")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 1c9f0eea6166..ffaf8ec84001 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -77,11 +77,11 @@ void get_duration(time_t start_time, char *output, int output_size)
 	time_t duration;
 
 	duration = difftime(now, start_time);
-	tm_info = localtime(&duration);
+	tm_info = gmtime(&duration);
 
 	snprintf(output, output_size, "%3d %02d:%02d:%02d",
 			tm_info->tm_yday,
-			tm_info->tm_hour - 1,
+			tm_info->tm_hour,
 			tm_info->tm_min,
 			tm_info->tm_sec);
 }
-- 
2.34.1

[PATCH 3/8] rtla/osnoise_top: Fix segmentation fault when failing to enable -t

[Daniel Bristot de Oliveira]

rtla osnoise top is causing a segmentation fault when running with
the --trace option on a kernel that does not support multiple
instances. For example:

    [root@f34 rtla]# rtla osnoise top -t
    failed to enable the tracer osnoise
    Could not enable osnoiser tracer for tracing
    Failed to enable the trace instance
    Segmentation fault (core dumped)

This error happens because the exit code of the tools is trying
to destroy the trace instance that failed to be created.

Rearrange the order in which trace instances are destroyed to avoid
this problem.

Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/osnoise_top.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
index 332b2ac205fc..546769bc7ff7 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -546,7 +546,7 @@ int osnoise_top_main(int argc, char **argv)
 						    trace);
 		if (retval < 0) {
 			err_msg("Error iterating on events\n");
-			goto out_top;
+			goto out_trace;
 		}
 
 		if (!params->quiet)
@@ -569,11 +569,12 @@ int osnoise_top_main(int argc, char **argv)
 		}
 	}
 
+out_trace:
+	if (params->trace_output)
+		osnoise_destroy_tool(record);
 out_top:
 	osnoise_free_top(tool->data);
 	osnoise_destroy_tool(tool);
-	if (params->trace_output)
-		osnoise_destroy_tool(record);
 out_exit:
 	exit(return_value);
 }
-- 
2.34.1

Re: [PATCH 3/8] rtla/osnoise_top: Fix segmentation fault when failing to enable -t

[Steven Rostedt]

On Mon, 24 Jan 2022 15:24:06 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> rtla osnoise top is causing a segmentation fault when running with
> the --trace option on a kernel that does not support multiple
> instances. For example:
> 
>     [root@f34 rtla]# rtla osnoise top -t
>     failed to enable the tracer osnoise
>     Could not enable osnoiser tracer for tracing
>     Failed to enable the trace instance
>     Segmentation fault (core dumped)
> 
> This error happens because the exit code of the tools is trying
> to destroy the trace instance that failed to be created.
> 
> Rearrange the order in which trace instances are destroyed to avoid
> this problem.
> 
> Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
> Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-trace-devel@vger.kernel.org
> Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
> ---
>  tools/tracing/rtla/src/osnoise_top.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
> index 332b2ac205fc..546769bc7ff7 100644
> --- a/tools/tracing/rtla/src/osnoise_top.c
> +++ b/tools/tracing/rtla/src/osnoise_top.c
> @@ -546,7 +546,7 @@ int osnoise_top_main(int argc, char **argv)
>  						    trace);
>  		if (retval < 0) {
>  			err_msg("Error iterating on events\n");
> -			goto out_top;
> +			goto out_trace;
>  		}
>  
>  		if (!params->quiet)
> @@ -569,11 +569,12 @@ int osnoise_top_main(int argc, char **argv)
>  		}
>  	}
>  
> +out_trace:
> +	if (params->trace_output)
> +		osnoise_destroy_tool(record);
>  out_top:
>  	osnoise_free_top(tool->data);
>  	osnoise_destroy_tool(tool);
> -	if (params->trace_output)
> -		osnoise_destroy_tool(record);

Wouldn't these four patches be more robust if you just initialized record
(and tool) to NULL, and change osnoise_destroy_tool() to:

void osnoise_destroy_tool(struct osnoise_tool *top)
{
	if (!top)
		return;

        trace_instance_destroy(&top->trace);

        if (top->context)
                osnoise_put_context(top->context);

        free(top);
}

Then you don't need these extra labels and if statements in the main code.

-- Steve


>  out_exit:
>  	exit(return_value);
>  }

Re: [PATCH 3/8] rtla/osnoise_top: Fix segmentation fault when failing to enable -t

[Steven Rostedt]

On Thu, 3 Feb 2022 11:41:26 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Wouldn't these four patches be more robust if you just initialized record
> (and tool) to NULL, and change osnoise_destroy_tool() to:

And if you do this, it should be one patch, not four.

-- Steve

> 
> void osnoise_destroy_tool(struct osnoise_tool *top)
> {
> 	if (!top)
> 		return;
> 
>         trace_instance_destroy(&top->trace);
> 
>         if (top->context)
>                 osnoise_put_context(top->context);
> 
>         free(top);
> }
> 
> Then you don't need these extra labels and if statements in the main code.

Re: [PATCH 3/8] rtla/osnoise_top: Fix segmentation fault when failing to enable -t

[Daniel Bristot de Oliveira]

On 2/3/22 17:43, Steven Rostedt wrote:
> On Thu, 3 Feb 2022 11:41:26 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> Wouldn't these four patches be more robust if you just initialized record
>> (and tool) to NULL, and change osnoise_destroy_tool() to:
> And if you do this, it should be one patch, not four.


Yeah, it works. The order would still be wrong, but it would be just an esthetic
thing.

I will send a v2 removing these four patches, and adding a patch with your
suggestion.

[ thinking aloud, is it possible to have multiple Fixes:? well, adding just one
would also solve the issue, and... we are still in the same release ]

Thanks,
-- Daniel

Re: [PATCH 3/8] rtla/osnoise_top: Fix segmentation fault when failing to enable -t

[Steven Rostedt]

On Thu, 3 Feb 2022 18:30:39 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> [ thinking aloud, is it possible to have multiple Fixes:? well, adding just one
> would also solve the issue, and... we are still in the same release ]

I've added multiple Fixes tags before. So sure.

-- Steve

[PATCH 4/8] rtla/osnoise_hist: Fix segmentation fault when failing to enable -t

[Daniel Bristot de Oliveira]

rtla osnoise hist is causing a segmentation fault when running with
the --trace option on a kernel that does not support multiple
instances. For example:

    [root@f34 rtla]# rtla osnoise hist -t daniel -s 10
    failed to enable the tracer osnoise
    Could not enable osnoiser tracer for tracing
    Failed to enable the trace instance
    Segmentation fault (core dumped)

This error happens because the exit code of the tools is trying
to destroy the trace instance that failed to be created.

Rearrange the order in which trace instances are destroyed to avoid
this problem.

Fixes: 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/osnoise_hist.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index 180fcbe423cd..60bf3bb4c460 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -768,7 +768,7 @@ int osnoise_hist_main(int argc, char *argv[])
 						    trace);
 		if (retval < 0) {
 			err_msg("Error iterating on events\n");
-			goto out_hist;
+			goto out_trace;
 		}
 
 		if (!tracefs_trace_is_on(trace->inst))
@@ -789,12 +789,13 @@ int osnoise_hist_main(int argc, char *argv[])
 		}
 	}
 
+out_trace:
+	if (params->trace_output)
+		osnoise_destroy_tool(record);
 out_hist:
 	osnoise_free_histogram(tool->data);
 out_destroy:
 	osnoise_destroy_tool(tool);
-	if (params->trace_output)
-		osnoise_destroy_tool(record);
 	free(params);
 out_exit:
 	exit(return_value);
-- 
2.34.1

[PATCH 5/8] rtla/timerlat_top: Fix segmentation fault when failing to enable -t

[Daniel Bristot de Oliveira]

rtla timerlat top is causing a segmentation fault when running with
the --trace option on a kernel that does not support multiple
instances. For example:

    [root@f34 rtla]# rtla timerlat top -t
    failed to enable the tracer timerlat
    Could not enable osnoiser tracer for tracing
    Failed to enable the trace instance
    Segmentation fault (core dumped)

This error happens because the exit code of the tools is trying
to destroy the trace instance that failed to be created.

Rearrange the order in which trace instances are destroyed to avoid
this problem.

Fixes: a828cd18bc4a ("rtla: Add timerlat tool and timelart top mode")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/timerlat_top.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index 1ebd5291539c..323fd1f88313 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -584,7 +584,7 @@ int timerlat_top_main(int argc, char *argv[])
 						    trace);
 		if (retval < 0) {
 			err_msg("Error iterating on events\n");
-			goto out_top;
+			goto out_trace;
 		}
 
 		if (!params->quiet)
@@ -606,12 +606,12 @@ int timerlat_top_main(int argc, char *argv[])
 			save_trace_to_file(record->trace.inst, params->trace_output);
 		}
 	}
-
+out_trace:
+	if (params->trace_output)
+		osnoise_destroy_tool(record);
 out_top:
 	timerlat_free_top(top->data);
 	osnoise_destroy_tool(top);
-	if (params->trace_output)
-		osnoise_destroy_tool(record);
 	free(params);
 out_exit:
 	exit(return_value);
-- 
2.34.1

[PATCH 6/8] rtla/timerlat_hist: Fix segmentation fault when failing to enable -t

[Daniel Bristot de Oliveira]

rtla timerlat hist is causing a segmentation fault when running with
the --trace option on a kernel that does not support multiple
instances. For example:

    [root@f34 rtla]# rtla timerlat hist -t
    failed to enable the tracer timerlat
    Could not enable osnoiser tracer for tracing
    Failed to enable the trace instance
    Segmentation fault (core dumped)

This error happens because the exit code of the tools is trying
to destroy the trace instance that failed to be created.

Rearrange the order in which trace instances are destroyed to avoid
this problem.

Fixes: (1eeb6328e8b3 "rtla/timerlat: Add timerlat hist mode")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/timerlat_hist.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 235f9620ef3d..74ccb6351ad2 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -792,7 +792,7 @@ int timerlat_hist_main(int argc, char *argv[])
 						    trace);
 		if (retval < 0) {
 			err_msg("Error iterating on events\n");
-			goto out_hist;
+			goto out_trace;
 		}
 
 		if (!tracefs_trace_is_on(trace->inst))
@@ -811,11 +811,12 @@ int timerlat_hist_main(int argc, char *argv[])
 		}
 	}
 
+out_trace:
+	if (params->trace_output)
+		osnoise_destroy_tool(record);
 out_hist:
 	timerlat_free_histogram(tool->data);
 	osnoise_destroy_tool(tool);
-	if (params->trace_output)
-		osnoise_destroy_tool(record);
 	free(params);
 out_exit:
 	exit(return_value);
-- 
2.34.1

[PATCH 7/8] rtla/osnoise: Fix error message when failing to enable trace instance

[Daniel Bristot de Oliveira]

When a trace instance creation fails, tools are printing:

	Could not enable -> osnoiser <- tracer for tracing

Print the actual (and correct) name of the tracer it fails to enable.

Fixes: b1696371d865 ("rtla: Helper functions for rtla")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/osnoise.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 7b73d1eccd0e..c1368315d508 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -807,7 +807,7 @@ struct osnoise_tool *osnoise_init_trace_tool(char *tracer)
 
 	retval = enable_tracer_by_name(trace->trace.inst, tracer);
 	if (retval) {
-		err_msg("Could not enable osnoiser tracer for tracing\n");
+		err_msg("Could not enable %s tracer for tracing\n", tracer);
 		goto out_err;
 	}
 
-- 
2.34.1

[PATCH 8/8] rtla/trace: Error message fixup

[Daniel Bristot de Oliveira]

Use capital and change "tracer %s" to "%s tracer".

No functional change.

Fixes: b1696371d865 ("rtla: Helper functions for rtla")
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-devel@vger.kernel.org
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
---
 tools/tracing/rtla/src/trace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 107a0c6387f7..83de259abcc1 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -20,14 +20,14 @@ int enable_tracer_by_name(struct tracefs_instance *inst, const char *tracer_name
 
 	tracer = TRACEFS_TRACER_CUSTOM;
 
-	debug_msg("enabling %s tracer\n", tracer_name);
+	debug_msg("Enabling %s tracer\n", tracer_name);
 
 	retval = tracefs_tracer_set(inst, tracer, tracer_name);
 	if (retval < 0) {
 		if (errno == ENODEV)
-			err_msg("tracer %s not found!\n", tracer_name);
+			err_msg("Tracer %s not found!\n", tracer_name);
 
-		err_msg("failed to enable the tracer %s\n", tracer_name);
+		err_msg("Failed to enable the %s tracer\n", tracer_name);
 		return -1;
 	}
 
@@ -44,7 +44,7 @@ void disable_tracer(struct tracefs_instance *inst)
 
 	retval = tracefs_tracer_set(inst, t);
 	if (retval < 0)
-		err_msg("oops, error disabling tracer\n");
+		err_msg("Oops, error disabling tracer\n");
 }
 
 /*
-- 
2.34.1