* [PATCH 1/3] perf: fix comm for processes with named threads
2011-12-09 17:43 [PATCH 0/3] perf: working with multithreaded processes with named threads David Ahern
@ 2011-12-09 17:43 ` David Ahern
2011-12-09 17:43 ` [PATCH 2/3] perf: look up thread names for system wide profiling David Ahern
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2011-12-09 17:43 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.
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.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/perf/util/event.c | 36 +++++++++++++++++++++++++++++++-----
1 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 97c479b..c424324 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,36 @@ 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 +359,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.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] perf: look up thread names for system wide profiling
2011-12-09 17:43 [PATCH 0/3] perf: working with multithreaded processes with named threads David Ahern
2011-12-09 17:43 ` [PATCH 1/3] perf: fix comm for " David Ahern
@ 2011-12-09 17:43 ` David Ahern
2011-12-09 17:43 ` [PATCH 3/3] perf script: look up thread using tid instead of pid David Ahern
2011-12-16 23:50 ` [PATCH 0/3] perf: working with multithreaded processes with named threads David Ahern
3 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2011-12-09 17:43 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: mingo, peterz, fweisbec, David Ahern
This handles multithreaded processes with named threads when
doing system wide profiling: the comm for each thread is
looked up allowing them to be different from the thread
group leader.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/perf/util/event.c | 75 +++++++++++++++++++++++++++++++++--------------
1 files changed, 53 insertions(+), 22 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index c424324..a886bfc 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -43,37 +43,27 @@ static struct perf_sample synth_sample = {
.period = 1,
};
-static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
- union perf_event *event, pid_t pid,
- int full, perf_event__handler_t process,
- struct machine *machine)
+static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
{
char filename[PATH_MAX];
char bf[BUFSIZ];
FILE *fp;
size_t size = 0;
- DIR *tasks;
- struct dirent dirent, *next;
- pid_t tgid = 0;
+ pid_t tgid = -1;
snprintf(filename, sizeof(filename), "/proc/%d/status", pid);
fp = fopen(filename, "r");
if (fp == NULL) {
-out_race:
- /*
- * We raced with a task exiting - just return:
- */
pr_debug("couldn't open %s\n", filename);
return 0;
}
- memset(&event->comm, 0, sizeof(event->comm));
-
- while (!event->comm.comm[0] || !event->comm.pid) {
+ while (!comm[0] || (tgid < 0)) {
if (fgets(bf, sizeof(bf), fp) == NULL) {
- pr_warning("couldn't get COMM and pgid, malformed %s\n", filename);
- goto out;
+ pr_warning("couldn't get COMM and pgid, malformed %s\n",
+ filename);
+ break;
}
if (memcmp(bf, "Name:", 5) == 0) {
@@ -81,16 +71,46 @@ out_race:
while (*name && isspace(*name))
++name;
size = strlen(name) - 1;
- memcpy(event->comm.comm, name, size++);
+ if (size >= len)
+ size = len - 1;
+ memcpy(comm, name, size);
+
} else if (memcmp(bf, "Tgid:", 5) == 0) {
char *tgids = bf + 5;
while (*tgids && isspace(*tgids))
++tgids;
- tgid = event->comm.pid = atoi(tgids);
+ tgid = atoi(tgids);
}
}
+ fclose(fp);
+
+ return tgid;
+}
+
+static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
+ union perf_event *event, pid_t pid,
+ int full,
+ perf_event__handler_t process,
+ struct machine *machine)
+{
+ char filename[PATH_MAX];
+ size_t size;
+ DIR *tasks;
+ struct dirent dirent, *next;
+ pid_t tgid;
+
+ memset(&event->comm, 0, sizeof(event->comm));
+
+ tgid = perf_event__get_comm_tgid(pid, event->comm.comm,
+ sizeof(event->comm.comm));
+ if (tgid < 0)
+ goto out;
+
+ event->comm.pid = tgid;
event->comm.header.type = PERF_RECORD_COMM;
+
+ size = strlen(event->comm.comm) + 1;
size = ALIGN(size, sizeof(u64));
memset(event->comm.comm + size, 0, machine->id_hdr_size);
event->comm.header.size = (sizeof(event->comm) -
@@ -106,8 +126,10 @@ out_race:
snprintf(filename, sizeof(filename), "/proc/%d/task", pid);
tasks = opendir(filename);
- if (tasks == NULL)
- goto out_race;
+ if (tasks == NULL) {
+ pr_debug("couldn't open %s\n", filename);
+ return 0;
+ }
while (!readdir_r(tasks, &dirent, &next) && next) {
char *end;
@@ -115,6 +137,17 @@ out_race:
if (*end)
continue;
+ /* already have tgid; jut want to update the comm */
+ (void) perf_event__get_comm_tgid(pid, event->comm.comm,
+ sizeof(event->comm));
+
+ size = strlen(event->comm.comm) + 1;
+ size = ALIGN(size, sizeof(u64));
+ memset(event->comm.comm + size, 0, machine->id_hdr_size);
+ event->comm.header.size = (sizeof(event->comm) -
+ (sizeof(event->comm.comm) - size) +
+ machine->id_hdr_size);
+
event->comm.tid = pid;
process(tool, event, &synth_sample, machine);
@@ -122,8 +155,6 @@ out_race:
closedir(tasks);
out:
- fclose(fp);
-
return tgid;
}
--
1.7.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] perf script: look up thread using tid instead of pid
2011-12-09 17:43 [PATCH 0/3] perf: working with multithreaded processes with named threads David Ahern
2011-12-09 17:43 ` [PATCH 1/3] perf: fix comm for " David Ahern
2011-12-09 17:43 ` [PATCH 2/3] perf: look up thread names for system wide profiling David Ahern
@ 2011-12-09 17:43 ` David Ahern
2011-12-16 23:50 ` [PATCH 0/3] perf: working with multithreaded processes with named threads David Ahern
3 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2011-12-09 17:43 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: mingo, peterz, fweisbec, David Ahern
This allows the thread name to be dispalyed when dumping
events:
myapp 25118 [000] 450385.538815: context-switches ...
myapp:worker 25119 [000] 450385.538894: context-switches ...
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/perf/builtin-script.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index ea71c5e..d71b745 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -443,7 +443,7 @@ static int process_sample_event(struct perf_tool *tool __used,
struct machine *machine)
{
struct addr_location al;
- struct thread *thread = machine__findnew_thread(machine, event->ip.pid);
+ struct thread *thread = machine__findnew_thread(machine, event->ip.tid);
if (thread == NULL) {
pr_debug("problem processing %d event, skipping it.\n",
--
1.7.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] perf: working with multithreaded processes with named threads
2011-12-09 17:43 [PATCH 0/3] perf: working with multithreaded processes with named threads David Ahern
` (2 preceding siblings ...)
2011-12-09 17:43 ` [PATCH 3/3] perf script: look up thread using tid instead of pid David Ahern
@ 2011-12-16 23:50 ` David Ahern
3 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2011-12-16 23:50 UTC (permalink / raw)
To: acme; +Cc: linux-kernel, mingo, peterz, fweisbec
On 12/09/2011 10:43 AM, David Ahern wrote:
> One bug fix plus a couple of enhancements to improve handling of
> named threads.
>
> David Ahern (3):
> perf: fix comm for processes with named threads
> perf: look up thread names for system wide profiling
> perf script: look up thread using tid instead of pid
>
> tools/perf/builtin-script.c | 2 +-
> tools/perf/util/event.c | 111 ++++++++++++++++++++++++++++++++----------
> 2 files changed, 85 insertions(+), 28 deletions(-)
>
Any comments?
David
^ permalink raw reply [flat|nested] 6+ messages in thread