[PATCHSET 0/9] perf tools: Fix various memory leaks

[PATCHSET 0/9] perf tools: Fix various memory leaks

[Namhyung Kim]

Hello,

I've found and fixed a bunch of memory leaks during perf pmu and
metric tests with address sanitizer.  Before this, the tests were
mostly failed due to the leaks since ASAN makes it return non-zero.

Now I'm seeing no error with ASAN like below:

  $ ./perf test pmu metric
   9: Parse perf pmu format                                 : Ok
  10: PMU events                                            :
  10.1: PMU event table sanity                              : Ok
  10.2: PMU event map aliases                               : Ok
  10.3: Parsing of PMU event table metrics                  : Skip (some metrics failed)
  10.4: Parsing of PMU event table metrics with fake PMUs   : Ok
  67: Parse and process metrics                             : Ok

The failure in 10.3 seems due to parse errors like below:

  Multiple errors dropping message: unknown term 'filter_opc' for pmu 'uncore_cbox_0'
  (valid terms: event,edge,inv,umask,cmask,config,config1,config2,name,period,freq,
                branch_type,time,call-graph,stack-size,no-inherit,inherit,max-stack,
		nr,no-overwrite,overwrite,driver-config,percore,aux-output,aux-sample-size)


  Parse event failed metric 'DRAM_Parallel_Reads' id 'arb/event=0x80,umask=0x2,thresh=1/'
    expr 'arb@event\=0x80\,umask\=0x2@ / arb@event\=0x80\,umask\=0x2\,thresh\=1@'
  Error string 'unknown term 'thresh' for pmu 'uncore_arb'' help
    'valid terms: event,edge,inv,umask,cmask,config,config1,config2,name,period,freq,
                  branch_type,time,call-graph,stack-size,no-inherit,inherit,max-stack,
		  nr,no-overwrite,overwrite,driver-config,percore,aux-output,aux-sample-size'


The patches are also available at 'perf/metric-fix-v1' branch on

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks
Namhyung


Namhyung Kim (9):
  perf evlist: Fix cpu/thread map leak
  perf parse-event: Fix cpu map leaks
  perf parse-event: Fix memory leak in evsel->unit
  perf test: Fix memory leaks in parse-metric test
  perf metric: Release expr_parse_ctx after testing
  perf metric: Free metric when it failed to resolve
  perf metric: Do not free metric when failed to resolve
  perf test: Free aliases for PMU event map aliases test
  perf test: Free formats for perf pmu parse test

 tools/perf/tests/parse-metric.c | 14 +++++++++-----
 tools/perf/tests/pmu-events.c   |  5 +++++
 tools/perf/tests/pmu.c          |  1 +
 tools/perf/util/evlist.c        | 11 ++++++++---
 tools/perf/util/metricgroup.c   | 26 ++++++++++++++++++--------
 tools/perf/util/parse-events.c  |  9 +++++++--
 tools/perf/util/pmu.c           | 13 ++++++++++++-
 tools/perf/util/pmu.h           |  2 ++
 tools/perf/util/stat-shadow.c   |  8 +++++---
 9 files changed, 67 insertions(+), 22 deletions(-)

-- 
2.28.0.526.ge36021eeef-goog

[PATCH 1/9] perf evlist: Fix cpu/thread map leak

[Namhyung Kim]

Asan reported leak of cpu and thread maps as they have one more
refcount than released.  I found that after setting evlist maps it
should release it's refcount.

It seems to be broken from the beginning so I chose the original
commit as the culprit.  But not sure how it's applied to stable trees
since there are many changes in the code after that.

Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread maps")
Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no thread/cpu maps set")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/evlist.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e3fa3bf7498a..c0768c61eb43 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, struct target *target)
 
 	perf_evlist__set_maps(&evlist->core, cpus, threads);
 
+	/* as evlist now has references, put count here */
+	perf_cpu_map__put(cpus);
+	perf_thread_map__put(threads);
+
 	return 0;
 
 out_delete_threads:
@@ -1273,11 +1277,12 @@ static int perf_evlist__create_syswide_maps(struct evlist *evlist)
 		goto out_put;
 
 	perf_evlist__set_maps(&evlist->core, cpus, threads);
-out:
-	return err;
+
+	perf_thread_map__put(threads);
 out_put:
 	perf_cpu_map__put(cpus);
-	goto out;
+out:
+	return err;
 }
 
 int evlist__open(struct evlist *evlist)
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 2/9] perf parse-event: Fix cpu map leaks

[Namhyung Kim]

Like evlist cpu map, evsel's cpu map should have proper refcount by
releasing the original count after creation.

