From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>, Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Stephane Eranian <eranian@google.com>,
LKML <linux-kernel@vger.kernel.org>,
Andi Kleen <ak@linux.intel.com>, Ian Rogers <irogers@google.com>
Subject: Re: [PATCH v2 2/2] perf parse-event: Fix cpu map leaks
Date: Thu, 17 Sep 2020 13:27:48 -0300 [thread overview]
Message-ID: <20200917162748.GC1322686@kernel.org> (raw)
In-Reply-To: <20200917060219.1287863-2-namhyung@kernel.org>
Em Thu, Sep 17, 2020 at 03:02:19PM +0900, Namhyung Kim escreveu:
> Like evlist cpu map, evsel's cpu map should have a proper refcount.
> As it's created with a refcount, we don't need to get an extra count.
> Thanks to Arnaldo for the simpler suggestion.
>
> This fixes the following ASAN report:
This one should come first, and the patch summary is:
"perf parse-events Fix cpu map refcounting"
Then the next patch fixes the refcoutnt leak on the error path.
I'm fixing this up,
Thanks,
- Arnaldo
> 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. ;-/
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/parse-events.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 176a51698a64..fbe0d3143353 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -353,7 +353,7 @@ __add_event(struct list_head *list, int *idx,
> const char *cpu_list)
> {
> struct evsel *evsel;
> - struct perf_cpu_map *cpus = pmu ? pmu->cpus :
> + struct perf_cpu_map *cpus = pmu ? perf_cpu_map__get(pmu->cpus) :
> cpu_list ? perf_cpu_map__new(cpu_list) : NULL;
>
> if (init_attr)
> @@ -366,7 +366,7 @@ __add_event(struct list_head *list, int *idx,
> }
>
> (*idx)++;
> - evsel->core.cpus = perf_cpu_map__get(cpus);
> + evsel->core.cpus = cpus;
> evsel->core.own_cpus = perf_cpu_map__get(cpus);
> evsel->core.system_wide = pmu ? pmu->is_uncore : false;
> evsel->auto_merge_stats = auto_merge_stats;
> --
> 2.28.0.618.gf4bc123cb7-goog
>
--
- Arnaldo
next prev parent reply other threads:[~2020-09-17 16:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-17 6:02 [PATCH v2 1/2] perf parse-event: Release cpu_map if evsel alloc failed Namhyung Kim
2020-09-17 6:02 ` [PATCH v2 2/2] perf parse-event: Fix cpu map leaks Namhyung Kim
2020-09-17 16:27 ` Arnaldo Carvalho de Melo [this message]
2020-09-17 13:10 ` [PATCH v2 1/2] perf parse-event: Release cpu_map if evsel alloc failed Jiri Olsa
2020-09-17 13:27 ` Namhyung Kim
2020-09-17 16:30 ` Arnaldo Carvalho de Melo
2020-09-18 1:51 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200917162748.GC1322686@kernel.org \
--to=acme@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.