Messages in this thread | | | Date | Thu, 19 Feb 2015 18:17:36 +0200 | From | Adrian Hunter <> | Subject | Re: [PATCH] perf: Fix probing for PERF_FLAG_FD_CLOEXEC flag |
| |
On 19/02/2015 4:55 p.m., David Ahern wrote: > On 2/19/15 12:06 AM, Adrian Hunter wrote: >>> /* not supported, confirm error related to PERF_FLAG_FD_CLOEXEC */ >>> - fd = sys_perf_event_open(&attr, pid, cpu, -1, 0); >>> + fd = sys_perf_event_open(&attr, 0, cpu, -1, 0); >> >> I would prefer to avoid pid = 0 unless necessary and so just do the same >> thing again i.e. >> >> while (1) { >> fd = sys_perf_event_open(&attr, pid, cpu, -1, 0); >> if (fd < 0 && pid == -1 && errno == EACCES) { >> pid = 0; >> continue; >> } >> break; >> } >> > > The probing is getting of hand. In this case the intent is a probe for a flag > and flags are the first thing checked kernel side. Given that the parameters > passed to sys_perf_event_open should be as simple and known safe as possible. > pid = -1 has known limitations. Why can't pid just be getpid() in both cases? > > Simplifies this function a lot and removes the need for sched_getcpu(). So > pid = getpid(); > > fd = sys_perf_event_open(&attr, pid, -1, -1, PERF_FLAG_FD_CLOEXEC); > > and if that fails > > fd = sys_perf_event_open(&attr, pid, -1, -1, 0); > > Why is anything more complicated needed?
Yes, I am sorry it is a pain. I don't know why I didn't add a comment to the code :-(. Using -1 for the pid is a workaround to avoid gratuitous jump label changes. If pid=0 is used and then a system-wide trace is done with Intel PT, there will be a jump label change shortly after the tracing starts. That means the running code gets changed, but Intel PT decoding has to walk the code to reconstruct the trace - so errors result. There will always be occasional jump label changes, but this avoids one that would otherwise always happen.
| |