This fixes the following ASAN report:

  Direct leak of 840 byte(s) in 70 object(s) allocated from:
    #0 0x7fe36703f628 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    #1 0x559fbbf611ca in cpu_map__trim_new /home/namhyung/project/linux/tools/lib/perf/cpumap.c:79
    #2 0x559fbbf6229c in perf_cpu_map__new /home/namhyung/project/linux/tools/lib/perf/cpumap.c:237
    #3 0x559fbbcc6c6d in __add_event util/parse-events.c:357
    #4 0x559fbbcc6c6d in add_event_tool util/parse-events.c:408
    #5 0x559fbbcc6c6d in parse_events_add_tool util/parse-events.c:1414
    #6 0x559fbbd8474d in parse_events_parse util/parse-events.y:439
    #7 0x559fbbcc95da in parse_events__scanner util/parse-events.c:2096
    #8 0x559fbbcc95da in __parse_events util/parse-events.c:2141
    #9 0x559fbbc2788b in check_parse_id tests/pmu-events.c:406
    #10 0x559fbbc2788b in check_parse_id tests/pmu-events.c:393
    #11 0x559fbbc2788b in check_parse_fake tests/pmu-events.c:436
    #12 0x559fbbc2788b in metric_parse_fake tests/pmu-events.c:553
    #13 0x559fbbc27e2d in test_parsing_fake tests/pmu-events.c:599
    #14 0x559fbbc27e2d in test_parsing_fake tests/pmu-events.c:574
    #15 0x559fbbc0109b in run_test tests/builtin-test.c:410
    #16 0x559fbbc0109b in test_and_print tests/builtin-test.c:440
    #17 0x559fbbc03e69 in __cmd_test tests/builtin-test.c:695
    #18 0x559fbbc03e69 in cmd_test tests/builtin-test.c:807
    #19 0x559fbbc691f4 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
    #20 0x559fbbb071a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
    #21 0x559fbbb071a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
    #22 0x559fbbb071a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
    #23 0x7fe366b68cc9 in __libc_start_main ../csu/libc-start.c:308

And I've failed which commit introduced this bug as the code was
heavily changed since then. ;-/

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/parse-events.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c4d2394e2b2d..b35e4bb1cecb 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -360,8 +360,11 @@ __add_event(struct list_head *list, int *idx,
 		event_attr_init(attr);
 
 	evsel = evsel__new_idx(attr, *idx);
-	if (!evsel)
+	if (!evsel) {
+		if (!pmu)
+			perf_cpu_map__put(cpus);
 		return NULL;
+	}
 
 	(*idx)++;
 	evsel->core.cpus   = perf_cpu_map__get(cpus);
@@ -369,6 +372,8 @@ __add_event(struct list_head *list, int *idx,
 	evsel->core.system_wide = pmu ? pmu->is_uncore : false;
 	evsel->auto_merge_stats = auto_merge_stats;
 
+	if (!pmu)
+		perf_cpu_map__put(cpus);
 	if (name)
 		evsel->name = strdup(name);
 
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 3/9] perf parse-event: Fix memory leak in evsel->unit

[Namhyung Kim]

The evsel->unit borrows a pointer of pmu event or alias instead of
owns a string.  But tool event (duration_time) passes a result of
strdup() caused a leak.

It was found by ASAN during metric test:

  Direct leak of 210 byte(s) in 70 object(s) allocated from:
    #0 0x7fe366fca0b5 in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x920b5)
    #1 0x559fbbcc6ea3 in add_event_tool util/parse-events.c:414
    #2 0x559fbbcc6ea3 in parse_events_add_tool util/parse-events.c:1414
    #3 0x559fbbd8474d in parse_events_parse util/parse-events.y:439
    #4 0x559fbbcc95da in parse_events__scanner util/parse-events.c:2096
    #5 0x559fbbcc95da in __parse_events util/parse-events.c:2141
    #6 0x559fbbc28555 in check_parse_id tests/pmu-events.c:406
    #7 0x559fbbc28555 in check_parse_id tests/pmu-events.c:393
    #8 0x559fbbc28555 in check_parse_cpu tests/pmu-events.c:415
    #9 0x559fbbc28555 in test_parsing tests/pmu-events.c:498
    #10 0x559fbbc0109b in run_test tests/builtin-test.c:410
    #11 0x559fbbc0109b in test_and_print tests/builtin-test.c:440
    #12 0x559fbbc03e69 in __cmd_test tests/builtin-test.c:695
    #13 0x559fbbc03e69 in cmd_test tests/builtin-test.c:807
    #14 0x559fbbc691f4 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
    #15 0x559fbbb071a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
    #16 0x559fbbb071a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
    #17 0x559fbbb071a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
    #18 0x7fe366b68cc9 in __libc_start_main ../csu/libc-start.c:308

Fixes: f0fbb114e3025 ("perf stat: Implement duration_time as a proper event")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/parse-events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index b35e4bb1cecb..ece321ccf599 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -416,7 +416,7 @@ static int add_event_tool(struct list_head *list, int *idx,
 		return -ENOMEM;
 	evsel->tool_event = tool_event;
 	if (tool_event == PERF_TOOL_DURATION_TIME)
-		evsel->unit = strdup("ns");
+		evsel->unit = "ns";
 	return 0;
 }
 
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 4/9] perf test: Fix memory leaks in parse-metric test

[Namhyung Kim]

It didn't release resources when there's an error so the
test_recursion_fail() will leak some memory.

