Re: [PATCH 03/25] perf tools: Add reference counting for cpu_map object

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Jun 10, 2015 at 08:10:36PM +0200, Jiri Olsa escreveu:
> Adding refference counting for cpu_map object, so
> it could be easily shared among other objects.
> 
> Using cpu_map__put instead cpu_map__delete and making
> cpu_map__delete static.

Please use atomic.h, as was done recently for the 'map', 'dso' and
'thread' structs, but other than that, its fine.

- Arnaldo
 
> Link: http://lkml.kernel.org/n/tip-gwy96ohuwhjz603bnf70af6i@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/tests/code-reading.c    |  2 +-
>  tools/perf/tests/keep-tracking.c   |  2 +-
>  tools/perf/tests/mmap-basic.c      |  2 +-
>  tools/perf/tests/switch-tracking.c |  2 +-
>  tools/perf/util/cpumap.c           | 24 ++++++++++++++++++++++--
>  tools/perf/util/cpumap.h           |  5 ++++-
>  tools/perf/util/evlist.c           |  4 ++--
>  tools/perf/util/evsel.c            |  1 +
>  tools/perf/util/parse-events.c     |  5 ++++-
>  tools/perf/util/python.c           |  2 +-
>  tools/perf/util/record.c           |  4 ++--
>  tools/perf/util/session.c          |  2 +-
>  tools/perf/util/svghelper.c        |  2 +-
>  13 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index e2a432b67d52..86d17d2e9e50 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -545,7 +545,7 @@ out_err:
>  	if (evlist) {
>  		perf_evlist__delete(evlist);
>  	} else {
> -		cpu_map__delete(cpus);
> +		cpu_map__put(cpus);
>  		thread_map__delete(threads);
>  	}
>  	machines__destroy_kernel_maps(&machines);
> diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
> index 5b171d1e338b..a330235cefc0 100644
> --- a/tools/perf/tests/keep-tracking.c
> +++ b/tools/perf/tests/keep-tracking.c
> @@ -144,7 +144,7 @@ out_err:
>  		perf_evlist__disable(evlist);
>  		perf_evlist__delete(evlist);
>  	} else {
> -		cpu_map__delete(cpus);
> +		cpu_map__put(cpus);
>  		thread_map__delete(threads);
>  	}
>  
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index 5855cf471210..5a9ef5833452 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -140,7 +140,7 @@ out_delete_evlist:
>  	cpus	= NULL;
>  	threads = NULL;
>  out_free_cpus:
> -	cpu_map__delete(cpus);
> +	cpu_map__put(cpus);
>  out_free_threads:
>  	thread_map__delete(threads);
>  	return err;
> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
> index 0d31403ea593..1b06122beb76 100644
> --- a/tools/perf/tests/switch-tracking.c
> +++ b/tools/perf/tests/switch-tracking.c
> @@ -560,7 +560,7 @@ out:
>  		perf_evlist__disable(evlist);
>  		perf_evlist__delete(evlist);
>  	} else {
> -		cpu_map__delete(cpus);
> +		cpu_map__put(cpus);
>  		thread_map__delete(threads);
>  	}
>  
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index c4e55b71010c..60f4297ed233 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -5,6 +5,7 @@
>  #include <assert.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include "asm/bug.h"
>  
>  static struct cpu_map *cpu_map__default_new(void)
>  {
> @@ -22,6 +23,7 @@ static struct cpu_map *cpu_map__default_new(void)
>  			cpus->map[i] = i;
>  
>  		cpus->nr = nr_cpus;
> +		cpus->refcnt = 1;
>  	}
>  
>  	return cpus;
> @@ -35,6 +37,7 @@ static struct cpu_map *cpu_map__trim_new(int nr_cpus, int *tmp_cpus)
>  	if (cpus != NULL) {
>  		cpus->nr = nr_cpus;
>  		memcpy(cpus->map, tmp_cpus, payload_size);
> +		cpus->refcnt = 1;
>  	}
>  
>  	return cpus;
> @@ -194,14 +197,30 @@ struct cpu_map *cpu_map__dummy_new(void)
>  	if (cpus != NULL) {
>  		cpus->nr = 1;
>  		cpus->map[0] = -1;
> +		cpus->refcnt = 1;
>  	}
>  
>  	return cpus;
>  }
>  
> -void cpu_map__delete(struct cpu_map *map)
> +static void cpu_map__delete(struct cpu_map *map)
>  {
> -	free(map);
> +	if (map) {
> +		WARN_ONCE(map->refcnt != 0, "cpu map refcnt disbalanced\n");
> +		free(map);
> +	}
> +}
> +
> +struct cpu_map *cpu_map__get(struct cpu_map *map)
> +{
> +	map->refcnt++;
> +	return map;
> +}
> +
> +void cpu_map__put(struct cpu_map *map)
> +{
> +	if (map && --map->refcnt == 0)
> +		cpu_map__delete(map);
>  }
>  
>  int cpu_map__get_socket(struct cpu_map *map, int idx)
> @@ -263,6 +282,7 @@ static int cpu_map__build_map(struct cpu_map *cpus, struct cpu_map **res,
>  	/* ensure we process id in increasing order */
>  	qsort(c->map, c->nr, sizeof(int), cmp_ids);
>  
> +	c->refcnt = 1;
>  	*res = c;
>  	return 0;
>  }
> diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
> index 61a654849002..19fe50b29375 100644
> --- a/tools/perf/util/cpumap.h
> +++ b/tools/perf/util/cpumap.h
> @@ -8,13 +8,13 @@
>  #include "util/debug.h"
>  
>  struct cpu_map {
> +	int refcnt;
>  	int nr;
>  	int map[];
>  };
>  
>  struct cpu_map *cpu_map__new(const char *cpu_list);
>  struct cpu_map *cpu_map__dummy_new(void);
> -void cpu_map__delete(struct cpu_map *map);
>  struct cpu_map *cpu_map__read(FILE *file);
>  size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
>  int cpu_map__get_socket(struct cpu_map *map, int idx);
> @@ -22,6 +22,9 @@ int cpu_map__get_core(struct cpu_map *map, int idx);
>  int cpu_map__build_socket_map(struct cpu_map *cpus, struct cpu_map **sockp);
>  int cpu_map__build_core_map(struct cpu_map *cpus, struct cpu_map **corep);
>  
> +struct cpu_map *cpu_map__get(struct cpu_map *map);
> +void cpu_map__put(struct cpu_map *map);
> +
>  static inline int cpu_map__socket(struct cpu_map *sock, int s)
>  {
>  	if (!sock || s > sock->nr || s < 0)
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index dc1dc2c181ef..6b895de1144d 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -114,7 +114,7 @@ void perf_evlist__delete(struct perf_evlist *evlist)
>  {
>  	perf_evlist__munmap(evlist);
>  	perf_evlist__close(evlist);
> -	cpu_map__delete(evlist->cpus);
> +	cpu_map__put(evlist->cpus);
>  	thread_map__delete(evlist->threads);
>  	evlist->cpus = NULL;
>  	evlist->threads = NULL;
> @@ -1337,7 +1337,7 @@ static int perf_evlist__create_syswide_maps(struct perf_evlist *evlist)
>  out:
>  	return err;
>  out_free_cpus:
> -	cpu_map__delete(evlist->cpus);
> +	cpu_map__put(evlist->cpus);
>  	evlist->cpus = NULL;
>  	goto out;
>  }
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index a3e36fc634dc..3bd5769dd7b9 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -902,6 +902,7 @@ void perf_evsel__exit(struct perf_evsel *evsel)
>  	perf_evsel__free_fd(evsel);
>  	perf_evsel__free_id(evsel);
>  	close_cgroup(evsel->cgrp);
> +	cpu_map__put(evsel->cpus);
>  	zfree(&evsel->group_name);
>  	zfree(&evsel->name);
>  	perf_evsel__object.fini(evsel);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 2a4d1ec02846..09f8d2357108 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -17,6 +17,7 @@
>  #include "parse-events-flex.h"
>  #include "pmu.h"
>  #include "thread_map.h"
> +#include "cpumap.h"
>  #include "asm/bug.h"
>  
>  #define MAX_NAME_LEN 100
> @@ -285,7 +286,9 @@ __add_event(struct list_head *list, int *idx,
>  	if (!evsel)
>  		return NULL;
>  
> -	evsel->cpus = cpus;
> +	if (cpus)
> +		evsel->cpus = cpu_map__get(cpus);
> +
>  	if (name)
>  		evsel->name = strdup(name);
>  	list_add_tail(&evsel->node, list);
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index d906d0ad5d40..b106d56df240 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -384,7 +384,7 @@ static int pyrf_cpu_map__init(struct pyrf_cpu_map *pcpus,
>  
>  static void pyrf_cpu_map__delete(struct pyrf_cpu_map *pcpus)
>  {
> -	cpu_map__delete(pcpus->cpus);
> +	cpu_map__put(pcpus->cpus);
>  	pcpus->ob_type->tp_free((PyObject*)pcpus);
>  }
>  
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index d457c523a33d..1f7becbe5e18 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -64,7 +64,7 @@ static bool perf_probe_api(setup_probe_fn_t fn)
>  	if (!cpus)
>  		return false;
>  	cpu = cpus->map[0];
> -	cpu_map__delete(cpus);
> +	cpu_map__put(cpus);
>  
>  	do {
>  		ret = perf_do_probe_api(fn, cpu, try[i++]);
> @@ -226,7 +226,7 @@ bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str)
>  		struct cpu_map *cpus = cpu_map__new(NULL);
>  
>  		cpu =  cpus ? cpus->map[0] : 0;
> -		cpu_map__delete(cpus);
> +		cpu_map__put(cpus);
>  	} else {
>  		cpu = evlist->cpus->map[0];
>  	}
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 88d87bf3049f..c09f178cea63 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1838,7 +1838,7 @@ int perf_session__cpu_bitmap(struct perf_session *session,
>  	err = 0;
>  
>  out_delete_map:
> -	cpu_map__delete(map);
> +	cpu_map__put(map);
>  	return err;
>  }
>  
> diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
> index 283d3e73e2f2..eec6c1149f44 100644
> --- a/tools/perf/util/svghelper.c
> +++ b/tools/perf/util/svghelper.c
> @@ -748,7 +748,7 @@ static int str_to_bitmap(char *s, cpumask_t *b)
>  		set_bit(c, cpumask_bits(b));
>  	}
>  
> -	cpu_map__delete(m);
> +	cpu_map__put(m);
>  
>  	return ret;
>  }
> -- 
> 1.9.3