perf hist: Avoid 'struct hist_entry_iter' mem_info memory leak

'struct mem_info' is reference counted while 'struct branch_info' and
he_cache (struct hist_entry **) are not.

Break apart the priv field in 'struct hist_entry_iter' so that we can
know which values are owned by the iter and do the appropriate free or
put.

Move hide_unresolved to marginally shrink the size of the now grown
struct.

Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ben Gainey <ben.gainey@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Li Dong <lidong@vivo.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Paran Lee <p4ranlee@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Sun Haiyong <sunhaiyong@loongson.cn>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Yanteng Si <siyanteng@loongson.cn>
Cc: Yicong Yang <yangyicong@hisilicon.com>
Link: https://lore.kernel.org/r/20240507183545.1236093-9-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 00814d42d5f108da169f6af42e03aaaefcb147b4..2e9e193179dd54dbdc7352dfb187ce68cdc53a1f 100644 (file)
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -476,13 +476,6 @@ static int hist_entry__init(struct hist_entry *he,
                he->branch_info->to.ms.map = map__get(he->branch_info->to.ms.map);
        }
 
-       if (he->mem_info) {
-               mem_info__iaddr(he->mem_info)->ms.map =
-                       map__get(mem_info__iaddr(he->mem_info)->ms.map);
-               mem_info__daddr(he->mem_info)->ms.map =
-                       map__get(mem_info__daddr(he->mem_info)->ms.map);
-       }
-
        if (hist_entry__has_callchains(he) && symbol_conf.use_callchain)
                callchain_init(he->callchain);
 
@@ -574,7 +567,6 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
                        he = NULL;
                }
        }
-
        return he;
 }
 
@@ -747,7 +739,7 @@ __hists__add_entry(struct hists *hists,
                .filtered = symbol__parent_filter(sym_parent) | al->filtered,
                .hists  = hists,
                .branch_info = bi,
-               .mem_info = mi,
+               .mem_info = mem_info__get(mi),
                .kvm_info = ki,
                .block_info = block_info,
                .transaction = sample->transaction,
@@ -836,7 +828,7 @@ iter_prepare_mem_entry(struct hist_entry_iter *iter, struct addr_location *al)
        if (mi == NULL)
                return -ENOMEM;
 
-       iter->priv = mi;
+       iter->mi = mi;
        return 0;
 }
 
@@ -844,7 +836,7 @@ static int
 iter_add_single_mem_entry(struct hist_entry_iter *iter, struct addr_location *al)
 {
        u64 cost;
-       struct mem_info *mi = iter->priv;
+       struct mem_info *mi = iter->mi;
        struct hists *hists = evsel__hists(iter->evsel);
        struct perf_sample *sample = iter->sample;
        struct hist_entry *he;
@@ -891,12 +883,7 @@ iter_finish_mem_entry(struct hist_entry_iter *iter,
        err = hist_entry__append_callchain(he, iter->sample);
 
 out:
-       /*
-        * We don't need to free iter->priv (mem_info) here since the mem info
-        * was either already freed in hists__findnew_entry() or passed to a
-        * new hist entry by hist_entry__new().
-        */
-       iter->priv = NULL;
+       mem_info__zput(iter->mi);
 
        iter->he = NULL;
        return err;
@@ -915,7 +902,7 @@ iter_prepare_branch_entry(struct hist_entry_iter *iter, struct addr_location *al
        iter->curr = 0;
        iter->total = sample->branch_stack->nr;
 
-       iter->priv = bi;
+       iter->bi = bi;
        return 0;
 }
 
@@ -929,7 +916,7 @@ iter_add_single_branch_entry(struct hist_entry_iter *iter __maybe_unused,
 static int
 iter_next_branch_entry(struct hist_entry_iter *iter, struct addr_location *al)
 {
-       struct branch_info *bi = iter->priv;
+       struct branch_info *bi = iter->bi;
        int i = iter->curr;
 
        if (bi == NULL)
@@ -958,7 +945,7 @@ iter_add_next_branch_entry(struct hist_entry_iter *iter, struct addr_location *a
        int i = iter->curr;
        int err = 0;
 
-       bi = iter->priv;
+       bi = iter->bi;
 
        if (iter->hide_unresolved && !(bi[i].from.ms.sym && bi[i].to.ms.sym))
                goto out;
@@ -987,7 +974,7 @@ static int
 iter_finish_branch_entry(struct hist_entry_iter *iter,
                         struct addr_location *al __maybe_unused)
 {
-       zfree(&iter->priv);
+       zfree(&iter->bi);
        iter->he = NULL;
 
        return iter->curr >= iter->total ? 0 : -1;
@@ -1055,7 +1042,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter,
        if (he_cache == NULL)
                return -ENOMEM;
 
-       iter->priv = he_cache;
+       iter->he_cache = he_cache;
        iter->curr = 0;
 
        return 0;
@@ -1068,7 +1055,7 @@ iter_add_single_cumulative_entry(struct hist_entry_iter *iter,
        struct evsel *evsel = iter->evsel;
        struct hists *hists = evsel__hists(evsel);
        struct perf_sample *sample = iter->sample;
-       struct hist_entry **he_cache = iter->priv;
+       struct hist_entry **he_cache = iter->he_cache;
        struct hist_entry *he;
        int err = 0;
 
@@ -1126,7 +1113,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
 {
        struct evsel *evsel = iter->evsel;
        struct perf_sample *sample = iter->sample;
-       struct hist_entry **he_cache = iter->priv;
+       struct hist_entry **he_cache = iter->he_cache;
        struct hist_entry *he;
        struct hist_entry he_tmp = {
                .hists = evsel__hists(evsel),
@@ -1192,7 +1179,9 @@ static int
 iter_finish_cumulative_entry(struct hist_entry_iter *iter,
                             struct addr_location *al __maybe_unused)
 {
-       zfree(&iter->priv);
+       mem_info__zput(iter->mi);
+       zfree(&iter->bi);
+       zfree(&iter->he_cache);
        iter->he = NULL;
 
        return 0;

diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 5260822b9773e1bc688707cb6f721ebf2cc4862d..8fb3bdd2918814aefad06339dd23be9cbd30834a 100644 (file)
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -132,18 +132,20 @@ struct hist_entry_iter {
        int total;
        int curr;
 
-       bool hide_unresolved;
-
        struct evsel *evsel;
        struct perf_sample *sample;
        struct hist_entry *he;
        struct symbol *parent;
-       void *priv;
+
+       struct mem_info *mi;
+       struct branch_info *bi;
+       struct hist_entry **he_cache;
 
        const struct hist_iter_ops *ops;
        /* user-defined callback function (optional) */
        int (*add_entry_cb)(struct hist_entry_iter *iter,
                            struct addr_location *al, bool single, void *arg);
+       bool hide_unresolved;
 };
 
 extern const struct hist_iter_ops hist_iter_normal;