Commit 97ea1a7fa62af0d8d49a0fc12796b0073537c9d8
Committed by
Ingo Molnar
1 parent
2e538c4a18
Exists in
master
and in
7 other branches
perf tools: Fix thread comm resolution in perf sched
This reverts commit 9a92b479b2f088ee2d3194243f4c8e59b1b8c9c2 ("perf tools: Improve thread comm resolution in perf sched") and fixes the real bug. The bug was elsewhere: We are failing to resolve thread names in perf sched because the table of threads we are building, on top of comm events, has a per process granularity. But perf sched, unlike the other perf tools, needs a per thread granularity as we are profiling every tasks individually. So fix it by building our threads table using the tid instead of the pid as the thread identifier. v2: Revert the previous fix - it is not really needed Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> LKML-Reference: <1255028657-11158-1-git-send-email-fweisbec@gmail.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Showing 3 changed files with 13 additions and 68 deletions Side-by-side Diff
tools/perf/builtin-sched.c
... | ... | @@ -638,7 +638,7 @@ |
638 | 638 | { |
639 | 639 | struct thread *thread; |
640 | 640 | |
641 | - thread = threads__findnew(event->comm.pid, &threads, &last_match); | |
641 | + thread = threads__findnew(event->comm.tid, &threads, &last_match); | |
642 | 642 | |
643 | 643 | dump_printf("%p [%p]: perf_event_comm: %s:%d\n", |
644 | 644 | (void *)(offset + head), |
... | ... | @@ -1034,36 +1034,6 @@ |
1034 | 1034 | atoms->nb_atoms++; |
1035 | 1035 | } |
1036 | 1036 | |
1037 | -static struct thread * | |
1038 | -threads__findnew_from_ctx(u32 pid, struct trace_switch_event *switch_event) | |
1039 | -{ | |
1040 | - struct thread *th; | |
1041 | - | |
1042 | - th = threads__findnew_nocomm(pid, &threads, &last_match); | |
1043 | - if (th->comm) | |
1044 | - return th; | |
1045 | - | |
1046 | - if (pid == switch_event->prev_pid) | |
1047 | - thread__set_comm(th, switch_event->prev_comm); | |
1048 | - else | |
1049 | - thread__set_comm(th, switch_event->next_comm); | |
1050 | - return th; | |
1051 | -} | |
1052 | - | |
1053 | -static struct thread * | |
1054 | -threads__findnew_from_wakeup(struct trace_wakeup_event *wakeup_event) | |
1055 | -{ | |
1056 | - struct thread *th; | |
1057 | - | |
1058 | - th = threads__findnew_nocomm(wakeup_event->pid, &threads, &last_match); | |
1059 | - if (th->comm) | |
1060 | - return th; | |
1061 | - | |
1062 | - thread__set_comm(th, wakeup_event->comm); | |
1063 | - | |
1064 | - return th; | |
1065 | -} | |
1066 | - | |
1067 | 1037 | static void |
1068 | 1038 | latency_switch_event(struct trace_switch_event *switch_event, |
1069 | 1039 | struct event *event __used, |
... | ... | @@ -1089,10 +1059,8 @@ |
1089 | 1059 | die("hm, delta: %Ld < 0 ?\n", delta); |
1090 | 1060 | |
1091 | 1061 | |
1092 | - sched_out = threads__findnew_from_ctx(switch_event->prev_pid, | |
1093 | - switch_event); | |
1094 | - sched_in = threads__findnew_from_ctx(switch_event->next_pid, | |
1095 | - switch_event); | |
1062 | + sched_out = threads__findnew(switch_event->prev_pid, &threads, &last_match); | |
1063 | + sched_in = threads__findnew(switch_event->next_pid, &threads, &last_match); | |
1096 | 1064 | |
1097 | 1065 | out_events = thread_atoms_search(&atom_root, sched_out, &cmp_pid); |
1098 | 1066 | if (!out_events) { |
... | ... | @@ -1158,7 +1126,7 @@ |
1158 | 1126 | if (!wakeup_event->success) |
1159 | 1127 | return; |
1160 | 1128 | |
1161 | - wakee = threads__findnew_from_wakeup(wakeup_event); | |
1129 | + wakee = threads__findnew(wakeup_event->pid, &threads, &last_match); | |
1162 | 1130 | atoms = thread_atoms_search(&atom_root, wakee, &cmp_pid); |
1163 | 1131 | if (!atoms) { |
1164 | 1132 | thread_atoms_insert(wakee); |
... | ... | @@ -1418,10 +1386,8 @@ |
1418 | 1386 | die("hm, delta: %Ld < 0 ?\n", delta); |
1419 | 1387 | |
1420 | 1388 | |
1421 | - sched_out = threads__findnew_from_ctx(switch_event->prev_pid, | |
1422 | - switch_event); | |
1423 | - sched_in = threads__findnew_from_ctx(switch_event->next_pid, | |
1424 | - switch_event); | |
1389 | + sched_out = threads__findnew(switch_event->prev_pid, &threads, &last_match); | |
1390 | + sched_in = threads__findnew(switch_event->next_pid, &threads, &last_match); | |
1425 | 1391 | |
1426 | 1392 | curr_thread[this_cpu] = sched_in; |
1427 | 1393 |
tools/perf/util/thread.c
... | ... | @@ -6,17 +6,15 @@ |
6 | 6 | #include "util.h" |
7 | 7 | #include "debug.h" |
8 | 8 | |
9 | -static struct thread *thread__new(pid_t pid, int set_comm) | |
9 | +static struct thread *thread__new(pid_t pid) | |
10 | 10 | { |
11 | 11 | struct thread *self = calloc(1, sizeof(*self)); |
12 | 12 | |
13 | 13 | if (self != NULL) { |
14 | 14 | self->pid = pid; |
15 | - if (set_comm) { | |
16 | - self->comm = malloc(32); | |
17 | - if (self->comm) | |
18 | - snprintf(self->comm, 32, ":%d", self->pid); | |
19 | - } | |
15 | + self->comm = malloc(32); | |
16 | + if (self->comm) | |
17 | + snprintf(self->comm, 32, ":%d", self->pid); | |
20 | 18 | self->maps = RB_ROOT; |
21 | 19 | INIT_LIST_HEAD(&self->removed_maps); |
22 | 20 | } |
... | ... | @@ -52,10 +50,8 @@ |
52 | 50 | return ret; |
53 | 51 | } |
54 | 52 | |
55 | -static struct thread * | |
56 | -__threads__findnew(pid_t pid, struct rb_root *threads, | |
57 | - struct thread **last_match, | |
58 | - int set_comm) | |
53 | +struct thread * | |
54 | +threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match) | |
59 | 55 | { |
60 | 56 | struct rb_node **p = &threads->rb_node; |
61 | 57 | struct rb_node *parent = NULL; |
... | ... | @@ -84,8 +80,7 @@ |
84 | 80 | p = &(*p)->rb_right; |
85 | 81 | } |
86 | 82 | |
87 | - th = thread__new(pid, set_comm); | |
88 | - | |
83 | + th = thread__new(pid); | |
89 | 84 | if (th != NULL) { |
90 | 85 | rb_link_node(&th->rb_node, parent, p); |
91 | 86 | rb_insert_color(&th->rb_node, threads); |
... | ... | @@ -93,19 +88,6 @@ |
93 | 88 | } |
94 | 89 | |
95 | 90 | return th; |
96 | -} | |
97 | - | |
98 | -struct thread * | |
99 | -threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match) | |
100 | -{ | |
101 | - return __threads__findnew(pid, threads, last_match, 1); | |
102 | -} | |
103 | - | |
104 | -struct thread * | |
105 | -threads__findnew_nocomm(pid_t pid, struct rb_root *threads, | |
106 | - struct thread **last_match) | |
107 | -{ | |
108 | - return __threads__findnew(pid, threads, last_match, 0); | |
109 | 91 | } |
110 | 92 | |
111 | 93 | struct thread * |
tools/perf/util/thread.h
... | ... | @@ -18,9 +18,6 @@ |
18 | 18 | struct thread * |
19 | 19 | threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match); |
20 | 20 | struct thread * |
21 | -threads__findnew_nocomm(pid_t pid, struct rb_root *threads, | |
22 | - struct thread **last_match); | |
23 | -struct thread * | |
24 | 21 | register_idle_thread(struct rb_root *threads, struct thread **last_match); |
25 | 22 | void thread__insert_map(struct thread *self, struct map *map); |
26 | 23 | int thread__fork(struct thread *self, struct thread *parent); |