[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240507183545.1236093-9-irogers@google.com>
Date: Tue, 7 May 2024 11:35:45 -0700
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, Oliver Upton <oliver.upton@...ux.dev>,
James Clark <james.clark@....com>, Tim Chen <tim.c.chen@...ux.intel.com>,
Yicong Yang <yangyicong@...ilicon.com>, K Prateek Nayak <kprateek.nayak@....com>,
Yanteng Si <siyanteng@...ngson.cn>, Sun Haiyong <sunhaiyong@...ngson.cn>,
Kajol Jain <kjain@...ux.ibm.com>, Ravi Bangoria <ravi.bangoria@....com>, Li Dong <lidong@...o.com>,
Paran Lee <p4ranlee@...il.com>, Ben Gainey <ben.gainey@....com>, Andi Kleen <ak@...ux.intel.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: [PATCH v1 8/8] perf hist: Avoid hist_entry_iter mem_info memory leak
struct mem_info is reference counted while branch_info and he_cache
are not. Break apart the priv field in 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@...gle.com>
---
tools/perf/util/hist.c | 39 ++++++++++++++-------------------------
tools/perf/util/hist.h | 8 +++++---
2 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 00814d42d5f1..2e9e193179dd 100644
--- 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 5260822b9773..8fb3bdd29188 100644
--- 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;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
Powered by blists - more mailing lists