Re: [PATCH] perf tools: Fix bug for perf kvm report without guestmount.

Dongsheng Yang
2013-12-10T05:56:53Z

> NACK. Not needed and breaks the very intent of those 2 options.
>
> David
> --

Okey, David, I saw your commit 207b57926 (perf kvm: Fix regression with
guest machine creation) here. It is applied to fix a bug of SEGFAULT. I
have to say that it changed the behavior of report and lead to current bug.

commit 207b5792696206663a38e525b9793644895bad3b
Author: David Ahern <dsahern@gmail.com>
Date: Sun Jul 1 16:11:37 2012 -0600

perf kvm: Fix regression with guest machine creation

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index c3e399b..56142d0 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -926,7 +926,7 @@ static struct machine *
else
pid = event->ip.pid;

- return perf_session__find_machine(session, pid);
+ return perf_session__findnew_machine(session, pid);
}

return perf_session__find_host_machine(session);

In original, it uses perf_session__find_machine(), it means we deliver
symbol to machine which has the same pid, if no machine found, deliver
it to *default* guest. But if we use perf_session__findnew_machine()
here, if no machine was found, new machine with pid will be built and
added. Then the default guest which with pid == 0 will never get a
symbol, right?

And because the new machine initialized here has no kernel map created,
the symbol delivered to it will be marked as "unknown".

So, my patch here is to revert commit 207b57926 and fix the SEGFAULT bug
in another way.

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8a7da6f..1770f2f 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -851,6 +851,7 @@ static struct machine *
struct perf_sample *sample)
{
const u8 cpumode = event->header.misc &
PERF_RECORD_MISC_CPUMODE_MASK;
+ struct machine *machine;

if (perf_guest &&
((cpumode == PERF_RECORD_MISC_GUEST_KERNEL) ||
@@ -863,7 +864,11 @@ static struct machine *
else
pid = sample->pid;

- return perf_session__findnew_machine(session, pid);
+ machine = perf_session__find_machine(session, pid);
+ if (!machine)
+ machine = perf_session__findnew_machine(session,
+ DEFAULT_GUEST_KERNEL_ID);
+ return machine;
}

return &session->machines.host;

If there is a machine for this event or sample, return this machine. If
no, return *default* guest.

About the SEGFAULT bug you mentioned, it is solved by if (!machine).

Please correct me if I am wrong, thanx !!

- Yang

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