public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
To: David Ahern <dsahern@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf tools: Fix bug for perf kvm report without guestmount.
Date: Tue, 10 Dec 2013 13:54:54 -0500	[thread overview]
Message-ID: <52A7637E.8080007@cn.fujitsu.com> (raw)
In-Reply-To: <52A61D43.6020007@gmail.com>


> 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/
>


  reply	other threads:[~2013-12-10  5:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06 21:33 [PATCH] perf tools: Fix bug for perf kvm report without guestmount Dongsheng Yang
2013-12-06 11:56 ` Arnaldo Carvalho de Melo
2013-12-08 17:39   ` David Ahern
2013-12-09 18:07     ` Arnaldo Carvalho de Melo
2013-12-09 18:08       ` David Ahern
2013-12-09 18:49         ` Arnaldo Carvalho de Melo
2013-12-09 19:42           ` David Ahern
2013-12-10 18:54             ` Dongsheng Yang [this message]
2013-12-10  6:02               ` David Ahern
2013-12-10 19:07                 ` Dongsheng Yang
2013-12-16 16:26               ` [PATCH V2] " Dongsheng Yang
2013-12-16 16:36                 ` Dongsheng Yang
2013-12-20  5:31                 ` David Ahern
2013-12-20 18:37                   ` Dongsheng Yang
2013-12-20 18:41                     ` [PATCH V3] " Dongsheng Yang
2013-12-20  5:46                       ` David Ahern
2014-01-12 18:36                       ` [tip:perf/core] perf kvm: Fix " tip-bot for Dongsheng Yang
2013-12-09 15:20   ` [PATCH] perf tools: Fix bug for perf " Dongsheng Yang
2013-12-09  3:42     ` David Ahern
2013-12-09 17:12       ` Dongsheng Yang
2013-12-09  4:32         ` David Ahern
2013-12-09 18:06           ` Dongsheng Yang
2013-12-09 18:41             ` Dongsheng Yang
2013-12-09 18:44               ` Dongsheng Yang

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=52A7637E.8080007@cn.fujitsu.com \
    --to=yangds.fnst@cn.fujitsu.com \
    --cc=acme@ghostprotocols.net \
    --cc=dsahern@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /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