| Message ID | 20190305143924.11056-3-ykaradzhov@vmware.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3336de24917f8e3334c89606e2c0ff25650eedb5 |
| Headers | show |
| Series | Improvements and fixes for sched_switch plugin | expand |
On Tue, Mar 05, 2019 at 04:39:23PM +0200, Yordan Karadzhov wrote: > When the sched_events plugin fails to initialize not all the > memory allocated for the context of the plugin is freed properly. > The problem gets fixed by using the free_context helper function > defined in the previous patch. > > Fixes: 4f392730e ("kernel-shark-qt: Make Sched event plugin use ...") > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > --- > kernel-shark/src/plugins/sched_events.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/kernel-shark/src/plugins/sched_events.c b/kernel-shark/src/plugins/sched_events.c > index 0d6de2d..fe13e6a 100644 > --- a/kernel-shark/src/plugins/sched_events.c > +++ b/kernel-shark/src/plugins/sched_events.c > @@ -74,8 +74,12 @@ static bool plugin_sched_init_context(struct kshark_context *kshark_ctx) > > event = tep_find_event_by_name(plugin_ctx->pevent, > "sched", "sched_switch"); > - if (!event) > + if (!event) { > + plugin_free_context(plugin_ctx); > + plugin_sched_context_handler = NULL; > + Nit: drop the above empty line? > return false; > + } > > plugin_ctx->sched_switch_event = event; > plugin_ctx->sched_switch_next_field = > @@ -320,11 +324,8 @@ static int plugin_sched_init(struct kshark_context *kshark_ctx) > { > struct plugin_sched_context *plugin_ctx; > > - if (!plugin_sched_init_context(kshark_ctx)) { > - free(plugin_sched_context_handler); > - plugin_sched_context_handler = NULL; > + if (!plugin_sched_init_context(kshark_ctx)) > return 0; > - } > > plugin_ctx = plugin_sched_context_handler; Looks good to me. Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com> -- Slavi
On Tue, 5 Mar 2019 16:39:23 +0200 Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > When the sched_events plugin fails to initialize not all the > memory allocated for the context of the plugin is freed properly. > The problem gets fixed by using the free_context helper function > defined in the previous patch. > > Fixes: 4f392730e ("kernel-shark-qt: Make Sched event plugin use ...") I took the patch, but don't truncate the fixes line. It's fine to go over 80 characters: Fixes: 4f392730e ("kernel-shark-qt: Make Sched event plugin use its own data collections") -- Steve > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > --- > kernel-shark/src/plugins/sched_events.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/kernel-shark/src/plugins/sched_events.c b/kernel-shark/src/plugins/sched_events.c > index 0d6de2d..fe13e6a 100644 > --- a/kernel-shark/src/plugins/sched_events.c > +++ b/kernel-shark/src/plugins/sched_events.c > @@ -74,8 +74,12 @@ static bool plugin_sched_init_context(struct kshark_context *kshark_ctx) > > event = tep_find_event_by_name(plugin_ctx->pevent, > "sched", "sched_switch"); > - if (!event) > + if (!event) { > + plugin_free_context(plugin_ctx); > + plugin_sched_context_handler = NULL; > + > return false; > + } > > plugin_ctx->sched_switch_event = event; > plugin_ctx->sched_switch_next_field = > @@ -320,11 +324,8 @@ static int plugin_sched_init(struct kshark_context *kshark_ctx) > { > struct plugin_sched_context *plugin_ctx; > > - if (!plugin_sched_init_context(kshark_ctx)) { > - free(plugin_sched_context_handler); > - plugin_sched_context_handler = NULL; > + if (!plugin_sched_init_context(kshark_ctx)) > return 0; > - } > > plugin_ctx = plugin_sched_context_handler; >
On Tue, 5 Mar 2019 17:56:28 +0200 Slavomir Kaslev <kaslevs@vmware.com> wrote: > On Tue, Mar 05, 2019 at 04:39:23PM +0200, Yordan Karadzhov wrote: > > When the sched_events plugin fails to initialize not all the > > memory allocated for the context of the plugin is freed properly. > > The problem gets fixed by using the free_context helper function > > defined in the previous patch. > > > > Fixes: 4f392730e ("kernel-shark-qt: Make Sched event plugin use ...") > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > > --- > > kernel-shark/src/plugins/sched_events.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/kernel-shark/src/plugins/sched_events.c b/kernel-shark/src/plugins/sched_events.c > > index 0d6de2d..fe13e6a 100644 > > --- a/kernel-shark/src/plugins/sched_events.c > > +++ b/kernel-shark/src/plugins/sched_events.c > > @@ -74,8 +74,12 @@ static bool plugin_sched_init_context(struct kshark_context *kshark_ctx) > > > > event = tep_find_event_by_name(plugin_ctx->pevent, > > "sched", "sched_switch"); > > - if (!event) > > + if (!event) { > > + plugin_free_context(plugin_ctx); > > + plugin_sched_context_handler = NULL; > > + > > Nit: drop the above empty line? Yeah, it would be better to do that. But for now, I'll take the patch anyway. Perhaps clean it up later. > > > return false; > > + } > > > > plugin_ctx->sched_switch_event = event; > > plugin_ctx->sched_switch_next_field = > > @@ -320,11 +324,8 @@ static int plugin_sched_init(struct kshark_context *kshark_ctx) > > { > > struct plugin_sched_context *plugin_ctx; > > > > - if (!plugin_sched_init_context(kshark_ctx)) { > > - free(plugin_sched_context_handler); > > - plugin_sched_context_handler = NULL; > > + if (!plugin_sched_init_context(kshark_ctx)) > > return 0; > > - } > > > > plugin_ctx = plugin_sched_context_handler; > > Looks good to me. > > Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com> Thanks Slavomir! -- Steve
diff --git a/kernel-shark/src/plugins/sched_events.c b/kernel-shark/src/plugins/sched_events.c index 0d6de2d..fe13e6a 100644 --- a/kernel-shark/src/plugins/sched_events.c +++ b/kernel-shark/src/plugins/sched_events.c @@ -74,8 +74,12 @@ static bool plugin_sched_init_context(struct kshark_context *kshark_ctx) event = tep_find_event_by_name(plugin_ctx->pevent, "sched", "sched_switch"); - if (!event) + if (!event) { + plugin_free_context(plugin_ctx); + plugin_sched_context_handler = NULL; + return false; + } plugin_ctx->sched_switch_event = event; plugin_ctx->sched_switch_next_field = @@ -320,11 +324,8 @@ static int plugin_sched_init(struct kshark_context *kshark_ctx) { struct plugin_sched_context *plugin_ctx; - if (!plugin_sched_init_context(kshark_ctx)) { - free(plugin_sched_context_handler); - plugin_sched_context_handler = NULL; + if (!plugin_sched_init_context(kshark_ctx)) return 0; - } plugin_ctx = plugin_sched_context_handler;
When the sched_events plugin fails to initialize not all the memory allocated for the context of the plugin is freed properly. The problem gets fixed by using the free_context helper function defined in the previous patch. Fixes: 4f392730e ("kernel-shark-qt: Make Sched event plugin use ...") Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> --- kernel-shark/src/plugins/sched_events.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)