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

[Dongsheng Yang]

> 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@xxxxxxxxx>
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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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