All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Frederic Weisbecker <fweisbec@gmail.com>
To: linux-tip-commits@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, paulus@samba.org, acme@redhat.com,
	hpa@zytor.com, mingo@redhat.com, efault@gmx.de,
	peterz@infradead.org, fweisbec@gmail.com, tglx@linutronix.de,
	mingo@elte.hu
Subject: [tip:perf/core] perf tools: Fix thread comm resolution in perf sched
Date: Thu, 8 Oct 2009 19:13:05 GMT	[thread overview]
Message-ID: <tip-97ea1a7fa62af0d8d49a0fc12796b0073537c9d8@git.kernel.org> (raw)
In-Reply-To: <1255028657-11158-1-git-send-email-fweisbec@gmail.com>

Commit-ID:  97ea1a7fa62af0d8d49a0fc12796b0073537c9d8
Gitweb:     http://git.kernel.org/tip/97ea1a7fa62af0d8d49a0fc12796b0073537c9d8
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Thu, 8 Oct 2009 21:04:17 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 8 Oct 2009 21:10:21 +0200

perf tools: Fix thread comm resolution in perf sched

This reverts commit 9a92b479b2f088ee2d3194243f4c8e59b1b8c9c2 ("perf
tools: Improve thread comm resolution in perf sched") and fixes the
real bug.

The bug was elsewhere:

We are failing to resolve thread names in perf sched because the
table of threads we are building, on top of comm events, has a per
process granularity. But perf sched, unlike the other perf tools,
needs a per thread granularity as we are profiling every tasks
individually.

So fix it by building our threads table using the tid instead of
the pid as the thread identifier.

v2: Revert the previous fix - it is not really needed

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1255028657-11158-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-sched.c |   46 +++++--------------------------------------
 tools/perf/util/thread.c   |   32 ++++++-----------------------
 tools/perf/util/thread.h   |    3 --
 3 files changed, 13 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 25b91e7..6b00529 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -638,7 +638,7 @@ process_comm_event(event_t *event, unsigned long offset, unsigned long head)
 {
 	struct thread *thread;
 
-	thread = threads__findnew(event->comm.pid, &threads, &last_match);
+	thread = threads__findnew(event->comm.tid, &threads, &last_match);
 
 	dump_printf("%p [%p]: perf_event_comm: %s:%d\n",
 		(void *)(offset + head),
@@ -1034,36 +1034,6 @@ add_sched_in_event(struct work_atoms *atoms, u64 timestamp)
 	atoms->nb_atoms++;
 }
 
-static struct thread *
-threads__findnew_from_ctx(u32 pid, struct trace_switch_event *switch_event)
-{
-	struct thread *th;
-
-	th = threads__findnew_nocomm(pid, &threads, &last_match);
-	if (th->comm)
-		return th;
-
-	if (pid == switch_event->prev_pid)
-		thread__set_comm(th, switch_event->prev_comm);
-	else
-		thread__set_comm(th, switch_event->next_comm);
-	return th;
-}
-
-static struct thread *
-threads__findnew_from_wakeup(struct trace_wakeup_event *wakeup_event)
-{
-	struct thread *th;
-
-	th =  threads__findnew_nocomm(wakeup_event->pid, &threads, &last_match);
-	if (th->comm)
-		return th;
-
-	thread__set_comm(th, wakeup_event->comm);
-
-	return th;
-}
-
 static void
 latency_switch_event(struct trace_switch_event *switch_event,
 		     struct event *event __used,
@@ -1089,10 +1059,8 @@ latency_switch_event(struct trace_switch_event *switch_event,
 		die("hm, delta: %Ld < 0 ?\n", delta);
 
 
-	sched_out = threads__findnew_from_ctx(switch_event->prev_pid,
-					      switch_event);
-	sched_in = threads__findnew_from_ctx(switch_event->next_pid,
-					     switch_event);
+	sched_out = threads__findnew(switch_event->prev_pid, &threads, &last_match);
+	sched_in = threads__findnew(switch_event->next_pid, &threads, &last_match);
 
 	out_events = thread_atoms_search(&atom_root, sched_out, &cmp_pid);
 	if (!out_events) {
@@ -1158,7 +1126,7 @@ latency_wakeup_event(struct trace_wakeup_event *wakeup_event,
 	if (!wakeup_event->success)
 		return;
 
-	wakee = threads__findnew_from_wakeup(wakeup_event);
+	wakee = threads__findnew(wakeup_event->pid, &threads, &last_match);
 	atoms = thread_atoms_search(&atom_root, wakee, &cmp_pid);
 	if (!atoms) {
 		thread_atoms_insert(wakee);
@@ -1418,10 +1386,8 @@ map_switch_event(struct trace_switch_event *switch_event,
 		die("hm, delta: %Ld < 0 ?\n", delta);
 
 
-	sched_out = threads__findnew_from_ctx(switch_event->prev_pid,
-					      switch_event);
-	sched_in = threads__findnew_from_ctx(switch_event->next_pid,
-					     switch_event);
+	sched_out = threads__findnew(switch_event->prev_pid, &threads, &last_match);
+	sched_in = threads__findnew(switch_event->next_pid, &threads, &last_match);
 
 	curr_thread[this_cpu] = sched_in;
 
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 8bd5ca2..3b56aeb 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -6,17 +6,15 @@
 #include "util.h"
 #include "debug.h"
 
-static struct thread *thread__new(pid_t pid, int set_comm)
+static struct thread *thread__new(pid_t pid)
 {
 	struct thread *self = calloc(1, sizeof(*self));
 
 	if (self != NULL) {
 		self->pid = pid;
-		if (set_comm) {
-			self->comm = malloc(32);
-			if (self->comm)
-				snprintf(self->comm, 32, ":%d", self->pid);
-		}
+		self->comm = malloc(32);
+		if (self->comm)
+			snprintf(self->comm, 32, ":%d", self->pid);
 		self->maps = RB_ROOT;
 		INIT_LIST_HEAD(&self->removed_maps);
 	}
@@ -52,10 +50,8 @@ static size_t thread__fprintf(struct thread *self, FILE *fp)
 	return ret;
 }
 
-static struct thread *
-__threads__findnew(pid_t pid, struct rb_root *threads,
-		   struct thread **last_match,
-		   int set_comm)
+struct thread *
+threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match)
 {
 	struct rb_node **p = &threads->rb_node;
 	struct rb_node *parent = NULL;
@@ -84,8 +80,7 @@ __threads__findnew(pid_t pid, struct rb_root *threads,
 			p = &(*p)->rb_right;
 	}
 
-	th = thread__new(pid, set_comm);
-
+	th = thread__new(pid);
 	if (th != NULL) {
 		rb_link_node(&th->rb_node, parent, p);
 		rb_insert_color(&th->rb_node, threads);
@@ -96,19 +91,6 @@ __threads__findnew(pid_t pid, struct rb_root *threads,
 }
 
 struct thread *
-threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match)
-{
-	return __threads__findnew(pid, threads, last_match, 1);
-}
-
-struct thread *
-threads__findnew_nocomm(pid_t pid, struct rb_root *threads,
-			struct thread **last_match)
-{
-	return __threads__findnew(pid, threads, last_match, 0);
-}
-
-struct thread *
 register_idle_thread(struct rb_root *threads, struct thread **last_match)
 {
 	struct thread *thread = threads__findnew(0, threads, last_match);
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 75bc843..845d9b6 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -18,9 +18,6 @@ int thread__set_comm(struct thread *self, const char *comm);
 struct thread *
 threads__findnew(pid_t pid, struct rb_root *threads, struct thread **last_match);
 struct thread *
-threads__findnew_nocomm(pid_t pid, struct rb_root *threads,
-			struct thread **last_match);
-struct thread *
 register_idle_thread(struct rb_root *threads, struct thread **last_match);
 void thread__insert_map(struct thread *self, struct map *map);
 int thread__fork(struct thread *self, struct thread *parent);

      reply	other threads:[~2009-10-08 19:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-08 14:37 [PATCH] perf tools: Improve thread comm resolution in perf sched Frederic Weisbecker
2009-10-08 15:12 ` [tip:perf/core] " tip-bot for Frederic Weisbecker
2009-10-08 16:33 ` [PATCH] " Peter Zijlstra
2009-10-08 17:18   ` Frederic Weisbecker
2009-10-08 18:34     ` Frederic Weisbecker
2009-10-08 18:48   ` [PATCH] perf tools: Fix " Frederic Weisbecker
2009-10-08 19:04     ` Frederic Weisbecker
2009-10-08 19:13       ` tip-bot for Frederic Weisbecker [this message]

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=tip-97ea1a7fa62af0d8d49a0fc12796b0073537c9d8@git.kernel.org \
    --to=fweisbec@gmail.com \
    --cc=acme@redhat.com \
    --cc=efault@gmx.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.