Fixes: 0a507af9c681a ("perf tests: Add parse metric test for ipc metric")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/parse-metric.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index 23db8acc492d..cd7331aac3bd 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -153,8 +153,10 @@ static int __compute_metric(const char *name, struct value *vals,
 		return -ENOMEM;
 
 	cpus = perf_cpu_map__new("0");
-	if (!cpus)
+	if (!cpus) {
+		evlist__delete(evlist);
 		return -ENOMEM;
+	}
 
 	perf_evlist__set_maps(&evlist->core, cpus, NULL);
 
@@ -163,10 +165,11 @@ static int __compute_metric(const char *name, struct value *vals,
 					     false, false,
 					     &metric_events);
 	if (err)
-		return err;
+		goto out;
 
-	if (perf_evlist__alloc_stats(evlist, false))
-		return -1;
+	err = perf_evlist__alloc_stats(evlist, false);
+	if (err)
+		goto out;
 
 	/* Load the runtime stats with given numbers for events. */
 	runtime_stat__init(&st);
@@ -178,13 +181,14 @@ static int __compute_metric(const char *name, struct value *vals,
 	if (name2 && ratio2)
 		*ratio2 = compute_single(&metric_events, evlist, &st, name2);
 
+out:
 	/* ... clenup. */
 	metricgroup__rblist_exit(&metric_events);
 	runtime_stat__exit(&st);
 	perf_evlist__free_stats(evlist);
 	perf_cpu_map__put(cpus);
 	evlist__delete(evlist);
-	return 0;
+	return err;
 }
 
 static int compute_metric(const char *name, struct value *vals, double *ratio)
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 5/9] perf metric: Release expr_parse_ctx after testing

[Namhyung Kim]

The test_generic_metric() missed to release entries in the pctx.
Asan reported following leak (and more):

  Direct leak of 128 byte(s) in 1 object(s) allocated from:
    #0 0x7f4c9396980e in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10780e)
    #1 0x55f7e748cc14 in hashmap_grow (/home/namhyung/project/linux/tools/perf/perf+0x90cc14)
    #2 0x55f7e748d497 in hashmap__insert (/home/namhyung/project/linux/tools/perf/perf+0x90d497)
    #3 0x55f7e7341667 in hashmap__set /home/namhyung/project/linux/tools/perf/util/hashmap.h:111
    #4 0x55f7e7341667 in expr__add_ref util/expr.c:120
    #5 0x55f7e7292436 in prepare_metric util/stat-shadow.c:783
    #6 0x55f7e729556d in test_generic_metric util/stat-shadow.c:858
    #7 0x55f7e712390b in compute_single tests/parse-metric.c:128
    #8 0x55f7e712390b in __compute_metric tests/parse-metric.c:180
    #9 0x55f7e712446d in compute_metric tests/parse-metric.c:196
    #10 0x55f7e712446d in test_dcache_l2 tests/parse-metric.c:295
    #11 0x55f7e712446d in test__parse_metric tests/parse-metric.c:355
    #12 0x55f7e70be09b in run_test tests/builtin-test.c:410
    #13 0x55f7e70be09b in test_and_print tests/builtin-test.c:440
    #14 0x55f7e70c101a in __cmd_test tests/builtin-test.c:661
    #15 0x55f7e70c101a in cmd_test tests/builtin-test.c:807
    #16 0x55f7e7126214 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
    #17 0x55f7e6fc41a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
    #18 0x55f7e6fc41a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
    #19 0x55f7e6fc41a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
    #20 0x7f4c93492cc9 in __libc_start_main ../csu/libc-start.c:308

Fixes: 6d432c4c8aa56 ("perf tools: Add test_generic_metric function")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/stat-shadow.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index e1ba6c1b916a..a5f42c22c484 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -853,14 +853,16 @@ static void generic_metric(struct perf_stat_config *config,
 double test_generic_metric(struct metric_expr *mexp, int cpu, struct runtime_stat *st)
 {
 	struct expr_parse_ctx pctx;
-	double ratio;
+	double ratio = 0.0;
 
 	if (prepare_metric(mexp->metric_events, mexp->metric_refs, &pctx, cpu, st) < 0)
-		return 0.;
+		goto out;
 
 	if (expr__parse(&ratio, &pctx, mexp->metric_expr, 1))
-		return 0.;
+		ratio = 0.0;
 
+out:
+	expr__ctx_clear(&pctx);
 	return ratio;
 }
 
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 6/9] perf metric: Free metric when it failed to resolve

[Namhyung Kim]

The metricgroup__add_metric() can find multiple match for a metric
group and it's possible to fail.  Also it can fail in the middle like
in resolve_metric() even for single metric.

