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