From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Changbin Du <changbin.du@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
namhyung@kernel.org, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Alexei Starovoitov <ast@kernel.org>,
rostedt@goodmis.org, Daniel Borkmann <daniel@iogearbox.net>,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH 13/16] perf: free all counts in perf_evsel__exit
Date: Mon, 18 Mar 2019 16:39:39 -0300 [thread overview]
Message-ID: <20190318193939.GC3967@kernel.org> (raw)
In-Reply-To: <20190316080556.3075-14-changbin.du@gmail.com>
Em Sat, Mar 16, 2019 at 04:05:53PM +0800, Changbin Du escreveu:
> Ensure that we have freed all allocated counts for struct perf_evsel.
>
> =================================================================
> ==7494==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
> #0 0x7f0333a89138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
> #1 0x5625e5330a5e in zalloc util/util.h:23
> #2 0x5625e5330a9b in perf_counts__new util/counts.c:10
> #3 0x5625e5330ca0 in perf_evsel__alloc_counts util/counts.c:47
> #4 0x5625e520d8e5 in __perf_evsel__read_on_cpu util/evsel.c:1505
Does this backtrace corresponds to this patch? I don't think so, this is
allocating evsel->counts, not evsel->prev_raw_counts.
The fix for this specific one is to call perf_evsel__free_counts() in
perf_evsel__exit(), ok? I'm adding a patch to that effect.
The case for evsel->prev_raw_counts is different, I think this started
in 'perf stat' and ended up moving from evsel->priv handled by 'perf
stat' to a new evsel field, prev_raw_counts, and that was being handled
by the stat code, that would call perf_evsel__free_prev_raw_counts()
before calling perf_evsel__delete(), I think. But I would have to double
check.
If you find some leak where evsel->prev_raw_counts leaks, or if you
determine that by looking at the code, please submit a patch for that
case, ok?
- Arnaldo
> #5 0x5625e517a985 in perf_evsel__read_on_cpu /home/work/linux/tools/perf/util/evsel.h:347
> #6 0x5625e517ad1a in test__openat_syscall_event tests/openat-syscall.c:47
> #7 0x5625e51528e6 in run_test tests/builtin-test.c:358
> #8 0x5625e5152baf in test_and_print tests/builtin-test.c:388
> #9 0x5625e51543fe in __cmd_test tests/builtin-test.c:583
> #10 0x5625e515572f in cmd_test tests/builtin-test.c:722
> #11 0x5625e51c3fb8 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
> #12 0x5625e51c44f7 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
> #13 0x5625e51c48fb in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
> #14 0x5625e51c5069 in main /home/changbin/work/linux/tools/perf/perf.c:520
> #15 0x7f033214d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
>
> Indirect leak of 72 byte(s) in 1 object(s) allocated from:
> #0 0x7f0333a89138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
> #1 0x5625e532560d in zalloc util/util.h:23
> #2 0x5625e532566b in xyarray__new util/xyarray.c:10
> #3 0x5625e5330aba in perf_counts__new util/counts.c:15
> #4 0x5625e5330ca0 in perf_evsel__alloc_counts util/counts.c:47
> #5 0x5625e520d8e5 in __perf_evsel__read_on_cpu util/evsel.c:1505
> #6 0x5625e517a985 in perf_evsel__read_on_cpu /home/work/linux/tools/perf/util/evsel.h:347
> #7 0x5625e517ad1a in test__openat_syscall_event tests/openat-syscall.c:47
> #8 0x5625e51528e6 in run_test tests/builtin-test.c:358
> #9 0x5625e5152baf in test_and_print tests/builtin-test.c:388
> #10 0x5625e51543fe in __cmd_test tests/builtin-test.c:583
> #11 0x5625e515572f in cmd_test tests/builtin-test.c:722
> #12 0x5625e51c3fb8 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
> #13 0x5625e51c44f7 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
> #14 0x5625e51c48fb in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
> #15 0x5625e51c5069 in main /home/changbin/work/linux/tools/perf/perf.c:520
> #16 0x7f033214d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
> tools/perf/util/counts.c | 18 ++++++++++++++++++
> tools/perf/util/counts.h | 4 ++++
> tools/perf/util/evsel.c | 2 ++
> tools/perf/util/stat.c | 18 ------------------
> 4 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/tools/perf/util/counts.c b/tools/perf/util/counts.c
> index 03032b410c29..c5715d08b1f9 100644
> --- a/tools/perf/util/counts.c
> +++ b/tools/perf/util/counts.c
> @@ -53,3 +53,21 @@ void perf_evsel__free_counts(struct perf_evsel *evsel)
> perf_counts__delete(evsel->counts);
> evsel->counts = NULL;
> }
> +
> +int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
> + int ncpus, int nthreads)
> +{
> + struct perf_counts *counts;
> +
> + counts = perf_counts__new(ncpus, nthreads);
> + if (counts)
> + evsel->prev_raw_counts = counts;
> +
> + return counts ? 0 : -ENOMEM;
> +}
> +
> +void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
> +{
> + perf_counts__delete(evsel->prev_raw_counts);
> + evsel->prev_raw_counts = NULL;
> +}
> diff --git a/tools/perf/util/counts.h b/tools/perf/util/counts.h
> index 0d1050ccc586..b8bdbff586bb 100644
> --- a/tools/perf/util/counts.h
> +++ b/tools/perf/util/counts.h
> @@ -36,4 +36,8 @@ void perf_evsel__reset_counts(struct perf_evsel *evsel);
> int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus, int nthreads);
> void perf_evsel__free_counts(struct perf_evsel *evsel);
>
> +int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
> + int ncpus, int nthreads);
> +void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel);
> +
> #endif /* __PERF_COUNTS_H */
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3bbf73e979c0..8e0fbe34e5d9 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1294,6 +1294,8 @@ void perf_evsel__exit(struct perf_evsel *evsel)
> assert(evsel->evlist == NULL);
> perf_evsel__free_fd(evsel);
> perf_evsel__free_id(evsel);
> + perf_evsel__free_counts(evsel);
> + perf_evsel__free_prev_raw_counts(evsel);
> perf_evsel__free_config_terms(evsel);
> cgroup__put(evsel->cgrp);
> cpu_map__put(evsel->cpus);
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 4d40515307b8..6a22842f76d2 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -136,24 +136,6 @@ static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
> zfree(&evsel->stats);
> }
>
> -static int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel,
> - int ncpus, int nthreads)
> -{
> - struct perf_counts *counts;
> -
> - counts = perf_counts__new(ncpus, nthreads);
> - if (counts)
> - evsel->prev_raw_counts = counts;
> -
> - return counts ? 0 : -ENOMEM;
> -}
> -
> -static void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
> -{
> - perf_counts__delete(evsel->prev_raw_counts);
> - evsel->prev_raw_counts = NULL;
> -}
> -
> static int perf_evsel__alloc_stats(struct perf_evsel *evsel, bool alloc_raw)
> {
> int ncpus = perf_evsel__nr_cpus(evsel);
> --
> 2.19.1
--
- Arnaldo
next prev parent reply other threads:[~2019-03-18 19:40 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-16 8:05 [PATCH 00/16] fix some perf issues detected by ASan Changbin Du
2019-03-16 8:05 ` [PATCH 01/16] perf: add doc for how to build perf with Asan and UBSan Changbin Du
2019-03-18 10:07 ` Jiri Olsa
2019-03-18 20:08 ` Arnaldo Carvalho de Melo
2019-03-20 12:00 ` Changbin Du
2019-03-20 11:58 ` Changbin Du
2019-03-16 8:05 ` [PATCH 02/16] perf: list: fix memory leak in function is_event_supported Changbin Du
2019-03-16 8:05 ` [PATCH 03/16] perf: fix errors under optimization level '-Og' Changbin Du
2019-03-16 8:05 ` [PATCH 04/16] perf: fix an error in config template Changbin Du
2019-03-16 8:05 ` [PATCH 05/16] perf: fix a memory leak in collect_config Changbin Du
2019-03-16 8:05 ` [PATCH 06/16] perf: fix memory leak in print_sdt_events() Changbin Du
2019-03-16 8:05 ` [PATCH 07/16] perf: top: fix heap-use-after-free issue Changbin Du
2019-03-18 10:08 ` Jiri Olsa
2019-03-16 8:05 ` [PATCH 08/16] perf: top: fix error handing in cmd_top() Changbin Du
2019-03-16 8:05 ` [PATCH 09/16] perf: missed a map__put() in error case Changbin Du
2019-03-16 8:05 ` [PATCH 10/16] perf: remove map from names tree in __maps__remove Changbin Du
2019-03-16 8:05 ` [PATCH 11/16] perf: purge all maps from the names tree Changbin Du
2019-03-16 8:05 ` [PATCH 12/16] perf: top: fix global-buffer-overflow issue Changbin Du
2019-03-16 8:05 ` [PATCH 13/16] perf: free all counts in perf_evsel__exit Changbin Du
2019-03-18 19:39 ` Arnaldo Carvalho de Melo [this message]
2019-03-16 8:05 ` [PATCH 14/16] perf: fix a memory leak of cpu_map object Changbin Du
2019-03-16 8:05 ` [PATCH 15/16] perf: fix memory leak by expr__find_other Changbin Du
2019-03-16 8:05 ` [PATCH 16/16] perf: fix a memory leak in test__perf_evsel__tp_sched_test Changbin Du
2019-03-18 10:08 ` [PATCH 00/16] fix some perf issues detected by ASan Jiri Olsa
2019-03-18 16:16 ` Arnaldo Carvalho de Melo
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=20190318193939.GC3967@kernel.org \
--to=arnaldo.melo@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=changbin.du@gmail.com \
--cc=daniel@iogearbox.net \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).