In those cases, the intermediate list and ids will be leaked like:

  Direct leak of 3 byte(s) in 1 object(s) allocated from:
    #0 0x7f4c938f40b5 in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x920b5)
    #1 0x55f7e71c1bef in __add_metric util/metricgroup.c:683
    #2 0x55f7e71c31d0 in add_metric util/metricgroup.c:906
    #3 0x55f7e71c3844 in metricgroup__add_metric util/metricgroup.c:940
    #4 0x55f7e71c488d in metricgroup__add_metric_list util/metricgroup.c:993
    #5 0x55f7e71c488d in parse_groups util/metricgroup.c:1045
    #6 0x55f7e71c60a4 in metricgroup__parse_groups_test util/metricgroup.c:1087
    #7 0x55f7e71235ae in __compute_metric tests/parse-metric.c:164
    #8 0x55f7e7124650 in compute_metric tests/parse-metric.c:196
    #9 0x55f7e7124650 in test_recursion_fail tests/parse-metric.c:318
    #10 0x55f7e7124650 in test__parse_metric tests/parse-metric.c:356
    #11 0x55f7e70be09b in run_test tests/builtin-test.c:410
    #12 0x55f7e70be09b in test_and_print tests/builtin-test.c:440
    #13 0x55f7e70c101a in __cmd_test tests/builtin-test.c:661
    #14 0x55f7e70c101a in cmd_test tests/builtin-test.c:807
    #15 0x55f7e7126214 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
    #16 0x55f7e6fc41a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
    #17 0x55f7e6fc41a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
    #18 0x55f7e6fc41a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
    #19 0x7f4c93492cc9 in __libc_start_main ../csu/libc-start.c:308

Fixes: 83de0b7d535de ("perf metric: Collect referenced metrics in struct metric_ref_node")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/metricgroup.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index b28c09447c10..c8904e471a71 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -939,7 +939,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
 
 		ret = add_metric(&list, pe, metric_no_group, &m, NULL, &ids);
 		if (ret)
-			return ret;
+			goto out;
 
 		/*
 		 * Process any possible referenced metrics
@@ -948,12 +948,14 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
 		ret = resolve_metric(metric_no_group,
 				     &list, map, &ids);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
 	/* End of pmu events. */
-	if (!has_match)
-		return -EINVAL;
+	if (!has_match) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	list_for_each_entry(m, &list, nd) {
 		if (events->len > 0)
@@ -968,9 +970,14 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
 		}
 	}
 
+out:
+	/*
+	 * add to metric_list so that they can be released
+	 * even if it's failed
+	 */
 	list_splice(&list, metric_list);
 	expr_ids__exit(&ids);
-	return 0;
+	return ret;
 }
 
 static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 7/9] perf metric: Do not free metric when failed to resolve

[Namhyung Kim]

It's dangerous to free the original metric when it's called from
resolve_metric() as it's already in the metric_list and might have
other resources too.  Instead, it'd better let them bail out and be
released properly at the later stage.

So add a check when it's called from metricgroup__add_metric() and
release it.  Also make sure that mp is set properly.

Fixes: 83de0b7d535de ("perf metric: Collect referenced metrics in struct metric_ref_node")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/metricgroup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index c8904e471a71..ab5030fcfed4 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -672,7 +672,6 @@ static int __add_metric(struct list_head *metric_list,
 		m->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
 		INIT_LIST_HEAD(&m->metric_refs);
 		m->metric_refs_cnt = 0;
-		*mp = m;
 
 		parent = expr_ids__alloc(ids);
 		if (!parent) {
@@ -685,6 +684,7 @@ static int __add_metric(struct list_head *metric_list,
 			free(m);
 			return -ENOMEM;
 		}
+		*mp = m;
 	} else {
 		/*
 		 * We got here for the referenced metric, via the
@@ -719,8 +719,11 @@ static int __add_metric(struct list_head *metric_list,
 	 * all the metric's IDs and add it to the parent context.
 	 */
 	if (expr__find_other(pe->metric_expr, NULL, &m->pctx, runtime) < 0) {
-		expr__ctx_clear(&m->pctx);
-		free(m);
+		if (m->metric_refs_cnt == 0) {
+			expr__ctx_clear(&m->pctx);
+			free(m);
+			*mp = NULL;
+		}
 		return -EINVAL;
 	}
 
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 8/9] perf test: Free aliases for PMU event map aliases test

[Namhyung Kim]

The aliases were never released causing the following leaks:

  Indirect leak of 1224 byte(s) in 9 object(s) allocated from:
    #0 0x7feefb830628 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    #1 0x56332c8f1b62 in __perf_pmu__new_alias util/pmu.c:322
    #2 0x56332c8f401f in pmu_add_cpu_aliases_map util/pmu.c:778
    #3 0x56332c792ce9 in __test__pmu_event_aliases tests/pmu-events.c:295
    #4 0x56332c792ce9 in test_aliases tests/pmu-events.c:367
    #5 0x56332c76a09b in run_test tests/builtin-test.c:410
    #6 0x56332c76a09b in test_and_print tests/builtin-test.c:440
    #7 0x56332c76ce69 in __cmd_test tests/builtin-test.c:695
    #8 0x56332c76ce69 in cmd_test tests/builtin-test.c:807
    #9 0x56332c7d2214 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
    #10 0x56332c6701a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
    #11 0x56332c6701a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
    #12 0x56332c6701a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
    #13 0x7feefb359cc9 in __libc_start_main ../csu/libc-start.c:308

Cc: John Garry <john.garry@huawei.com>
Fixes: 956a78356c24c ("perf test: Test pmu-events aliases")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/pmu-events.c | 5 +++++
 tools/perf/util/pmu.c         | 2 +-
 tools/perf/util/pmu.h         | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index eb19f9a0bc15..d3517a74d95e 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -274,6 +274,7 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
 	int res = 0;
 	bool use_uncore_table;
 	struct pmu_events_map *map = __test_pmu_get_events_map();
+	struct perf_pmu_alias *a, *tmp;
 
 	if (!map)
 		return -1;
@@ -347,6 +348,10 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
 			  pmu_name, alias->name);
 	}
 
