]>
Commit | Line | Data |
---|---|---|
df1b7722 GKH |
1 | From foo@baz Mon Apr 9 13:58:16 CEST 2018 |
2 | From: Mengting Zhang <zhangmengting@huawei.com> | |
3 | Date: Wed, 13 Dec 2017 15:01:53 +0800 | |
4 | Subject: perf evsel: Enable ignore_missing_thread for pid option | |
5 | ||
6 | From: Mengting Zhang <zhangmengting@huawei.com> | |
7 | ||
8 | ||
9 | [ Upstream commit ca8000684ec4e66f965e1f9547a3c6cb834154ca ] | |
10 | ||
11 | While monitoring a multithread process with pid option, perf sometimes | |
12 | may return sys_perf_event_open failure with 3(No such process) if any of | |
13 | the process's threads die before we open the event. However, we want | |
14 | perf continue monitoring the remaining threads and do not exit with | |
15 | error. | |
16 | ||
17 | Here, the patch enables perf_evsel::ignore_missing_thread for -p option | |
18 | to ignore complete failure if any of threads die before we open the event. | |
19 | But it may still return sys_perf_event_open failure with 22(Invalid) if we | |
20 | monitors several event groups. | |
21 | ||
22 | sys_perf_event_open: pid 28960 cpu 40 group_fd 118202 flags 0x8 | |
23 | sys_perf_event_open: pid 28961 cpu 40 group_fd 118203 flags 0x8 | |
24 | WARNING: Ignored open failure for pid 28962 | |
25 | sys_perf_event_open: pid 28962 cpu 40 group_fd [118203] flags 0x8 | |
26 | sys_perf_event_open failed, error -22 | |
27 | ||
28 | That is because when we ignore a missing thread, we change the thread_idx | |
29 | without dealing with its fds, FD(evsel, cpu, thread). Then get_group_fd() | |
30 | may return a wrong group_fd for the next thread and sys_perf_event_open() | |
31 | return with 22. | |
32 | ||
33 | sys_perf_event_open(){ | |
34 | ... | |
35 | if (group_fd != -1) | |
36 | perf_fget_light()//to get corresponding group_leader by group_fd | |
37 | ... | |
38 | if (group_leader) | |
39 | if (group_leader->ctx->task != ctx->task)//should on the same task | |
40 | goto err_context | |
41 | ... | |
42 | } | |
43 | ||
44 | This patch also fixes this bug by introducing perf_evsel__remove_fd() and | |
45 | update_fds to allow removing fds for the missing thread. | |
46 | ||
47 | Changes since v1: | |
48 | - Change group_fd__remove() into a more genetic way without changing code logic | |
49 | - Remove redundant condition | |
50 | ||
51 | Changes since v2: | |
52 | - Use a proper function name and add some comment. | |
53 | - Multiline comment style fixes. | |
54 | ||
55 | Committer testing: | |
56 | ||
57 | Before this patch the recently added 'perf stat --per-thread' for system | |
58 | wide counting would race while enumerating all threads using /proc: | |
59 | ||
60 | [root@jouet ~]# perf stat --per-thread | |
61 | failed to parse CPUs map: No such file or directory | |
62 | ||
63 | Usage: perf stat [<options>] [<command>] | |
64 | ||
65 | -C, --cpu <cpu> list of cpus to monitor in system-wide | |
66 | -a, --all-cpus system-wide collection from all CPUs | |
67 | [root@jouet ~]# perf stat --per-thread | |
68 | failed to parse CPUs map: No such file or directory | |
69 | ||
70 | Usage: perf stat [<options>] [<command>] | |
71 | ||
72 | -C, --cpu <cpu> list of cpus to monitor in system-wide | |
73 | -a, --all-cpus system-wide collection from all CPUs | |
74 | [root@jouet ~]# | |
75 | ||
76 | When, say, the kernel was being built, so lots of shortlived threads, | |
77 | after this patch this doesn't happen. | |
78 | ||
79 | Signed-off-by: Mengting Zhang <zhangmengting@huawei.com> | |
80 | Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> | |
81 | Acked-by: Jiri Olsa <jolsa@redhat.com> | |
82 | Cc: Cheng Jian <cj.chengjian@huawei.com> | |
83 | Cc: Li Bin <huawei.libin@huawei.com> | |
84 | Cc: Wang Nan <wangnan0@huawei.com> | |
85 | Link: http://lkml.kernel.org/r/1513148513-6974-1-git-send-email-zhangmengting@huawei.com | |
86 | [ Remove one use 'evlist' alias variable ] | |
87 | Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> | |
88 | Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> | |
89 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
90 | --- | |
91 | tools/perf/builtin-record.c | 4 +-- | |
92 | tools/perf/util/evsel.c | 47 ++++++++++++++++++++++++++++++++++++++++++-- | |
93 | 2 files changed, 47 insertions(+), 4 deletions(-) | |
94 | ||
95 | --- a/tools/perf/builtin-record.c | |
96 | +++ b/tools/perf/builtin-record.c | |
97 | @@ -1856,8 +1856,8 @@ int cmd_record(int argc, const char **ar | |
98 | goto out; | |
99 | } | |
100 | ||
101 | - /* Enable ignoring missing threads when -u option is defined. */ | |
102 | - rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX; | |
103 | + /* Enable ignoring missing threads when -u/-p option is defined. */ | |
104 | + rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX || rec->opts.target.pid; | |
105 | ||
106 | err = -ENOMEM; | |
107 | if (perf_evlist__create_maps(rec->evlist, &rec->opts.target) < 0) | |
108 | --- a/tools/perf/util/evsel.c | |
109 | +++ b/tools/perf/util/evsel.c | |
110 | @@ -1591,10 +1591,46 @@ static int __open_attr__fprintf(FILE *fp | |
111 | return fprintf(fp, " %-32s %s\n", name, val); | |
112 | } | |
113 | ||
114 | +static void perf_evsel__remove_fd(struct perf_evsel *pos, | |
115 | + int nr_cpus, int nr_threads, | |
116 | + int thread_idx) | |
117 | +{ | |
118 | + for (int cpu = 0; cpu < nr_cpus; cpu++) | |
119 | + for (int thread = thread_idx; thread < nr_threads - 1; thread++) | |
120 | + FD(pos, cpu, thread) = FD(pos, cpu, thread + 1); | |
121 | +} | |
122 | + | |
123 | +static int update_fds(struct perf_evsel *evsel, | |
124 | + int nr_cpus, int cpu_idx, | |
125 | + int nr_threads, int thread_idx) | |
126 | +{ | |
127 | + struct perf_evsel *pos; | |
128 | + | |
129 | + if (cpu_idx >= nr_cpus || thread_idx >= nr_threads) | |
130 | + return -EINVAL; | |
131 | + | |
132 | + evlist__for_each_entry(evsel->evlist, pos) { | |
133 | + nr_cpus = pos != evsel ? nr_cpus : cpu_idx; | |
134 | + | |
135 | + perf_evsel__remove_fd(pos, nr_cpus, nr_threads, thread_idx); | |
136 | + | |
137 | + /* | |
138 | + * Since fds for next evsel has not been created, | |
139 | + * there is no need to iterate whole event list. | |
140 | + */ | |
141 | + if (pos == evsel) | |
142 | + break; | |
143 | + } | |
144 | + return 0; | |
145 | +} | |
146 | + | |
147 | static bool ignore_missing_thread(struct perf_evsel *evsel, | |
148 | + int nr_cpus, int cpu, | |
149 | struct thread_map *threads, | |
150 | int thread, int err) | |
151 | { | |
152 | + pid_t ignore_pid = thread_map__pid(threads, thread); | |
153 | + | |
154 | if (!evsel->ignore_missing_thread) | |
155 | return false; | |
156 | ||
157 | @@ -1610,11 +1646,18 @@ static bool ignore_missing_thread(struct | |
158 | if (threads->nr == 1) | |
159 | return false; | |
160 | ||
161 | + /* | |
162 | + * We should remove fd for missing_thread first | |
163 | + * because thread_map__remove() will decrease threads->nr. | |
164 | + */ | |
165 | + if (update_fds(evsel, nr_cpus, cpu, threads->nr, thread)) | |
166 | + return false; | |
167 | + | |
168 | if (thread_map__remove(threads, thread)) | |
169 | return false; | |
170 | ||
171 | pr_warning("WARNING: Ignored open failure for pid %d\n", | |
172 | - thread_map__pid(threads, thread)); | |
173 | + ignore_pid); | |
174 | return true; | |
175 | } | |
176 | ||
177 | @@ -1719,7 +1762,7 @@ retry_open: | |
178 | if (fd < 0) { | |
179 | err = -errno; | |
180 | ||
181 | - if (ignore_missing_thread(evsel, threads, thread, err)) { | |
182 | + if (ignore_missing_thread(evsel, cpus->nr, cpu, threads, thread, err)) { | |
183 | /* | |
184 | * We just removed 1 thread, so take a step | |
185 | * back on thread index and lower the upper |