* [PATCH 1/4] perf: Process comm events by tid
2010-05-31 23:02 [GIT PULL] perf fixes Frederic Weisbecker
@ 2010-05-31 23:02 ` Frederic Weisbecker
2010-05-31 23:02 ` [PATCH 2/4] perf: Use event__process_task from perf sched Frederic Weisbecker
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Frederic Weisbecker @ 2010-05-31 23:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
Arnaldo Carvalho de Melo, Paul Mackerras, Tom Zanussi,
Stephane Eranian
When we synthetize the existing running tasks though procfs,
we walk through every threads of a process, queuing one comm
events per tid.
But then on report time, event__process_comm() only creates and
sets the comm on a per process granularity. This is the right
thing for comm events that came from the kernel, as they are
only created on exec. Sub-threads then inherit their comm
from fork events. But that doesn't work with our synthetized
comm events taken from procfs informations as the per thread
granularity is done on comm events directly there.
Hence we need event__process_comm() to work with the tid rather
than the pid. It won't change anything for comm events coming
from the kernel but this will fix the synthetized ones.
Before:
$ ./perf report -D | grep COMM | grep firefox
0x2c7b8 [0x18]: PERF_RECORD_COMM: firefox:5297
0x2c7d0 [0x18]: PERF_RECORD_COMM: firefox:5297
0x2c7e8 [0x18]: PERF_RECORD_COMM: firefox:5297
0x2c800 [0x18]: PERF_RECORD_COMM: firefox:5297
0x2c818 [0x18]: PERF_RECORD_COMM: firefox:5297
0x2c830 [0x18]: PERF_RECORD_COMM: firefox:5297
After:
$ ./perf report -D | grep COMM | grep firefox
0x2c7b8 [0x18]: PERF_RECORD_COMM: firefox:5297
0x2c7d0 [0x18]: PERF_RECORD_COMM: firefox:5299
0x2c7e8 [0x18]: PERF_RECORD_COMM: firefox:5300
0x2c800 [0x18]: PERF_RECORD_COMM: firefox:5308
0x2c818 [0x18]: PERF_RECORD_COMM: firefox:5309
0x2c830 [0x18]: PERF_RECORD_COMM: firefox:5312
This fixes various unresolved pid on perf sched.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
---
tools/perf/util/event.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 50771b5..d28d809 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -370,9 +370,9 @@ static int thread__set_comm_adjust(struct thread *self, const char *comm)
int event__process_comm(event_t *self, struct perf_session *session)
{
- struct thread *thread = perf_session__findnew(session, self->comm.pid);
+ struct thread *thread = perf_session__findnew(session, self->comm.tid);
- dump_printf(": %s:%d\n", self->comm.comm, self->comm.pid);
+ dump_printf(": %s:%d\n", self->comm.comm, self->comm.tid);
if (thread == NULL || thread__set_comm_adjust(thread, self->comm.comm)) {
dump_printf("problem processing PERF_RECORD_COMM, skipping event.\n");
--
1.6.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] perf: Use event__process_task from perf sched
2010-05-31 23:02 [GIT PULL] perf fixes Frederic Weisbecker
2010-05-31 23:02 ` [PATCH 1/4] perf: Process comm events by tid Frederic Weisbecker
@ 2010-05-31 23:02 ` Frederic Weisbecker
2010-05-31 23:02 ` [PATCH 3/4] perf: Do the comm inheritance per thread in event__process_task Frederic Weisbecker
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Frederic Weisbecker @ 2010-05-31 23:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
Arnaldo Carvalho de Melo, Paul Mackerras, Tom Zanussi,
Stephane Eranian
perf sched uses event__process_comm(), which means it can resolve
comms from:
- tasks that have exec'ed (kernel comm events)
- tasks that were running when perf record started the actual
recording (synthetized comm events)
But perf sched can't resolve the pids of tasks that were created
after the recording started.
To solve this, we need to inherit the comms on fork events using
event__process_task().
This fixes various unresolved pids in perf sched, easily visible
with:
perf sched record perf bench sched messaging
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
---
tools/perf/builtin-sched.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index f67bce2..55f3b5d 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1645,6 +1645,7 @@ static struct perf_event_ops event_ops = {
.sample = process_sample_event,
.comm = event__process_comm,
.lost = event__process_lost,
+ .fork = event__process_task,
.ordered_samples = true,
};
--
1.6.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] perf: Do the comm inheritance per thread in event__process_task
2010-05-31 23:02 [GIT PULL] perf fixes Frederic Weisbecker
2010-05-31 23:02 ` [PATCH 1/4] perf: Process comm events by tid Frederic Weisbecker
2010-05-31 23:02 ` [PATCH 2/4] perf: Use event__process_task from perf sched Frederic Weisbecker
@ 2010-05-31 23:02 ` Frederic Weisbecker
2010-05-31 23:02 ` [PATCH 4/4] perf-record: Check correct pid when forking Frederic Weisbecker
2010-06-01 6:59 ` [GIT PULL] perf fixes Ingo Molnar
4 siblings, 0 replies; 6+ messages in thread
From: Frederic Weisbecker @ 2010-05-31 23:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
Arnaldo Carvalho de Melo, Paul Mackerras, Tom Zanussi,
Stephane Eranian
event__process_task() doesn't propagate the comm copy on clone,
but only on process fork. So we loose all the tid:comm resolution
for tasks that aren't a main process thread.
Progragate the per thread granularity to event__process_task for
pid resolution.
This fixes various unresolved pids in perf sched, especially when
we trace multithread processes. The problem is quickly reproducible
with the messaging benchmark using the multithread mode "-t" :
perf sched record perf bench sched messaging -t
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
---
tools/perf/util/event.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index d28d809..1f08f00 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -532,16 +532,11 @@ out_problem:
int event__process_task(event_t *self, struct perf_session *session)
{
- struct thread *thread = perf_session__findnew(session, self->fork.pid);
- struct thread *parent = perf_session__findnew(session, self->fork.ppid);
+ struct thread *thread = perf_session__findnew(session, self->fork.tid);
+ struct thread *parent = perf_session__findnew(session, self->fork.ptid);
dump_printf("(%d:%d):(%d:%d)\n", self->fork.pid, self->fork.tid,
self->fork.ppid, self->fork.ptid);
- /*
- * A thread clone will have the same PID for both parent and child.
- */
- if (thread == parent)
- return 0;
if (self->header.type == PERF_RECORD_EXIT)
return 0;
--
1.6.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] perf-record: Check correct pid when forking
2010-05-31 23:02 [GIT PULL] perf fixes Frederic Weisbecker
` (2 preceding siblings ...)
2010-05-31 23:02 ` [PATCH 3/4] perf: Do the comm inheritance per thread in event__process_task Frederic Weisbecker
@ 2010-05-31 23:02 ` Frederic Weisbecker
2010-06-01 6:59 ` [GIT PULL] perf fixes Ingo Molnar
4 siblings, 0 replies; 6+ messages in thread
From: Frederic Weisbecker @ 2010-05-31 23:02 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Borislav Petkov, Ingo Molnar, Peter Zijlstra,
Arnaldo Carvalho de Melo, Paul Mackerras, Tom Zanussi,
Stephane Eranian, Frederic Weisbecker
From: Borislav Petkov <bp@alien8.de>
When forking the child to be traced, we should check the correct
return value from fork() and not a local variable which is otherwise
unused.
Signed-off-by: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <20100531211818.GA30175@liondog.tnic>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
tools/perf/builtin-record.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 9bc8905..dc3435e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -503,7 +503,6 @@ static int __cmd_record(int argc, const char **argv)
{
int i, counter;
struct stat st;
- pid_t pid = 0;
int flags;
int err;
unsigned long waking = 0;
@@ -572,7 +571,7 @@ static int __cmd_record(int argc, const char **argv)
if (forks) {
child_pid = fork();
- if (pid < 0) {
+ if (child_pid < 0) {
perror("failed to fork");
exit(-1);
}
--
1.6.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [GIT PULL] perf fixes
2010-05-31 23:02 [GIT PULL] perf fixes Frederic Weisbecker
` (3 preceding siblings ...)
2010-05-31 23:02 ` [PATCH 4/4] perf-record: Check correct pid when forking Frederic Weisbecker
@ 2010-06-01 6:59 ` Ingo Molnar
4 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2010-06-01 6:59 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: LKML, Borislav Petkov, Peter Zijlstra, Arnaldo Carvalho de Melo,
Paul Mackerras, Thomas Gleixner
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> Ingo,
>
> Please pull the perf/urgent branch that can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> perf/urgent
>
> Thanks,
> Frederic
> ---
>
> Frederic Weisbecker (3):
> perf: Process comm events by tid
> perf: Use event__process_task from perf sched
> perf: Do the comm inheritance per thread in event__process_task
>
> Borislav Petkov (1):
> perf-record: Check correct pid when forking
>
>
> tools/perf/builtin-record.c | 3 +--
> tools/perf/builtin-sched.c | 1 +
> tools/perf/util/event.c | 13 ++++---------
> 3 files changed, 6 insertions(+), 11 deletions(-)
Pulled, thanks a lot Frederic!
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread