Commit d561e170 authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo
Browse files

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: default avatarIan 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: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 1a8c2e01

tools/perf/util/hist.c

+14 −25
Original line number Diff line number Diff line
@@ -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;

tools/perf/util/hist.h

+5 −3
Original line number Diff line number Diff line
@@ -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;