+	list_for_each_entry_safe(a, tmp, &aliases, list) {
+		list_del(&a->list);
+		perf_pmu_free_alias(a);
+	}
 	free(pmu);
 	return res;
 }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f1688e1f6ed7..555cb3524c25 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -274,7 +274,7 @@ static void perf_pmu_update_alias(struct perf_pmu_alias *old,
 }
 
 /* Delete an alias entry. */
-static void perf_pmu_free_alias(struct perf_pmu_alias *newalias)
+void perf_pmu_free_alias(struct perf_pmu_alias *newalias)
 {
 	zfree(&newalias->name);
 	zfree(&newalias->desc);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 44ccbdbb1c37..b63c4c5e335e 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -113,6 +113,7 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
 
 struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
+void perf_pmu_free_alias(struct perf_pmu_alias *alias);
 
 int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
 
-- 
2.28.0.526.ge36021eeef-goog

[PATCH 9/9] perf test: Free formats for perf pmu parse test

[Namhyung Kim]

The following leaks were detected by ASAN:

  Indirect leak of 360 byte(s) in 9 object(s) allocated from:
    #0 0x7fecc305180e in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10780e)
    #1 0x560578f6dce5 in perf_pmu__new_format util/pmu.c:1333
    #2 0x560578f752fc in perf_pmu_parse util/pmu.y:59
    #3 0x560578f6a8b7 in perf_pmu__format_parse util/pmu.c:73
    #4 0x560578e07045 in test__pmu tests/pmu.c:155
    #5 0x560578de109b in run_test tests/builtin-test.c:410
    #6 0x560578de109b in test_and_print tests/builtin-test.c:440
    #7 0x560578de401a in __cmd_test tests/builtin-test.c:661
    #8 0x560578de401a in cmd_test tests/builtin-test.c:807
    #9 0x560578e49354 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
    #10 0x560578ce71a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
    #11 0x560578ce71a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
    #12 0x560578ce71a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
    #13 0x7fecc2b7acc9 in __libc_start_main ../csu/libc-start.c:308

Fixes: cff7f956ec4a1 ("perf tests: Move pmu tests into separate object")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/pmu.c |  1 +
 tools/perf/util/pmu.c  | 11 +++++++++++
 tools/perf/util/pmu.h  |  1 +
 3 files changed, 13 insertions(+)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 5c11fe2b3040..714e6830a758 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -173,6 +173,7 @@ int test__pmu(struct test *test __maybe_unused, int subtest __maybe_unused)
 		ret = 0;
 	} while (0);
 
+	perf_pmu__del_formats(&formats);
 	test_format_dir_put(format);
 	return ret;
 }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 555cb3524c25..d41caeb35cf6 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1354,6 +1354,17 @@ void perf_pmu__set_format(unsigned long *bits, long from, long to)
 		set_bit(b, bits);
 }
 
+void perf_pmu__del_formats(struct list_head *formats)
+{
+	struct perf_pmu_format *fmt, *tmp;
+
+	list_for_each_entry_safe(fmt, tmp, formats, list) {
+		list_del(&fmt->list);
+		free(fmt->name);
+		free(fmt);
+	}
+}
+
 static int sub_non_neg(int a, int b)
 {
 	if (b > a)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index b63c4c5e335e..a64e9c9ce731 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -94,6 +94,7 @@ int perf_pmu__new_format(struct list_head *list, char *name,
 			 int config, unsigned long *bits);
 void perf_pmu__set_format(unsigned long *bits, long from, long to);
 int perf_pmu__format_parse(char *dir, struct list_head *head);
+void perf_pmu__del_formats(struct list_head *formats);
 
 struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);
 
-- 
2.28.0.526.ge36021eeef-goog

Re: [PATCH 8/9] perf test: Free aliases for PMU event map aliases test

[John Garry]

