| Message ID | 264ff7941b7551ec0b6e5862e40cf3dd593d0ff0.1643990447.git.bristot@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Some RTLA fixes | expand |
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
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
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
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)
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(+)