Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

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

Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
> We need to store command names with the pid. Changing
> map to be struct holding pid. Process name is coming
> in shortly.
> 
> Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6vg7@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/builtin-trace.c                  |  4 ++--
>  tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
>  tools/perf/util/auxtrace.c                  |  4 ++--
>  tools/perf/util/event.c                     |  6 +++---
>  tools/perf/util/evlist.c                    |  4 ++--
>  tools/perf/util/evsel.c                     |  2 +-
>  tools/perf/util/thread_map.c                | 22 +++++++++++-----------
>  tools/perf/util/thread_map.h                |  8 +++++++-
>  8 files changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 4bf805b2fbf6..b75a68c385ea 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
>  	 */
>  	if (trace->filter_pids.nr > 0)
>  		err = perf_evlist__set_filter_pids(evlist, trace->filter_pids.nr, trace->filter_pids.entries);
> -	else if (evlist->threads->map[0] == -1)
> +	else if (thread_map__pid(evlist->threads, 0) == -1)
>  		err = perf_evlist__set_filter_pid(evlist, getpid());
>  
>  	if (err < 0) {
> @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
>  	if (forks)
>  		perf_evlist__start_workload(evlist);
>  
> -	trace->multiple_threads = evlist->threads->map[0] == -1 ||
> +	trace->multiple_threads = thread_map__pid(evlist->threads, 0) == -1 ||
>  				  evlist->threads->nr > 1 ||
>  				  perf_evlist__first(evlist)->attr.inherit;
>  again:
> diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c
> index 6245221479d7..ebc6e7938c9a 100644
> --- a/tools/perf/tests/openat-syscall-tp-fields.c
> +++ b/tools/perf/tests/openat-syscall-tp-fields.c
> @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
>  
>  	perf_evsel__config(evsel, &opts);
>  
> -	evlist->threads->map[0] = getpid();
> +	thread_map__pid(evlist->threads, 0) = getpid();

So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
we keep using:

	evlist->thread->map[0].pid = getpid();

I.e. just add the .pid to the existing usage.

- Arnaldo

>  
>  	err = perf_evlist__open(evlist);
>  	if (err < 0) {
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index df66966cfde7..3dab006b4a03 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -119,12 +119,12 @@ void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp,
>  	if (per_cpu) {
>  		mp->cpu = evlist->cpus->map[idx];
>  		if (evlist->threads)
> -			mp->tid = evlist->threads->map[0];
> +			mp->tid = thread_map__pid(evlist->threads, 0);
>  		else
>  			mp->tid = -1;
>  	} else {
>  		mp->cpu = -1;
> -		mp->tid = evlist->threads->map[idx];
> +		mp->tid = thread_map__pid(evlist->threads, idx);
>  	}
>  }
>  
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 793b1503d437..51a1bedf90fb 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -479,7 +479,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
>  	for (thread = 0; thread < threads->nr; ++thread) {
>  		if (__event__synthesize_thread(comm_event, mmap_event,
>  					       fork_event,
> -					       threads->map[thread], 0,
> +					       thread_map__pid(threads, thread), 0,
>  					       process, tool, machine,
>  					       mmap_data)) {
>  			err = -1;
> @@ -490,12 +490,12 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
>  		 * comm.pid is set to thread group id by
>  		 * perf_event__synthesize_comm
>  		 */
> -		if ((int) comm_event->comm.pid != threads->map[thread]) {
> +		if ((int) comm_event->comm.pid != thread_map__pid(threads, thread)) {
>  			bool need_leader = true;
>  
>  			/* is thread group leader in thread_map? */
>  			for (j = 0; j < threads->nr; ++j) {
> -				if ((int) comm_event->comm.pid == threads->map[j]) {
> +				if ((int) comm_event->comm.pid == thread_map__pid(threads, j)) {
>  					need_leader = false;
>  					break;
>  				}
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 8366511b45f8..a62b3adcc32b 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -548,7 +548,7 @@ static void perf_evlist__set_sid_idx(struct perf_evlist *evlist,
>  	else
>  		sid->cpu = -1;
>  	if (!evsel->system_wide && evlist->threads && thread >= 0)
> -		sid->tid = evlist->threads->map[thread];
> +		sid->tid = thread_map__pid(evlist->threads, thread);
>  	else
>  		sid->tid = -1;
>  }
> @@ -1475,7 +1475,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar
>  				__func__, __LINE__);
>  			goto out_close_pipes;
>  		}
> -		evlist->threads->map[0] = evlist->workload.pid;
> +		thread_map__pid(evlist->threads, 0) = evlist->workload.pid;
>  	}
>  
>  	close(child_ready_pipe[1]);
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 33449decf7bd..1b56047af96b 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1167,7 +1167,7 @@ retry_sample_id:
>  			int group_fd;
>  
>  			if (!evsel->cgrp && !evsel->system_wide)
> -				pid = threads->map[thread];
> +				pid = thread_map__pid(threads, thread);
>  
>  			group_fd = get_group_fd(evsel, cpu, thread);
>  retry_open:
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index f4822bd03709..7f03f9facfdd 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> @@ -22,7 +22,7 @@ static int filter(const struct dirent *dir)
>  
>  static struct thread_map *thread_map__realloc(struct thread_map *map, int nr)
>  {
> -	size_t size = sizeof(*map) + sizeof(pid_t) * nr;
> +	size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr;
>  
>  	return realloc(map, size);
>  }
> @@ -45,7 +45,7 @@ struct thread_map *thread_map__new_by_pid(pid_t pid)
>  	threads = thread_map__alloc(items);
>  	if (threads != NULL) {
>  		for (i = 0; i < items; i++)
> -			threads->map[i] = atoi(namelist[i]->d_name);
> +			thread_map__pid(threads, i) = atoi(namelist[i]->d_name);
>  		threads->nr = items;
>  	}
>  
> @@ -61,8 +61,8 @@ struct thread_map *thread_map__new_by_tid(pid_t tid)
>  	struct thread_map *threads = thread_map__alloc(1);
>  
>  	if (threads != NULL) {
> -		threads->map[0] = tid;
> -		threads->nr	= 1;
> +		thread_map__pid(threads, 0) = tid;
> +		threads->nr		    = 1;
>  	}
>  
>  	return threads;
> @@ -124,7 +124,7 @@ struct thread_map *thread_map__new_by_uid(uid_t uid)
>  		}
>  
>  		for (i = 0; i < items; i++)
> -			threads->map[threads->nr + i] = atoi(namelist[i]->d_name);
> +			thread_map__pid(threads, threads->nr + i) = atoi(namelist[i]->d_name);
>  
>  		for (i = 0; i < items; i++)
>  			zfree(&namelist[i]);
> @@ -201,7 +201,7 @@ static struct thread_map *thread_map__new_by_pid_str(const char *pid_str)
>  		threads = nt;
>  
>  		for (i = 0; i < items; i++) {
> -			threads->map[j++] = atoi(namelist[i]->d_name);
> +			thread_map__pid(threads, j++) = atoi(namelist[i]->d_name);
>  			zfree(&namelist[i]);
>  		}
>  		threads->nr = total_tasks;
> @@ -227,8 +227,8 @@ struct thread_map *thread_map__new_dummy(void)
>  	struct thread_map *threads = thread_map__alloc(1);
>  
>  	if (threads != NULL) {
> -		threads->map[0]	= -1;
> -		threads->nr	= 1;
> +		thread_map__pid(threads, 0) = -1;
> +		threads->nr		    = 1;
>  	}
>  	return threads;
>  }
> @@ -267,8 +267,8 @@ static struct thread_map *thread_map__new_by_tid_str(const char *tid_str)
>  			goto out_free_threads;
>  
>  		threads = nt;
> -		threads->map[ntasks - 1] = tid;
> -		threads->nr		 = ntasks;
> +		thread_map__pid(threads, ntasks - 1) = tid;
> +		threads->nr			     = ntasks;
>  	}
>  out:
>  	return threads;
> @@ -301,7 +301,7 @@ size_t thread_map__fprintf(struct thread_map *threads, FILE *fp)
>  	size_t printed = fprintf(fp, "%d thread%s: ",
>  				 threads->nr, threads->nr > 1 ? "s" : "");
>  	for (i = 0; i < threads->nr; ++i)
> -		printed += fprintf(fp, "%s%d", i ? ", " : "", threads->map[i]);
> +		printed += fprintf(fp, "%s%d", i ? ", " : "", thread_map__pid(threads, i));
>  
>  	return printed + fprintf(fp, "\n");
>  }
> diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
> index 95313f43cc0f..9377850c7b71 100644
> --- a/tools/perf/util/thread_map.h
> +++ b/tools/perf/util/thread_map.h
> @@ -4,11 +4,17 @@
>  #include <sys/types.h>
>  #include <stdio.h>
>  
> +struct thread_map_data {
> +	pid_t    pid;
> +};
> +
>  struct thread_map {
>  	int nr;
> -	pid_t map[];
> +	struct thread_map_data map[];
>  };
>  
> +#define thread_map__pid(__m, __t)  __m->map[__t].pid
> +
>  struct thread_map *thread_map__new_dummy(void);
>  struct thread_map *thread_map__new_by_pid(pid_t pid);
>  struct thread_map *thread_map__new_by_tid(pid_t tid);
> -- 
> 1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/