From: Arnaldo Carvalho de Melo <acme@infradead.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: linux-kernel@vger.kernel.org, David Ahern <dsahern@gmail.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 02/15] perf tools: Fix comm for processes with named threads
Date: Fri, 23 Dec 2011 19:53:05 -0200 [thread overview]
Message-ID: <1324677198-5191-3-git-send-email-acme@infradead.org> (raw)
In-Reply-To: <1324677198-5191-1-git-send-email-acme@infradead.org>
From: David Ahern <dsahern@gmail.com>
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.
The end result (looping over thread_map + synthesizing comm events for
each thread each time) 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.
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1324578603-12762-2-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/event.c | 37 ++++++++++++++++++++++++++++++++-----
1 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index b7c7f39..a578726 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -261,11 +261,12 @@ int perf_event__synthesize_modules(struct perf_tool *tool,
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_tool *tool,
struct machine *machine)
{
- pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, 1,
+ pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, full,
process, machine);
if (tgid == -1)
return -1;
@@ -279,7 +280,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
struct machine *machine)
{
union perf_event *comm_event, *mmap_event;
- int err = -1, thread;
+ int err = -1, thread, j;
comm_event = malloc(sizeof(comm_event->comm) + machine->id_hdr_size);
if (comm_event == NULL)
@@ -292,11 +293,37 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
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, tool, machine)) {
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, tool, machine)) {
+ err = -1;
+ break;
+ }
+ }
}
free(mmap_event);
out_free_comm:
@@ -333,7 +360,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
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, tool, machine);
}
--
1.7.8.rc0.35.gee6df
next prev parent reply other threads:[~2011-12-23 21:56 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-23 21:53 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 01/15] perf report: Fix usage string Arnaldo Carvalho de Melo
2011-12-23 21:53 ` Arnaldo Carvalho de Melo [this message]
2011-12-23 21:53 ` [PATCH 03/15] perf tools: Look up thread names for system wide profiling Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 04/15] perf script: look up thread using tid instead of pid Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 05/15] perf tools: Fix truncated annotation Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 06/15] perf: builtin-record: Provide advice if mmap'ing fails with EPERM Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 07/15] perf: builtin-record: Document and check that mmap_pages must be a power of two Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 08/15] perf tools: Improve macros for struct feature_ops Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 09/15] perf tools: Continue processing header on unknown features Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 10/15] perf tools: Fix out-of-bound access to struct perf_session Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 11/15] perf tools: Moving code in some files Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 12/15] perf report: Accept fifos as input file Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 13/15] perf tools: Unify handling of features when writing feature section Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 14/15] perf tools: Use for_each_set_bit() to iterate over feature flags Arnaldo Carvalho de Melo
2011-12-23 21:53 ` [PATCH 15/15] perf script: Add generic perl handler to process events Arnaldo Carvalho de Melo
2011-12-29 20:28 ` [GIT PULL 00/15] perf/core improvements and fixes Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1324677198-5191-3-git-send-email-acme@infradead.org \
--to=acme@infradead.org \
--cc=acme@redhat.com \
--cc=dsahern@gmail.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).