Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

perf tip: fails to convert comm

2 views
Skip to first unread message

David Ahern

unread,
Nov 13, 2013, 12:00:02 AM11/13/13
to
Hi Namhyung and Frederic:

If you recall I mentioned noting a problem with the callchain series
showing comm's. Well, it fails on acme's perf/core. git bisect points to:

$ git bisect bad
4dfced359fbc719a35527416f1b4b3999647f68b is the first bad commit
commit 4dfced359fbc719a35527416f1b4b3999647f68b
Author: Namhyung Kim <namhyu...@lge.com>
Date: Fri Sep 13 16:28:57 2013 +0900

perf tools: Get current comm instead of last one

At insert time, a hist entry should reference comm at the time
otherwise
it'll get the last comm anyway.


How to re-create:

Start point is tools/perf directory for 3.12 (Linus tree):
$ perf sched record -o /tmp/perf.data -g -- make -j 16
$ perf script -i /tmp/perf.data > /tmp/1

cd to Arnaldo's tree, make perf and use it to create /tmp/2:
$ perf script -i /tmp/perf.data > /tmp/1
$ diff -U3 /tmp/1 /tmp/2 | less

You'll see a number of comm's showing as :<pid> instead of make, etc.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Arnaldo Carvalho de Melo

unread,
Nov 13, 2013, 1:10:02 PM11/13/13
to
Em Wed, Nov 13, 2013 at 07:03:47PM +0100, Frederic Weisbecker escreveu:
> On Tue, Nov 12, 2013 at 09:58:29PM -0700, David Ahern wrote:
> > Hi Namhyung and Frederic:
> >
> > If you recall I mentioned noting a problem with the callchain series
> > showing comm's. Well, it fails on acme's perf/core. git bisect
> > points to:
> >
> > $ git bisect bad
> > 4dfced359fbc719a35527416f1b4b3999647f68b is the first bad commit
> > commit 4dfced359fbc719a35527416f1b4b3999647f68b
> > Author: Namhyung Kim <namhyu...@lge.com>
> > Date: Fri Sep 13 16:28:57 2013 +0900
> >
> > perf tools: Get current comm instead of last one
> >
> > At insert time, a hist entry should reference comm at the time
> > otherwise
> > it'll get the last comm anyway.
> >
> >
> > How to re-create:
> >
> > Start point is tools/perf directory for 3.12 (Linus tree):
> > $ perf sched record -o /tmp/perf.data -g -- make -j 16
> > $ perf script -i /tmp/perf.data > /tmp/1
> >
> > cd to Arnaldo's tree, make perf and use it to create /tmp/2:
> > $ perf script -i /tmp/perf.data > /tmp/1
> > $ diff -U3 /tmp/1 /tmp/2 | less
> >
> > You'll see a number of comm's showing as :<pid> instead of make, etc.
>
> I see. I can reproduce, I'll check and see what happens. It would be nice if
> we could have an option to dump internal perf events like comm events as well
> in the perf script stream.

'perf record -D' not enough?

- Arnaldo

Frederic Weisbecker

unread,
Nov 13, 2013, 1:10:03 PM11/13/13
to
On Tue, Nov 12, 2013 at 09:58:29PM -0700, David Ahern wrote:
> Hi Namhyung and Frederic:
>
> If you recall I mentioned noting a problem with the callchain series
> showing comm's. Well, it fails on acme's perf/core. git bisect
> points to:
>
> $ git bisect bad
> 4dfced359fbc719a35527416f1b4b3999647f68b is the first bad commit
> commit 4dfced359fbc719a35527416f1b4b3999647f68b
> Author: Namhyung Kim <namhyu...@lge.com>
> Date: Fri Sep 13 16:28:57 2013 +0900
>
> perf tools: Get current comm instead of last one
>
> At insert time, a hist entry should reference comm at the time
> otherwise
> it'll get the last comm anyway.
>
>
> How to re-create:
>
> Start point is tools/perf directory for 3.12 (Linus tree):
> $ perf sched record -o /tmp/perf.data -g -- make -j 16
> $ perf script -i /tmp/perf.data > /tmp/1
>
> cd to Arnaldo's tree, make perf and use it to create /tmp/2:
> $ perf script -i /tmp/perf.data > /tmp/1
> $ diff -U3 /tmp/1 /tmp/2 | less
>
> You'll see a number of comm's showing as :<pid> instead of make, etc.

I see. I can reproduce, I'll check and see what happens. It would be nice if
we could have an option to dump internal perf events like comm events as well
in the perf script stream.

David Ahern

unread,
Nov 13, 2013, 1:10:03 PM11/13/13
to
On 11/13/13, 11:03 AM, Frederic Weisbecker wrote:
> I see. I can reproduce, I'll check and see what happens. It would be nice if
> we could have an option to dump internal perf events like comm events as well
> in the perf script stream.

I've been thinking about moving the event dumping code from perf-script
to perf-dump. In the process could add an argument to dump events --
similar to -D option for report/script, but more inline with the output
that perf-script shows now.

David

Frederic Weisbecker

