lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <52B48E5A.8090106@cn.fujitsu.com>
Date:	Fri, 20 Dec 2013 13:37:14 -0500
From:	Dongsheng Yang <yangds.fnst@...fujitsu.com>
To:	David Ahern <dsahern@...il.com>
CC:	acme@...stprotocols.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] perf tools: Fix bug for perf kvm report without guestmount.

On 12/20/2013 12:31 AM, David Ahern wrote:
> On 12/16/13, 9: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@...il.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);
>
> Adding the diff to the commit message confuses git-am. Resubmit the 
> patch with just the commit id that introduced the change.
>
>> 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@...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;
>>
>
> And for this change you can add an
> Acked-by: David Ahern <dsahern@...il.com>
>
> So this patch fixes the behavior for defaulting to a guest machine 
> with the --guestkallsyms / --guestmodules options. And for the 
> curious, last properly working version of perf-kvm for --guestkallsyms 
> + --guestmodules options is v3.2. Thanks for digging into it the 
> regression.
>
> With this patch the only degradation for these options appears to be 
> the comm change. I'd like to see it stay [guest/<pid>], but was not 
> able to find a quick solution for that.
>

Thank you, David. I will send a v3 soon.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