* [PATCH] perf hist: Fix memory leak of a perf_hpp_fmt
@ 2021-11-18 7:12 Ian Rogers
2021-11-18 7:58 ` kajoljain
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2021-11-18 7:12 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-perf-users, linux-kernel
Cc: Stephane Eranian, Ian Rogers
perf_hpp__column_unregister removes an entry from a list but doesn't
free the memory causing a memory leak spotted by leak sanitizer. Add the
free while at the same time reducing the scope of the function to
static.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/ui/hist.c | 28 ++++++++++++++--------------
tools/perf/util/hist.h | 1 -
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index c1f24d004852..5075ecead5f3 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -535,6 +535,18 @@ struct perf_hpp_list perf_hpp_list = {
#undef __HPP_SORT_ACC_FN
#undef __HPP_SORT_RAW_FN
+static void fmt_free(struct perf_hpp_fmt *fmt)
+{
+ /*
+ * At this point fmt should be completely
+ * unhooked, if not it's a bug.
+ */
+ BUG_ON(!list_empty(&fmt->list));
+ BUG_ON(!list_empty(&fmt->sort_list));
+
+ if (fmt->free)
+ fmt->free(fmt);
+}
void perf_hpp__init(void)
{
@@ -598,9 +610,10 @@ void perf_hpp_list__prepend_sort_field(struct perf_hpp_list *list,
list_add(&format->sort_list, &list->sorts);
}
-void perf_hpp__column_unregister(struct perf_hpp_fmt *format)
+static void perf_hpp__column_unregister(struct perf_hpp_fmt *format)
{
list_del_init(&format->list);
+ fmt_free(format);
}
void perf_hpp__cancel_cumulate(void)
@@ -672,19 +685,6 @@ void perf_hpp__append_sort_keys(struct perf_hpp_list *list)
}
-static void fmt_free(struct perf_hpp_fmt *fmt)
-{
- /*
- * At this point fmt should be completely
- * unhooked, if not it's a bug.
- */
- BUG_ON(!list_empty(&fmt->list));
- BUG_ON(!list_empty(&fmt->sort_list));
-
- if (fmt->free)
- fmt->free(fmt);
-}
-
void perf_hpp__reset_output_field(struct perf_hpp_list *list)
{
struct perf_hpp_fmt *fmt, *tmp;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 5343b62476e6..621f35ae1efa 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -369,7 +369,6 @@ enum {
};
void perf_hpp__init(void);
-void perf_hpp__column_unregister(struct perf_hpp_fmt *format);
void perf_hpp__cancel_cumulate(void);
void perf_hpp__setup_output_field(struct perf_hpp_list *list);
void perf_hpp__reset_output_field(struct perf_hpp_list *list);
--
2.34.0.rc1.387.gb447b232ab-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf hist: Fix memory leak of a perf_hpp_fmt
2021-11-18 7:12 [PATCH] perf hist: Fix memory leak of a perf_hpp_fmt Ian Rogers
@ 2021-11-18 7:58 ` kajoljain
2021-11-18 13:16 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: kajoljain @ 2021-11-18 7:58 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel
Cc: Stephane Eranian
On 11/18/21 12:42 PM, Ian Rogers wrote:
> perf_hpp__column_unregister removes an entry from a list but doesn't
> free the memory causing a memory leak spotted by leak sanitizer. Add the
> free while at the same time reducing the scope of the function to
> static.
Patch looks good to me.
Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
Thanks,
Kajol jain
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/ui/hist.c | 28 ++++++++++++++--------------
> tools/perf/util/hist.h | 1 -
> 2 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index c1f24d004852..5075ecead5f3 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -535,6 +535,18 @@ struct perf_hpp_list perf_hpp_list = {
> #undef __HPP_SORT_ACC_FN
> #undef __HPP_SORT_RAW_FN
>
> +static void fmt_free(struct perf_hpp_fmt *fmt)
> +{
> + /*
> + * At this point fmt should be completely
> + * unhooked, if not it's a bug.
> + */
> + BUG_ON(!list_empty(&fmt->list));
> + BUG_ON(!list_empty(&fmt->sort_list));
> +
> + if (fmt->free)
> + fmt->free(fmt);
> +}
>
> void perf_hpp__init(void)
> {
> @@ -598,9 +610,10 @@ void perf_hpp_list__prepend_sort_field(struct perf_hpp_list *list,
> list_add(&format->sort_list, &list->sorts);
> }
>
> -void perf_hpp__column_unregister(struct perf_hpp_fmt *format)
> +static void perf_hpp__column_unregister(struct perf_hpp_fmt *format)
> {
> list_del_init(&format->list);
> + fmt_free(format);
> }
>
> void perf_hpp__cancel_cumulate(void)
> @@ -672,19 +685,6 @@ void perf_hpp__append_sort_keys(struct perf_hpp_list *list)
> }
>
>
> -static void fmt_free(struct perf_hpp_fmt *fmt)
> -{
> - /*
> - * At this point fmt should be completely
> - * unhooked, if not it's a bug.
> - */
> - BUG_ON(!list_empty(&fmt->list));
> - BUG_ON(!list_empty(&fmt->sort_list));
> -
> - if (fmt->free)
> - fmt->free(fmt);
> -}
> -
> void perf_hpp__reset_output_field(struct perf_hpp_list *list)
> {
> struct perf_hpp_fmt *fmt, *tmp;
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 5343b62476e6..621f35ae1efa 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -369,7 +369,6 @@ enum {
> };
>
> void perf_hpp__init(void);
> -void perf_hpp__column_unregister(struct perf_hpp_fmt *format);
> void perf_hpp__cancel_cumulate(void);
> void perf_hpp__setup_output_field(struct perf_hpp_list *list);
> void perf_hpp__reset_output_field(struct perf_hpp_list *list);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf hist: Fix memory leak of a perf_hpp_fmt
2021-11-18 7:58 ` kajoljain
@ 2021-11-18 13:16 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-18 13:16 UTC (permalink / raw)
To: kajoljain
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users,
linux-kernel, Stephane Eranian
Em Thu, Nov 18, 2021 at 01:28:11PM +0530, kajoljain escreveu:
>
>
> On 11/18/21 12:42 PM, Ian Rogers wrote:
> > perf_hpp__column_unregister removes an entry from a list but doesn't
> > free the memory causing a memory leak spotted by leak sanitizer. Add the
> > free while at the same time reducing the scope of the function to
> > static.
>
> Patch looks good to me.
>
> Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
Thanks, applied.
- Arnaldo
> Thanks,
> Kajol jain
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/ui/hist.c | 28 ++++++++++++++--------------
> > tools/perf/util/hist.h | 1 -
> > 2 files changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> > index c1f24d004852..5075ecead5f3 100644
> > --- a/tools/perf/ui/hist.c
> > +++ b/tools/perf/ui/hist.c
> > @@ -535,6 +535,18 @@ struct perf_hpp_list perf_hpp_list = {
> > #undef __HPP_SORT_ACC_FN
> > #undef __HPP_SORT_RAW_FN
> >
> > +static void fmt_free(struct perf_hpp_fmt *fmt)
> > +{
> > + /*
> > + * At this point fmt should be completely
> > + * unhooked, if not it's a bug.
> > + */
> > + BUG_ON(!list_empty(&fmt->list));
> > + BUG_ON(!list_empty(&fmt->sort_list));
> > +
> > + if (fmt->free)
> > + fmt->free(fmt);
> > +}
> >
> > void perf_hpp__init(void)
> > {
> > @@ -598,9 +610,10 @@ void perf_hpp_list__prepend_sort_field(struct perf_hpp_list *list,
> > list_add(&format->sort_list, &list->sorts);
> > }
> >
> > -void perf_hpp__column_unregister(struct perf_hpp_fmt *format)
> > +static void perf_hpp__column_unregister(struct perf_hpp_fmt *format)
> > {
> > list_del_init(&format->list);
> > + fmt_free(format);
> > }
> >
> > void perf_hpp__cancel_cumulate(void)
> > @@ -672,19 +685,6 @@ void perf_hpp__append_sort_keys(struct perf_hpp_list *list)
> > }
> >
> >
> > -static void fmt_free(struct perf_hpp_fmt *fmt)
> > -{
> > - /*
> > - * At this point fmt should be completely
> > - * unhooked, if not it's a bug.
> > - */
> > - BUG_ON(!list_empty(&fmt->list));
> > - BUG_ON(!list_empty(&fmt->sort_list));
> > -
> > - if (fmt->free)
> > - fmt->free(fmt);
> > -}
> > -
> > void perf_hpp__reset_output_field(struct perf_hpp_list *list)
> > {
> > struct perf_hpp_fmt *fmt, *tmp;
> > diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> > index 5343b62476e6..621f35ae1efa 100644
> > --- a/tools/perf/util/hist.h
> > +++ b/tools/perf/util/hist.h
> > @@ -369,7 +369,6 @@ enum {
> > };
> >
> > void perf_hpp__init(void);
> > -void perf_hpp__column_unregister(struct perf_hpp_fmt *format);
> > void perf_hpp__cancel_cumulate(void);
> > void perf_hpp__setup_output_field(struct perf_hpp_list *list);
> > void perf_hpp__reset_output_field(struct perf_hpp_list *list);
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-18 13:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 7:12 [PATCH] perf hist: Fix memory leak of a perf_hpp_fmt Ian Rogers
2021-11-18 7:58 ` kajoljain
2021-11-18 13:16 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.