Re: [PATCH 13/16] perf: free all counts in perf_evsel__exit

[Arnaldo Carvalho de Melo]

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@xxxxxxxxx>
> ---
>  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