On 07/09/2020 04:45, Namhyung Kim wrote:
> The aliases were never released causing the following leaks:
> 
>    Indirect leak of 1224 byte(s) in 9 object(s) allocated from:
>      #0 0x7feefb830628 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
>      #1 0x56332c8f1b62 in __perf_pmu__new_alias util/pmu.c:322
>      #2 0x56332c8f401f in pmu_add_cpu_aliases_map util/pmu.c:778
>      #3 0x56332c792ce9 in __test__pmu_event_aliases tests/pmu-events.c:295
>      #4 0x56332c792ce9 in test_aliases tests/pmu-events.c:367
>      #5 0x56332c76a09b in run_test tests/builtin-test.c:410
>      #6 0x56332c76a09b in test_and_print tests/builtin-test.c:440
>      #7 0x56332c76ce69 in __cmd_test tests/builtin-test.c:695
>      #8 0x56332c76ce69 in cmd_test tests/builtin-test.c:807
>      #9 0x56332c7d2214 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
>      #10 0x56332c6701a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
>      #11 0x56332c6701a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
>      #12 0x56332c6701a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
>      #13 0x7feefb359cc9 in __libc_start_main ../csu/libc-start.c:308
> 
> Cc: John Garry <john.garry@huawei.com>
> Fixes: 956a78356c24c ("perf test: Test pmu-events aliases")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>   tools/perf/tests/pmu-events.c | 5 +++++
>   tools/perf/util/pmu.c         | 2 +-
>   tools/perf/util/pmu.h         | 1 +
>   3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index eb19f9a0bc15..d3517a74d95e 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -274,6 +274,7 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
>   	int res = 0;
>   	bool use_uncore_table;
>   	struct pmu_events_map *map = __test_pmu_get_events_map();
> +	struct perf_pmu_alias *a, *tmp;
>   
>   	if (!map)
>   		return -1;
> @@ -347,6 +348,10 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
>   			  pmu_name, alias->name);
>   	}
>   
> +	list_for_each_entry_safe(a, tmp, &aliases, list) {
> +		list_del(&a->list);
> +		perf_pmu_free_alias(a);

This looks ok.

I also notice that we have other paths like this, where the allocated 
pmu (and aliases) are not freed for later error paths, it seems:

parse_events_add_pmu() -> perf_pmu_find() -> pmu_lookup() -> 
pmu_add_cpu_aliases().

I had a quick look at the rest of the series, and could not see if we 
fix up any of this.

Cheers,
john

> +	}
>   	free(pmu);
>   	return res;
>   }
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index f1688e1f6ed7..555cb3524c25 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -274,7 +274,7 @@ static void perf_pmu_update_alias(struct perf_pmu_alias *old,
>   }
>   
>   /* Delete an alias entry. */
> -static void perf_pmu_free_alias(struct perf_pmu_alias *newalias)
> +void perf_pmu_free_alias(struct perf_pmu_alias *newalias)
>   {
>   	zfree(&newalias->name);
>   	zfree(&newalias->desc);
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 44ccbdbb1c37..b63c4c5e335e 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -113,6 +113,7 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
>   
>   struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
>   bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
> +void perf_pmu_free_alias(struct perf_pmu_alias *alias);
>   
>   int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
>   
> 

Re: [PATCH 1/9] perf evlist: Fix cpu/thread map leak

[Jiri Olsa]

On Mon, Sep 07, 2020 at 12:44:54PM +0900, Namhyung Kim wrote:
> Asan reported leak of cpu and thread maps as they have one more
> refcount than released.  I found that after setting evlist maps it
> should release it's refcount.
> 
> It seems to be broken from the beginning so I chose the original
> commit as the culprit.  But not sure how it's applied to stable trees
> since there are many changes in the code after that.
> 
> Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread maps")
> Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no thread/cpu maps set")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/evlist.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e3fa3bf7498a..c0768c61eb43 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, struct target *target)
>  
>  	perf_evlist__set_maps(&evlist->core, cpus, threads);
>  
> +	/* as evlist now has references, put count here */
> +	perf_cpu_map__put(cpus);
> +	perf_thread_map__put(threads);

nice catch, I checked perf_evlist__create_syswide_maps is doing this
correctly, but I think we might have the same issue in script's
set_maps function

thanks,
jirka

> +
>  	return 0;
>  
>  out_delete_threads:
> @@ -1273,11 +1277,12 @@ static int perf_evlist__create_syswide_maps(struct evlist *evlist)
>  		goto out_put;
>  
>  	perf_evlist__set_maps(&evlist->core, cpus, threads);
> -out:
> -	return err;
> +
> +	perf_thread_map__put(threads);
>  out_put:
>  	perf_cpu_map__put(cpus);
> -	goto out;
> +out:
> +	return err;
>  }
>  
>  int evlist__open(struct evlist *evlist)
> -- 
> 2.28.0.526.ge36021eeef-goog
> 

Re: [PATCH 1/9] perf evlist: Fix cpu/thread map leak

[Jiri Olsa]

On Mon, Sep 07, 2020 at 01:29:29PM +0200, Jiri Olsa wrote:
> On Mon, Sep 07, 2020 at 12:44:54PM +0900, Namhyung Kim wrote:
> > Asan reported leak of cpu and thread maps as they have one more
> > refcount than released.  I found that after setting evlist maps it
> > should release it's refcount.
> > 
> > It seems to be broken from the beginning so I chose the original
> > commit as the culprit.  But not sure how it's applied to stable trees
> > since there are many changes in the code after that.
> > 
> > Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread maps")
> > Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no thread/cpu maps set")
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/evlist.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index e3fa3bf7498a..c0768c61eb43 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, struct target *target)
> >  
> >  	perf_evlist__set_maps(&evlist->core, cpus, threads);
> >  
> > +	/* as evlist now has references, put count here */
> > +	perf_cpu_map__put(cpus);
> > +	perf_thread_map__put(threads);
> 
> nice catch, I checked perf_evlist__create_syswide_maps is doing this

