[PATCH 0/4] Some RTLA fixes

[PATCH 0/4] Some RTLA fixes

[Daniel Bristot de Oliveira]

The first is a change in the Makefile to make rtla use the kernel
version.

The other fixes are in the error path when rtla fails to enable
the tracers. I got some segmentation faults when trying to use rtla
with kernel versions that do not support multiple instances (i.e.,
5.14 and 5.15) and fixed them. I also found some inconsistency in
error messages and also did a cleanup there.

Daniel Bristot de Oliveira (4):
  rtla: Follow kernel version
  rtla/utils: Fix session duration parsing
  rtla/trace: Error message fixup
  rtla/osnoise: Fix segmentation fault when failing to enable -t

 tools/tracing/rtla/Makefile      | 4 +++-
 tools/tracing/rtla/src/osnoise.c | 3 +++
 tools/tracing/rtla/src/trace.c   | 8 ++++----
 tools/tracing/rtla/src/utils.c   | 4 ++--
 4 files changed, 12 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/4] rtla: Follow kernel version

[Daniel Bristot de Oliveira]

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

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 7c39728d08de..5a1eda617992 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/4] 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/4] 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

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

[Daniel Bristot de Oliveira]

rtla osnoise and timerlat are 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.

Make osnoise_destroy_tool() aware of possible NULL osnoise_tool *,
and do not attempt to destroy it.

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Fixes: 1eceb2fc2ca5 ("rtla/osnoise: Add osnoise top mode")
Fixes: 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
Fixes: a828cd18bc4a ("rtla: Add timerlat tool and timelart top mode")
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/osnoise.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 7b73d1eccd0e..5648f9252e58 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -750,6 +750,9 @@ void osnoise_put_context(struct osnoise_context *context)
  */
 void osnoise_destroy_tool(struct osnoise_tool *top)
 {
+	if (!top)
+		return;
+
 	trace_instance_destroy(&top->trace);
 
 	if (top->context)
-- 
2.34.1

Re: [PATCH 3/4] rtla/trace: Error message fixup

[Steven Rostedt]

On Fri,  4 Feb 2022 17:24:04 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> Use capital and change "tracer %s" to "%s tracer".
> 
> No functional change.

Technically, there is a functional change. Yes, changing output strings is
functional changes. What a non functional change would be, is moving code
around, but the logic stays the same (or adding macro helpers, and such).

I'll keep this as is, but just stating it as an FYI.

-- Steve

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

[Steven Rostedt]

On Fri,  4 Feb 2022 17:24:05 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:

> --- a/tools/tracing/rtla/src/osnoise.c
> +++ b/tools/tracing/rtla/src/osnoise.c
> @@ -750,6 +750,9 @@ void osnoise_put_context(struct osnoise_context *context)
>   */
>  void osnoise_destroy_tool(struct osnoise_tool *top)
>  {
> +	if (!top)
> +		return;
> +
>  	trace_instance_destroy(&top->trace);
>  
>  	if (top->context)

Um, don't you still need to initialize everything to NULL?

i.e.

nt osnoise_top_main(int argc, char **argv)
{
        struct osnoise_top_params *params;
        struct trace_instance *trace;
        struct osnoise_tool *record;
        struct osnoise_tool *tool;
        int return_value = 1;
        int retval;



Does not guarantee that record and tool will be initialized to NULL.

-- Steve

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

[Daniel Bristot de Oliveira]

On 2/4/22 18:44, Steven Rostedt wrote:
> On Fri,  4 Feb 2022 17:24:05 +0100
> Daniel Bristot de Oliveira <bristot@kernel.org> wrote:
> 
>> --- a/tools/tracing/rtla/src/osnoise.c
>> +++ b/tools/tracing/rtla/src/osnoise.c
>> @@ -750,6 +750,9 @@ void osnoise_put_context(struct osnoise_context *context)
>>   */
>>  void osnoise_destroy_tool(struct osnoise_tool *top)
>>  {
>> +	if (!top)
>> +		return;
>> +
>>  	trace_instance_destroy(&top->trace);
>>  
>>  	if (top->context)
> Um, don't you still need to initialize everything to NULL?
> 
> i.e.
> 
> nt osnoise_top_main(int argc, char **argv)
> {
>         struct osnoise_top_params *params;
>         struct trace_instance *trace;
>         struct osnoise_tool *record;
>         struct osnoise_tool *tool;
>         int return_value = 1;
>         int retval;
> 
> 
> 
> Does not guarantee that record and tool will be initialized to NULL.

Aaarrrg, you're right.

As this is not related to the other patches, could you just ignore this one, so
I can re-send alone?

-- Daniel

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

[Steven Rostedt]

On Fri, 4 Feb 2022 18:46:59 +0100
Daniel Bristot de Oliveira <bristot@kernel.org> wrote:
> 
> As this is not related to the other patches, could you just ignore this one, so
> I can re-send alone?

Sure.

-- Steve