Hi:
On 3/13/13 3:42 AM, chenggang wrote:
---
tools/perf/builtin-stat.c | 2 +-
tools/perf/tests/open-syscall-tp-fields.c | 2 +-
tools/perf/util/event.c | 12 +-
tools/perf/util/evlist.c | 2 +-
tools/perf/util/evsel.c | 16 +-
tools/perf/util/python.c | 2 +-
tools/perf/util/thread_map.c | 281 ++++++++++++++++++++++-------
tools/perf/util/thread_map.h | 17 +-
8 files changed, 244 insertions(+), 90 deletions(-)
You need to target smaller changes per patch. Think small, bisectable
changes that evolve the code to where you want it to be.
For example, start with a patch that introduces the API
thread_map__set_pid_by_idx:
int thread_map__set_pid_by_idx(struct thread_map *map, int idx, pid_t pid)
{
if (idx >= map->nr)
return -EINVAL;
map[idx] = pid;
return 0;
}
and use that method for the perf-stat change,
tests/open-syscall-tp-fields.c and util/evlist.c. That's patch 1 --
small and focused.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 9984876..293b09c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -401,7 +401,7 @@ static int __run_perf_stat(int argc __maybe_unused, const
char **argv)
}
if (perf_target__none(&target))
- evsel_list->threads->map[0] = child_pid;
+ thread_map__set_pid(evsel_list->threads, 0, child_pid);
/*
* Wait for the child to be ready to exec.
diff --git a/tools/perf/tests/open-syscall-tp-fields.c
b/tools/perf/tests/open-syscall-tp-fields.c
index 1c52fdc..39eb770 100644
--- a/tools/perf/tests/open-syscall-tp-fields.c
+++ b/tools/perf/tests/open-syscall-tp-fields.c
@@ -43,7 +43,7 @@ int test__syscall_open_tp_fields(void)
perf_evsel__config(evsel, &opts);
- evlist->threads->map[0] = getpid();
+ thread_map__append(evlist->threads, getpid());
err = perf_evlist__open(evlist);
if (err < 0) {
The second small patch introduces the method thread_map__get_pid_by_idx
which is the counterpart to thread_map__set_pid_by_idx -- this time
returning the pid for a given index. This new method fixes this use in
util/event.c and the one in python.c below.
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 5cd13d7..d093460 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -326,9 +326,11 @@ int perf_event__synthesize_thread_map(struct perf_tool
*tool,
err = 0;
for (thread = 0; thread < threads->nr; ++thread) {
+ pid_t pid = thread_map__get_pid(threads, thread);
+
if (__event__synthesize_thread(comm_event, mmap_event,
- threads->map[thread], 0,
- process, tool, machine)) {
+ pid, 0, process, tool,
+ machine)) {
err = -1;
break;
}
@@ -337,12 +339,14 @@ 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 != pid) {
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]) {
+ pid_t pidj = thread_map__get_pid(threads, j);
+
+ if ((int) comm_event->comm.pid == pidj) {
need_leader = false;
break;
}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bc4ad79..d5063d6 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -793,7 +793,7 @@ int perf_evlist__prepare_workload(struct perf_evlist
*evlist,
}
if (perf_target__none(&opts->target))
- evlist->threads->map[0] = evlist->workload.pid;
+ thread_map__append(evlist->threads, evlist->workload.pid);
Here you can use thread_map__set_pid_by_idx. When you convert the
xyarray implementation you can come back to this call and change it to
thread_map__append or have set_pid_by_idx do the append internally if
the idx == threads->nr
close(child_ready_pipe[1]);
close(go_pipe[0]);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 9c82f98f..57c569d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -835,7 +835,7 @@ retry_sample_id:
int group_fd;
if (!evsel->cgrp)
- pid = threads->map[thread];
+ pid = thread_map__get_pid(threads, thread);
group_fd = get_group_fd(evsel, cpu, thread);
This part here can be a separate stand-alone patch. In thread_map.c
introduce the method:
struct thread_map *thread_map__empty_thread_map(void)
{
static struct {
struct thread_map map;
int threads[1];
} empty_thread_map = {
.map.nr = 1,
.threads = { -1, },
};
return &empty_thread_map.map;
}
In a follow up patch you can change the implementation of this method
but for now you want a small bisectable change.
@@ -894,14 +894,6 @@ static struct {
.cpus = { -1, },
};
-static struct {
- struct thread_map map;
- int threads[1];
-} empty_thread_map = {
- .map.nr = 1,
- .threads = { -1, },
-};
-
keep the above code removal and fix the 2 below fixes.
int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
struct thread_map *threads)
{
@@ -911,7 +903,7 @@ int perf_evsel__open(struct perf_evsel *evsel, struct
cpu_map *cpus,
}
if (threads == NULL)
- threads = &empty_thread_map.map;
+ threads = thread_map__empty_thread_map();
return __perf_evsel__open(evsel, cpus, threads);
}
@@ -919,7 +911,9 @@ int perf_evsel__open(struct perf_evsel *evsel, struct
cpu_map *cpus,
int perf_evsel__open_per_cpu(struct perf_evsel *evsel,
struct cpu_map *cpus)
{
- return __perf_evsel__open(evsel, cpus, &empty_thread_map.map);
+ struct thread_map *empty_thread_map = thread_map__empty_thread_map();
+
+ return __perf_evsel__open(evsel, cpus, empty_thread_map);
}
int perf_evsel__open_per_thread(struct perf_evsel *evsel,
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 925e0c3..e3f3f1b 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -458,7 +458,7 @@ static PyObject *pyrf_thread_map__item(PyObject *obj,
Py_ssize_t i)
if (i >= pthreads->threads->nr)
return NULL;
- return Py_BuildValue("i", pthreads->threads->map[i]);
+ return Py_BuildValue("i", thread_map__get_pid(pthreads->threads, i));
}
static PySequenceMethods pyrf_thread_map__sequence_methods = {
Once existing uses of threads->map are consolidated you can create a
patch to convert the thread_map code to xyarray and introduce new
methods needed.
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 9b5f856..301f4ce 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -19,9 +19,116 @@ static int filter(const struct dirent *dir)
return 1;
}
Does that make sense?
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