because you fixed that :))) missed the rest of the patch.. sry

jirka

> correctly, but I think we might have the same issue in script's
> set_maps function
> 
> thanks,
> jirka
> 
> > +
> >  	return 0;
> >  
> >  out_delete_threads:
> > @@ -1273,11 +1277,12 @@ static int perf_evlist__create_syswide_maps(struct evlist *evlist)
> >  		goto out_put;
> >  
> >  	perf_evlist__set_maps(&evlist->core, cpus, threads);
> > -out:
> > -	return err;
> > +
> > +	perf_thread_map__put(threads);
> >  out_put:
> >  	perf_cpu_map__put(cpus);
> > -	goto out;
> > +out:
> > +	return err;
> >  }
> >  
> >  int evlist__open(struct evlist *evlist)
> > -- 
> > 2.28.0.526.ge36021eeef-goog
> > 

Re: [PATCHSET 0/9] perf tools: Fix various memory leaks

[Jiri Olsa]

On Mon, Sep 07, 2020 at 12:44:53PM +0900, Namhyung Kim wrote:
> Hello,
> 
> I've found and fixed a bunch of memory leaks during perf pmu and
> metric tests with address sanitizer.  Before this, the tests were
> mostly failed due to the leaks since ASAN makes it return non-zero.
> 
> Now I'm seeing no error with ASAN like below:
> 
>   $ ./perf test pmu metric
>    9: Parse perf pmu format                                 : Ok
>   10: PMU events                                            :
>   10.1: PMU event table sanity                              : Ok
>   10.2: PMU event map aliases                               : Ok
>   10.3: Parsing of PMU event table metrics                  : Skip (some metrics failed)
>   10.4: Parsing of PMU event table metrics with fake PMUs   : Ok
>   67: Parse and process metrics                             : Ok
> 
> The failure in 10.3 seems due to parse errors like below:
> 
>   Multiple errors dropping message: unknown term 'filter_opc' for pmu 'uncore_cbox_0'
>   (valid terms: event,edge,inv,umask,cmask,config,config1,config2,name,period,freq,
>                 branch_type,time,call-graph,stack-size,no-inherit,inherit,max-stack,
> 		nr,no-overwrite,overwrite,driver-config,percore,aux-output,aux-sample-size)
> 
> 
>   Parse event failed metric 'DRAM_Parallel_Reads' id 'arb/event=0x80,umask=0x2,thresh=1/'
>     expr 'arb@event\=0x80\,umask\=0x2@ / arb@event\=0x80\,umask\=0x2\,thresh\=1@'
>   Error string 'unknown term 'thresh' for pmu 'uncore_arb'' help
>     'valid terms: event,edge,inv,umask,cmask,config,config1,config2,name,period,freq,
>                   branch_type,time,call-graph,stack-size,no-inherit,inherit,max-stack,
> 		  nr,no-overwrite,overwrite,driver-config,percore,aux-output,aux-sample-size'
> 
> 
> The patches are also available at 'perf/metric-fix-v1' branch on
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks
> Namhyung
> 
> 
> Namhyung Kim (9):
>   perf evlist: Fix cpu/thread map leak
>   perf parse-event: Fix cpu map leaks
>   perf parse-event: Fix memory leak in evsel->unit
>   perf test: Fix memory leaks in parse-metric test
>   perf metric: Release expr_parse_ctx after testing
>   perf metric: Free metric when it failed to resolve
>   perf metric: Do not free metric when failed to resolve
>   perf test: Free aliases for PMU event map aliases test
>   perf test: Free formats for perf pmu parse test

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

Re: [PATCH 8/9] perf test: Free aliases for PMU event map aliases test

[Namhyung Kim]

Hello,

