* [PATCH] perf: fix comm for processes with named threads
@ 2011-12-09 16:00 David Ahern
2011-12-09 17:08 ` David Ahern
0 siblings, 1 reply; 2+ messages in thread
From: David Ahern @ 2011-12-09 16:00 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: mingo, peterz, fweisbec, David Ahern
perf does not properly handle monitoring of processes with named threads.
For example:
$ ps -C myapp -L
PID LWP TTY TIME CMD
25118 25118 ? 00:00:00 myapp
25118 25119 ? 00:00:00 myapp:worker
perf record -e cs -c 1 -fo /tmp/perf.data -p 25118 -- sleep 10
perf report --stdio -i /tmp/perf.data
100.00% myapp:worker [kernel.kallsyms] [k] perf_event_task_sched_out
The process name is set to the name of the last thread it finds for the
process.
The Problem:
perf-top and perf-record both create a thread_map of threads to be
monitored. That map is used in perf_event__synthesize_thread_map which
loops over the entries in thread_map and calls __event__synthesize_thread
to generate COMM and MMAP events.
__event__synthesize_thread calls perf_event__synthesize_comm which opens
/proc/pid/status, reads the name of the task and its thread group id.
That's all fine. The problem is that it then reads /proc/pid/task and
generates COMM events for each task it finds - but using the name found
in /proc/pid/status where pid is the thread of interest. This means the
name of the last thread processed sets the name for all threads in the
process - which is not good for multithreaded processes with named
threads.
The Fix:
perf_event__synthesize_comm has an input argument (full) that decides
whether to process task entries for each pid it is passed. It currently
never set to 0 (perf_event__synthesize_comm has a single caller and it
always passes the value 1). Let's fix that.
Add the full input argument to __event__synthesize_thread which passes
it to perf_event__synthesize_comm. For thread/process monitoring set full
to 0 which means COMM and MMAP events are only generated for the pid
passed to it. For system wide monitoring set full to 1 so that COMM events
are generated for all threads in a process.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/perf/util/event.c | 35 ++++++++++++++++++++++++++++++-----
1 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 437f8ca..6085727 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -260,10 +260,10 @@ int perf_event__synthesize_modules(perf_event__handler_t process,
static int __event__synthesize_thread(union perf_event *comm_event,
union perf_event *mmap_event,
- pid_t pid, perf_event__handler_t process,
+ pid_t pid, int full, perf_event__handler_t process,
struct perf_session *session)
{
- pid_t tgid = perf_event__synthesize_comm(comm_event, pid, 1, process,
+ pid_t tgid = perf_event__synthesize_comm(comm_event, pid, full, process,
session);
if (tgid == -1)
return -1;
@@ -276,7 +276,7 @@ int perf_event__synthesize_thread_map(struct thread_map *threads,
struct perf_session *session)
{
union perf_event *comm_event, *mmap_event;
- int err = -1, thread;
+ int err = -1, thread, j;
comm_event = malloc(sizeof(comm_event->comm) + session->id_hdr_size);
if (comm_event == NULL)
@@ -289,11 +289,36 @@ int perf_event__synthesize_thread_map(struct thread_map *threads,
err = 0;
for (thread = 0; thread < threads->nr; ++thread) {
if (__event__synthesize_thread(comm_event, mmap_event,
- threads->map[thread],
+ threads->map[thread], 0,
process, session)) {
err = -1;
break;
}
+
+ /*
+ * comm.pid is set to thread group id by
+ * perf_event__synthesize_comm
+ */
+ if ((int) comm_event->comm.pid != threads->map[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]) {
+ need_leader = false;
+ break;
+ }
+ }
+
+ /* if not, generate events for it */
+ if (need_leader &&
+ __event__synthesize_thread(comm_event, mmap_event,
+ comm_event->comm.pid, 0,
+ process, session)) {
+ err = -1;
+ break;
+ }
+ }
}
free(mmap_event);
out_free_comm:
@@ -329,7 +354,7 @@ int perf_event__synthesize_threads(perf_event__handler_t process,
if (*end) /* only interested in proper numerical dirents */
continue;
- __event__synthesize_thread(comm_event, mmap_event, pid,
+ __event__synthesize_thread(comm_event, mmap_event, pid, 1,
process, session);
}
--
1.7.7.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] perf: fix comm for processes with named threads
2011-12-09 16:00 [PATCH] perf: fix comm for processes with named threads David Ahern
@ 2011-12-09 17:08 ` David Ahern
0 siblings, 0 replies; 2+ messages in thread
From: David Ahern @ 2011-12-09 17:08 UTC (permalink / raw)
To: acme; +Cc: linux-kernel, mingo, peterz, fweisbec
Disregard. Need to update to your latest core branch.
On 12/09/2011 09:00 AM, David Ahern wrote:
> perf does not properly handle monitoring of processes with named threads.
> For example:
>
> $ ps -C myapp -L
> PID LWP TTY TIME CMD
> 25118 25118 ? 00:00:00 myapp
> 25118 25119 ? 00:00:00 myapp:worker
>
> perf record -e cs -c 1 -fo /tmp/perf.data -p 25118 -- sleep 10
> perf report --stdio -i /tmp/perf.data
> 100.00% myapp:worker [kernel.kallsyms] [k] perf_event_task_sched_out
>
> The process name is set to the name of the last thread it finds for the
> process.
>
> The Problem:
> perf-top and perf-record both create a thread_map of threads to be
> monitored. That map is used in perf_event__synthesize_thread_map which
> loops over the entries in thread_map and calls __event__synthesize_thread
> to generate COMM and MMAP events.
>
> __event__synthesize_thread calls perf_event__synthesize_comm which opens
> /proc/pid/status, reads the name of the task and its thread group id.
> That's all fine. The problem is that it then reads /proc/pid/task and
> generates COMM events for each task it finds - but using the name found
> in /proc/pid/status where pid is the thread of interest. This means the
> name of the last thread processed sets the name for all threads in the
> process - which is not good for multithreaded processes with named
> threads.
>
> The Fix:
> perf_event__synthesize_comm has an input argument (full) that decides
> whether to process task entries for each pid it is passed. It currently
> never set to 0 (perf_event__synthesize_comm has a single caller and it
> always passes the value 1). Let's fix that.
>
> Add the full input argument to __event__synthesize_thread which passes
> it to perf_event__synthesize_comm. For thread/process monitoring set full
> to 0 which means COMM and MMAP events are only generated for the pid
> passed to it. For system wide monitoring set full to 1 so that COMM events
> are generated for all threads in a process.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> tools/perf/util/event.c | 35 ++++++++++++++++++++++++++++++-----
> 1 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 437f8ca..6085727 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -260,10 +260,10 @@ int perf_event__synthesize_modules(perf_event__handler_t process,
>
> static int __event__synthesize_thread(union perf_event *comm_event,
> union perf_event *mmap_event,
> - pid_t pid, perf_event__handler_t process,
> + pid_t pid, int full, perf_event__handler_t process,
> struct perf_session *session)
> {
> - pid_t tgid = perf_event__synthesize_comm(comm_event, pid, 1, process,
> + pid_t tgid = perf_event__synthesize_comm(comm_event, pid, full, process,
> session);
> if (tgid == -1)
> return -1;
> @@ -276,7 +276,7 @@ int perf_event__synthesize_thread_map(struct thread_map *threads,
> struct perf_session *session)
> {
> union perf_event *comm_event, *mmap_event;
> - int err = -1, thread;
> + int err = -1, thread, j;
>
> comm_event = malloc(sizeof(comm_event->comm) + session->id_hdr_size);
> if (comm_event == NULL)
> @@ -289,11 +289,36 @@ int perf_event__synthesize_thread_map(struct thread_map *threads,
> err = 0;
> for (thread = 0; thread < threads->nr; ++thread) {
> if (__event__synthesize_thread(comm_event, mmap_event,
> - threads->map[thread],
> + threads->map[thread], 0,
> process, session)) {
> err = -1;
> break;
> }
> +
> + /*
> + * comm.pid is set to thread group id by
> + * perf_event__synthesize_comm
> + */
> + if ((int) comm_event->comm.pid != threads->map[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]) {
> + need_leader = false;
> + break;
> + }
> + }
> +
> + /* if not, generate events for it */
> + if (need_leader &&
> + __event__synthesize_thread(comm_event, mmap_event,
> + comm_event->comm.pid, 0,
> + process, session)) {
> + err = -1;
> + break;
> + }
> + }
> }
> free(mmap_event);
> out_free_comm:
> @@ -329,7 +354,7 @@ int perf_event__synthesize_threads(perf_event__handler_t process,
> if (*end) /* only interested in proper numerical dirents */
> continue;
>
> - __event__synthesize_thread(comm_event, mmap_event, pid,
> + __event__synthesize_thread(comm_event, mmap_event, pid, 1,
> process, session);
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-12-09 17:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-09 16:00 [PATCH] perf: fix comm for processes with named threads David Ahern
2011-12-09 17:08 ` David Ahern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).