Re: [PATCH 04/25] perf tools: Add reference counting for thread_map object

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

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

Please use atomic.h, as was done recently for the 'map', 'dso' and
'thread' structs, but other than that, its fine.
 
> Link: http://lkml.kernel.org/n/tip-myadl53clbkjvzeqolwp95w0@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/mmap-thread-lookup.c      |  2 +-
>  tools/perf/tests/openat-syscall-all-cpus.c |  2 +-
>  tools/perf/tests/openat-syscall.c          |  2 +-
>  tools/perf/tests/switch-tracking.c         |  2 +-
>  tools/perf/util/evlist.c                   |  4 ++--
>  tools/perf/util/python.c                   |  2 +-
>  tools/perf/util/thread_map.c               | 28 ++++++++++++++++++++++++++--
>  tools/perf/util/thread_map.h               |  6 ++++--
>  11 files changed, 40 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 86d17d2e9e50..7dcd2c3d865f 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -546,7 +546,7 @@ out_err:
>  		perf_evlist__delete(evlist);
>  	} else {
>  		cpu_map__put(cpus);
> -		thread_map__delete(threads);
> +		thread_map__put(threads);
>  	}
>  	machines__destroy_kernel_maps(&machines);
>  	machine__delete_threads(machine);
> diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
> index a330235cefc0..4d4b9837b630 100644
> --- a/tools/perf/tests/keep-tracking.c
> +++ b/tools/perf/tests/keep-tracking.c
> @@ -145,7 +145,7 @@ out_err:
>  		perf_evlist__delete(evlist);
>  	} else {
>  		cpu_map__put(cpus);
> -		thread_map__delete(threads);
> +		thread_map__put(threads);
>  	}
>  
>  	return err;
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index 5a9ef5833452..666b67a4df9d 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -142,6 +142,6 @@ out_delete_evlist:
>  out_free_cpus:
>  	cpu_map__put(cpus);
>  out_free_threads:
> -	thread_map__delete(threads);
> +	thread_map__put(threads);
>  	return err;
>  }
> diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
> index 264e215c0d36..412a41eb3f81 100644
> --- a/tools/perf/tests/mmap-thread-lookup.c
> +++ b/tools/perf/tests/mmap-thread-lookup.c
> @@ -143,7 +143,7 @@ static int synth_process(struct machine *machine)
>  						perf_event__process,
>  						machine, 0);
>  
> -	thread_map__delete(map);
> +	thread_map__put(map);
>  	return err;
>  }
>  
> diff --git a/tools/perf/tests/openat-syscall-all-cpus.c b/tools/perf/tests/openat-syscall-all-cpus.c
> index e34dfdf96b5a..8801983a38f0 100644
> --- a/tools/perf/tests/openat-syscall-all-cpus.c
> +++ b/tools/perf/tests/openat-syscall-all-cpus.c
> @@ -110,6 +110,6 @@ out_close_fd:
>  out_evsel_delete:
>  	perf_evsel__delete(evsel);
>  out_thread_map_delete:
> -	thread_map__delete(threads);
> +	thread_map__put(threads);
>  	return err;
>  }
> diff --git a/tools/perf/tests/openat-syscall.c b/tools/perf/tests/openat-syscall.c
> index 9f9491bb8e48..bdfa1f446681 100644
> --- a/tools/perf/tests/openat-syscall.c
> +++ b/tools/perf/tests/openat-syscall.c
> @@ -56,6 +56,6 @@ out_close_fd:
>  out_evsel_delete:
>  	perf_evsel__delete(evsel);
>  out_thread_map_delete:
> -	thread_map__delete(threads);
> +	thread_map__put(threads);
>  	return err;
>  }
> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
> index 1b06122beb76..e698742d4fec 100644
> --- a/tools/perf/tests/switch-tracking.c
> +++ b/tools/perf/tests/switch-tracking.c
> @@ -561,7 +561,7 @@ out:
>  		perf_evlist__delete(evlist);
>  	} else {
>  		cpu_map__put(cpus);
> -		thread_map__delete(threads);
> +		thread_map__put(threads);
>  	}
>  
>  	return err;
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 6b895de1144d..493675cfa90c 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -115,7 +115,7 @@ void perf_evlist__delete(struct perf_evlist *evlist)
>  	perf_evlist__munmap(evlist);
>  	perf_evlist__close(evlist);
>  	cpu_map__put(evlist->cpus);
> -	thread_map__delete(evlist->threads);
> +	thread_map__put(evlist->threads);
>  	evlist->cpus = NULL;
>  	evlist->threads = NULL;
>  	perf_evlist__purge(evlist);
> @@ -1104,7 +1104,7 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>  	return 0;
>  
>  out_delete_threads:
> -	thread_map__delete(evlist->threads);
> +	thread_map__put(evlist->threads);
>  	evlist->threads = NULL;
>  	return -1;
>  }
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index b106d56df240..626422eda727 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -453,7 +453,7 @@ static int pyrf_thread_map__init(struct pyrf_thread_map *pthreads,
>  
>  static void pyrf_thread_map__delete(struct pyrf_thread_map *pthreads)
>  {
> -	thread_map__delete(pthreads->threads);
> +	thread_map__put(pthreads->threads);
>  	pthreads->ob_type->tp_free((PyObject*)pthreads);
>  }
>  
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index f93b9734735b..04bc15e339dd 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> @@ -8,6 +8,7 @@
>  #include <unistd.h>
>  #include "strlist.h"
>  #include <string.h>
> +#include "asm/bug.h"
>  #include "thread_map.h"
>  #include "util.h"
>  
> @@ -38,6 +39,7 @@ struct thread_map *thread_map__new_by_pid(pid_t pid)
>  		for (i = 0; i < items; i++)
>  			threads->map[i] = atoi(namelist[i]->d_name);
>  		threads->nr = items;
> +		threads->refcnt = 1;
>  	}
>  
>  	for (i=0; i<items; i++)
> @@ -54,6 +56,7 @@ struct thread_map *thread_map__new_by_tid(pid_t tid)
>  	if (threads != NULL) {
>  		threads->map[0] = tid;
>  		threads->nr	= 1;
> +		threads->refcnt = 1;
>  	}
>  
>  	return threads;
> @@ -75,6 +78,7 @@ struct thread_map *thread_map__new_by_uid(uid_t uid)
>  		goto out_free_threads;
>  
>  	threads->nr = 0;
> +	threads->refcnt = 1;
>  
>  	while (!readdir_r(proc, &dirent, &next) && next) {
>  		char *end;
> @@ -202,6 +206,8 @@ static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
>  
>  out:
>  	strlist__delete(slist);
> +	if (threads)
> +		threads->refcnt = 1;
>  	return threads;
>  
>  out_free_namelist:
> @@ -221,6 +227,7 @@ struct thread_map *thread_map__new_dummy(void)
>  	if (threads != NULL) {
>  		threads->map[0]	= -1;
>  		threads->nr	= 1;
> +		threads->refcnt = 1;
>  	}
>  	return threads;
>  }
> @@ -263,6 +270,8 @@ static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
>  		threads->nr		 = ntasks;
>  	}
>  out:
> +	if (threads)
> +		threads->refcnt = 1;
>  	return threads;
>  
>  out_free_threads:
> @@ -282,9 +291,24 @@ struct thread_map *thread_map__new_str(const char *pid, const char *tid,
>  	return thread_map__new_by_tid_str(tid);
>  }
>  
> -void thread_map__delete(struct thread_map *threads)
> +static void thread_map__delete(struct thread_map *threads)
>  {
> -	free(threads);
> +	if (threads) {
> +		WARN_ONCE(threads->refcnt != 0, "thread map refcnt disbalanced\n");
> +		free(threads);
> +	}
> +}
> +
> +struct thread_map *thread_map__get(struct thread_map *map)
> +{
> +	map->refcnt++;
> +	return map;
> +}
> +
> +void thread_map__put(struct thread_map *map)
> +{
> +	if (map && --map->refcnt == 0)
> +		thread_map__delete(map);
>  }
>  
>  size_t thread_map__fprintf(struct thread_map *threads, FILE *fp)
> diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
> index 95313f43cc0f..dc02c259da36 100644
> --- a/tools/perf/util/thread_map.h
> +++ b/tools/perf/util/thread_map.h
> @@ -5,6 +5,7 @@
>  #include <stdio.h>
>  
>  struct thread_map {
> +	int refcnt;
>  	int nr;
>  	pid_t map[];
>  };
> @@ -15,11 +16,12 @@ struct thread_map *thread_map__new_by_tid(pid_t tid);
>  struct thread_map *thread_map__new_by_uid(uid_t uid);
>  struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid);
>  
> +struct thread_map *thread_map__get(struct thread_map *map);
> +void thread_map__put(struct thread_map *map);
> +
>  struct thread_map *thread_map__new_str(const char *pid,
>  		const char *tid, uid_t uid);
>  
> -void thread_map__delete(struct thread_map *threads);
> -
>  size_t thread_map__fprintf(struct thread_map *threads, FILE *fp);
>  
>  static inline int thread_map__nr(struct thread_map *threads)
> -- 
> 1.9.3