Re: [PATCH 08/25] perf stat: Introduce perf_counts__(alloc|free|reset) functions

Arnaldo Carvalho de Melo
2015-06-10T18:56:09Z

Em Wed, Jun 10, 2015 at 08:10:41PM +0200, Jiri Olsa escreveu:
> Move 'struct perf_counts' allocation|free|reset code into
> separate functions.
>
> Link: http://lkml.kernel.org/n/tip-qu64zmm5zbpbkuybusnkg4gl@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/builtin-stat.c | 19 +++++++------------
> tools/perf/util/evsel.c | 28 +++++++++++++++++++++++-----
> tools/perf/util/evsel.h | 3 +++
> 3 files changed, 33 insertions(+), 17 deletions(-)

But please do not move it to 'evsel.[ch]', i.e. you're introducing a new
class, i.e. "perf_counts", you might as well put them in a separate
file, i.e. tools/perf/util/counts.[ch].

And please rename perf_counts__alloc() to perf_counds__new(), ditto for
__free() -> __delete(),

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 79a596fcfd6c..90766538fd8f 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -168,24 +168,19 @@ static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
>
> static int perf_evsel__alloc_prev_raw_counts(struct perf_evsel *evsel)
> {
> - void *addr;
> - size_t sz;
> + struct perf_counts *counts;
>
> - sz = sizeof(*evsel->counts) +
> - (perf_evsel__nr_cpus(evsel) * sizeof(struct perf_counts_values));
> + counts = perf_counts__alloc(perf_evsel__nr_cpus(evsel));
> + if (counts)
> + evsel->prev_raw_counts = counts;
>
> - addr = zalloc(sz);
> - if (!addr)
> - return -ENOMEM;
> -
> - evsel->prev_raw_counts = addr;
> -
> - return 0;
> + return counts ? 0 : -ENOMEM;
> }
>
> static void perf_evsel__free_prev_raw_counts(struct perf_evsel *evsel)
> {
> - zfree(&evsel->prev_raw_counts);
> + perf_counts__free(evsel->prev_raw_counts);
> + evsel->prev_raw_counts = NULL;
> }
>
> static void perf_evlist__free_stats(struct perf_evlist *evlist)
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index bd1c3b71e455..fde2416921cc 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -851,16 +851,33 @@ int perf_evsel__alloc_id(struct perf_evsel *evsel, int ncpus, int nthreads)
> return 0;
> }
>
> +struct perf_counts *perf_counts__alloc(int ncpus)
> +{
> + int size = sizeof(struct perf_counts) +
> + ncpus * sizeof(struct perf_counts_values);
> +
> + return zalloc(size);
> +}
> +
> +void perf_counts__free(struct perf_counts *counts)
> +{
> + free(counts);
> +}
> +
> +static void perf_counts__reset(struct perf_counts *counts, int ncpus)
> +{
> + memset(counts, 0, (sizeof(*counts) +
> + (ncpus * sizeof(struct perf_counts_values))));
> +}
> +
> void perf_evsel__reset_counts(struct perf_evsel *evsel, int ncpus)
> {
> - memset(evsel->counts, 0, (sizeof(*evsel->counts) +
> - (ncpus * sizeof(struct perf_counts_values))));
> + perf_counts__reset(evsel->counts, ncpus);
> }
>
> int perf_evsel__alloc_counts(struct perf_evsel *evsel, int ncpus)
> {
> - evsel->counts = zalloc((sizeof(*evsel->counts) +
> - (ncpus * sizeof(struct perf_counts_values))));
> + evsel->counts = perf_counts__alloc(ncpus);
> return evsel->counts != NULL ? 0 : -ENOMEM;
> }
>
> @@ -893,7 +910,8 @@ void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
>
> void perf_evsel__free_counts(struct perf_evsel *evsel)
> {
> - zfree(&evsel->counts);
> + perf_counts__free(evsel->counts);
> + evsel->counts = NULL;
> }
>
> void perf_evsel__exit(struct perf_evsel *evsel)
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 54afdc80a651..ef619645a08f 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -131,6 +131,9 @@ void perf_counts_values__scale(struct perf_counts_values *count,
> void perf_evsel__compute_deltas(struct perf_evsel *evsel, int cpu,
> struct perf_counts_values *count);
>
> +struct perf_counts *perf_counts__alloc(int ncpus);
> +void perf_counts__free(struct perf_counts *counts);
> +
> int perf_evsel__object_config(size_t object_size,
> int (*init)(struct perf_evsel *evsel),
> void (*fini)(struct perf_evsel *evsel));
> --
> 1.9.3