unread,
Nov 13, 2013, 1:40:02 PM11/13/13
to
On Wed, Nov 13, 2013 at 11:06:11AM -0700, David Ahern wrote:
> On 11/13/13, 11:03 AM, Frederic Weisbecker wrote:
> >I see. I can reproduce, I'll check and see what happens. It would be nice if
> >we could have an option to dump internal perf events like comm events as well
> >in the perf script stream.
>
> I've been thinking about moving the event dumping code from
> perf-script to perf-dump. In the process could add an argument to
> dump events -- similar to -D option for report/script, but more
> inline with the output that perf-script shows now.

Yeah indeed,sounds good!

Frederic Weisbecker

unread,
Nov 13, 2013, 1:40:02 PM11/13/13
to
No because it's not easy to correlate with perf script event output. Although it could
be if we simply dump the time the same way in both.

David Ahern

unread,
Nov 15, 2013, 11:40:01 AM11/15/13
to
HI Frederic:

On 11/13/13, 11:03 AM, Frederic Weisbecker wrote:
>
> I see. I can reproduce, I'll check and see what happens. It would be nice if
> we could have an option to dump internal perf events like comm events as well
> in the perf script stream.

Any progress on a solution? This is a regression in 3.13.

David

Frederic Weisbecker

unread,
Nov 15, 2013, 8:10:02 PM11/15/13
to
On Fri, Nov 15, 2013 at 09:29:51AM -0700, David Ahern wrote:
> HI Frederic:
>
> On 11/13/13, 11:03 AM, Frederic Weisbecker wrote:
> >
> >I see. I can reproduce, I'll check and see what happens. It would be nice if
> >we could have an option to dump internal perf events like comm events as well
> >in the perf script stream.
>
> Any progress on a solution? This is a regression in 3.13.

So the problem is that when a thread overrides its default ":%pid" comm, we forget
to tag the thread comm as overriden. Hence, this overriden comm is not inherited on
future forks.

So here is a fix. Tell me if you see more issue, I'll cook a proper changelog and
resend if everyting looks good.

Thanks.

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index cd8e2f5..49eaf1d 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -70,14 +70,13 @@ int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
/* Override latest entry if it had no specific time coverage */
if (!curr->start) {
comm__override(curr, str, timestamp);
- return 0;
+ } else {
+ new = comm__new(str, timestamp);
+ if (!new)
+ return -ENOMEM;
+ list_add(&new->list, &thread->comm_list);
}

- new = comm__new(str, timestamp);
- if (!new)
- return -ENOMEM;
-
- list_add(&new->list, &thread->comm_list);
thread->comm_set = true;

return 0;

Namhyung Kim

unread,
Nov 16, 2013, 7:00:02 AM11/16/13
to
Hi Frederic,

2013-11-16 (토), 02:02 +0100, Frederic Weisbecker:
Looks good to me.

Thanks for the fix.
Namhyung

David Ahern

unread,
Nov 16, 2013, 10:20:01 AM11/16/13
to
On 11/15/13, 6:02 PM, Frederic Weisbecker wrote:
> On Fri, Nov 15, 2013 at 09:29:51AM -0700, David Ahern wrote:
>> HI Frederic:
>>
>> On 11/13/13, 11:03 AM, Frederic Weisbecker wrote:
>>>
>>> I see. I can reproduce, I'll check and see what happens. It would be nice if
>>> we could have an option to dump internal perf events like comm events as well
>>> in the perf script stream.
>>
>> Any progress on a solution? This is a regression in 3.13.
>
> So the problem is that when a thread overrides its default ":%pid" comm, we forget
> to tag the thread comm as overriden. Hence, this overriden comm is not inherited on
> future forks.
>
> So here is a fix. Tell me if you see more issue, I'll cook a proper changelog and
> resend if everyting looks good.

That works for me. Thank you.

Tested-by: David Ahern <dsa...@gmail.com>

tip-bot for Frederic Weisbecker

unread,
Nov 20, 2013, 9:00:03 AM11/20/13
to
Commit-ID: a5285ad9e30fd90b88a11adcab97bd4c3ffe44eb
Gitweb: http://git.kernel.org/tip/a5285ad9e30fd90b88a11adcab97bd4c3ffe44eb
Author: Frederic Weisbecker <fwei...@gmail.com>
AuthorDate: Sat, 16 Nov 2013 02:02:09 +0100
Committer: Arnaldo Carvalho de Melo <ac...@redhat.com>
CommitDate: Tue, 19 Nov 2013 10:33:29 -0300

perf tools: Tag thread comm as overriden

The problem is that when a thread overrides its default ":%pid" comm, we
forget to tag the thread comm as overriden. Hence, this overriden comm
is not inherited on future forks. Fix it.

Signed-off-by: Frederic Weisbecker <fwei...@gmail.com>
Tested-by: David Ahern <dsa...@gmail.com>
Acked-by: Namhyung Kim <namh...@kernel.org>
Cc: David Ahern <dsa...@gmail.com>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Namhyung Kim <namh...@kernel.org>
Link: http://lkml.kernel.org/r/20131116010...@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/util/thread.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
0 new messages