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

From: Arnaldo Carvalho de Melo
Date: Fri Jun 19 2015 - 17:07:31 EST


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@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> 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/