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

Dongsheng Yang
2013-12-16T03:38:13Z

David,
Could you please help to review this patch when you have a time ?

Thanx in advance

- Yang

On 12/16/2013 11:26 AM, Dongsheng Yang wrote:
> Currently, if we use perf kvm --guestkallsyms --guestmodules report,
> we can not get the perf information from perf data file. The all sample
> are shown as unknown.
>
> Reproducing steps:
> # perf kvm --guestkallsyms /tmp/kallsyms --guestmodules /tmp/modules record -a sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.624 MB perf.data.guest (~27260 samples) ]
> # perf kvm --guestkallsyms /tmp/kallsyms --guestmodules /tmp/modules report |grep %
> 100.00% [guest/6471] [unknown] [g] 0xffffffff8164f330
>
> This bug was introduced by 207b57926 (perf kvm: Fix regression with guest machine creation).
>
> 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 code, 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.
>
> And because the new machine initialized here has no kernel map created, the symbol delivered to
> it will be marked as "unknown".
>
> This patch here is to revert commit 207b57926 and fix the SEGFAULT bug in another way.
>
> Verification steps:
> # ./perf kvm --guestkallsyms /home/kallsyms --guestmodules /home/modules record -a sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.651 MB perf.data.guest (~28437 samples) ]
> # ./perf kvm --guestkallsyms /home/kallsyms --guestmodules /home/modules report |grep %
> 22.64% :6471 [guest.kernel.kallsyms] [g] update_rq_clock.part.70
> 19.99% :6471 [guest.kernel.kallsyms] [g] d_free
> 18.46% :6471 [guest.kernel.kallsyms] [g] bio_phys_segments
> 16.25% :6471 [guest.kernel.kallsyms] [g] dequeue_task
> 12.78% :6471 [guest.kernel.kallsyms] [g] __switch_to
> 7.91% :6471 [guest.kernel.kallsyms] [g] scheduler_tick
> 1.75% :6471 [guest.kernel.kallsyms] [g] native_apic_mem_write
> 0.21% :6471 [guest.kernel.kallsyms] [g] apic_timer_interrupt
>
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
> Changelog since V1:
> * More commit message.
>
> tools/perf/util/session.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 989b2e3..a12dfdd 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -830,6 +830,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) ||
> @@ -842,7 +843,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;