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

[Arnaldo Carvalho de Melo]

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@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/