mirror of https://lore.kernel.org/lkml/
 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 " 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome