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 <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Daniel Bristot de Oliveira <[email protected]>
---
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
On 2/3/22 17:43, Steven Rostedt wrote:
> On Thu, 3 Feb 2022 11:41:26 -0500
> Steven Rostedt <[email protected]> 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
On Thu, 3 Feb 2022 11:41:26 -0500
Steven Rostedt <[email protected]> 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.
On Mon, 24 Jan 2022 15:24:06 +0100
Daniel Bristot de Oliveira <[email protected]> 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 <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Daniel Bristot de Oliveira <[email protected]>
> ---
> 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);
> }