On Mon, Sep 7, 2020 at 7:31 PM John Garry <john.garry@huawei.com> wrote:
>
> On 07/09/2020 04:45, Namhyung Kim wrote:
> > The aliases were never released causing the following leaks:
> >
> >    Indirect leak of 1224 byte(s) in 9 object(s) allocated from:
> >      #0 0x7feefb830628 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
> >      #1 0x56332c8f1b62 in __perf_pmu__new_alias util/pmu.c:322
> >      #2 0x56332c8f401f in pmu_add_cpu_aliases_map util/pmu.c:778
> >      #3 0x56332c792ce9 in __test__pmu_event_aliases tests/pmu-events.c:295
> >      #4 0x56332c792ce9 in test_aliases tests/pmu-events.c:367
> >      #5 0x56332c76a09b in run_test tests/builtin-test.c:410
> >      #6 0x56332c76a09b in test_and_print tests/builtin-test.c:440
> >      #7 0x56332c76ce69 in __cmd_test tests/builtin-test.c:695
> >      #8 0x56332c76ce69 in cmd_test tests/builtin-test.c:807
> >      #9 0x56332c7d2214 in run_builtin /home/namhyung/project/linux/tools/perf/perf.c:312
> >      #10 0x56332c6701a8 in handle_internal_command /home/namhyung/project/linux/tools/perf/perf.c:364
> >      #11 0x56332c6701a8 in run_argv /home/namhyung/project/linux/tools/perf/perf.c:408
> >      #12 0x56332c6701a8 in main /home/namhyung/project/linux/tools/perf/perf.c:538
> >      #13 0x7feefb359cc9 in __libc_start_main ../csu/libc-start.c:308
> >
> > Cc: John Garry <john.garry@huawei.com>
> > Fixes: 956a78356c24c ("perf test: Test pmu-events aliases")
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >   tools/perf/tests/pmu-events.c | 5 +++++
> >   tools/perf/util/pmu.c         | 2 +-
> >   tools/perf/util/pmu.h         | 1 +
> >   3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> > index eb19f9a0bc15..d3517a74d95e 100644
> > --- a/tools/perf/tests/pmu-events.c
> > +++ b/tools/perf/tests/pmu-events.c
> > @@ -274,6 +274,7 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
> >       int res = 0;
> >       bool use_uncore_table;
> >       struct pmu_events_map *map = __test_pmu_get_events_map();
> > +     struct perf_pmu_alias *a, *tmp;
> >
> >       if (!map)
> >               return -1;
> > @@ -347,6 +348,10 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count)
> >                         pmu_name, alias->name);
> >       }
> >
> > +     list_for_each_entry_safe(a, tmp, &aliases, list) {
> > +             list_del(&a->list);
> > +             perf_pmu_free_alias(a);
>
> This looks ok.

Thanks!

>
> I also notice that we have other paths like this, where the allocated
> pmu (and aliases) are not freed for later error paths, it seems:
>
> parse_events_add_pmu() -> perf_pmu_find() -> pmu_lookup() ->
> pmu_add_cpu_aliases().
>
> I had a quick look at the rest of the series, and could not see if we
> fix up any of this.

Right, I also found that and wondered why ASAN didn't report it.
I think we should free all pmu instances from the pmus list.
But this can be a later work..

Thanks
Namhyung

Re: [PATCH 1/9] perf evlist: Fix cpu/thread map leak

[Namhyung Kim]

Hi Jiri,

On Mon, Sep 7, 2020 at 8:32 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Sep 07, 2020 at 01:29:29PM +0200, Jiri Olsa wrote:
> > On Mon, Sep 07, 2020 at 12:44:54PM +0900, Namhyung Kim wrote:
> > > Asan reported leak of cpu and thread maps as they have one more
> > > refcount than released.  I found that after setting evlist maps it
> > > should release it's refcount.
> > >
> > > It seems to be broken from the beginning so I chose the original
> > > commit as the culprit.  But not sure how it's applied to stable trees
> > > since there are many changes in the code after that.
> > >
> > > Fixes: 7e2ed097538c5 ("perf evlist: Store pointer to the cpu and thread maps")
> > > Fixes: 4112eb1899c0e ("perf evlist: Default to syswide target when no thread/cpu maps set")
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/util/evlist.c | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > > index e3fa3bf7498a..c0768c61eb43 100644
> > > --- a/tools/perf/util/evlist.c
> > > +++ b/tools/perf/util/evlist.c
> > > @@ -946,6 +946,10 @@ int perf_evlist__create_maps(struct evlist *evlist, struct target *target)
> > >
> > >     perf_evlist__set_maps(&evlist->core, cpus, threads);
> > >
> > > +   /* as evlist now has references, put count here */
> > > +   perf_cpu_map__put(cpus);
> > > +   perf_thread_map__put(threads);
> >
> > nice catch, I checked perf_evlist__create_syswide_maps is doing this
>
> because you fixed that :))) missed the rest of the patch.. sry

No worries and thanks for the review! :)

Namhyung

Re: [PATCH 8/9] perf test: Free aliases for PMU event map aliases test

[John Garry]

On 07/09/2020 14:20, Namhyung Kim wrote:
>> I also notice that we have other paths like this, where the allocated
>> pmu (and aliases) are not freed for later error paths, it seems:
>>
>> parse_events_add_pmu() -> perf_pmu_find() -> pmu_lookup() ->
>> pmu_add_cpu_aliases().
>>
>> I had a quick look at the rest of the series, and could not see if we
>> fix up any of this.

Hi Namhyung,

> Right, I also found that and wondered why ASAN didn't report it.
> I think we should free all pmu instances from the pmus list.
> But this can be a later work..

ok, good. I was going to say that we could at least add a pmu free 
helper in pmu.c now (and reference it here, from this test code, for 
now), but maybe you just want to fix the reported leaks for now and work 
on this as a follow up.

Thanks,
John