[PATCH 0/3] perf: working with multithreaded processes with named threads

[PATCH 0/3] perf: working with multithreaded processes with named threads

[David Ahern]

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(-)

-- 
1.7.7.3

[PATCH 1/3] perf: fix comm for processes with named threads

[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

[PATCH 2/3] perf: look up thread names for system wide profiling

[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

[PATCH 3/3] perf script: look up thread using tid instead of pid

[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

Re: [PATCH 0/3] perf: working with multithreaded processes with named threads

[David Ahern]

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

[PATCH 0/3] perf: working with multithreaded processes with named threads

[David Ahern]

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     |  112 ++++++++++++++++++++++++++++++++----------
 2 files changed, 86 insertions(+), 28 deletions(-)

-- 
1.7.7.4