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

Arnaldo Carvalho de Melo
2015-06-10T19:08:00Z

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