lkml.org 
[lkml]   [2012]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/4] perf kvm: fix bug resolving guest kernel syms
On 7/10/12 6:54 PM, Namhyung Kim wrote:
> Hi, David
>
> On Tue, 10 Jul 2012 15:48:14 -0600, David Ahern wrote:
>> Guest kernel symbols are not resolved despite passing the information
>> needed to resolve them. e.g.,
>>
>> perf kvm --guest --guestmount=/tmp/guest-mount record -a -- sleep 1
>> perf kvm --guest --guestmount=/tmp/guest-mount report --stdio
>>
>> 36.55% [guest/11399] [unknown] [g] 0xffffffff81600bc8
>> 33.19% [guest/10474] [unknown] [g] 0x00000000c0116e00
>> 30.26% [guest/11094] [unknown] [g] 0xffffffff8100a288
>>
>> 43.69% [guest/10474] [unknown] [g] 0x00000000c0103d90
>> 37.38% [guest/11399] [unknown] [g] 0xffffffff81600bc8
>> 12.24% [guest/11094] [unknown] [g] 0xffffffff810aa91d
>> 6.69% [guest/11094] [unknown] [u] 0x00007fa784d721c3
>>
>> which is just pathetic.
>>
>> After a maddening 2 days sifting through perf minutia I found it --
>> id_hdr_size is not initialized for guest machines. This shows up on the
>> report side as random garbage for the cpu and timestamp, e.g.,
>>
>> 29816 7310572949125804849 0x1ac0 [0x50]: PERF_RECORD_MMAP ...
>>
>> that messes up the sample sorting such that synthesized guest maps are
>> processed last.
>>
>> With this patch you get a much more helpful report:
>>
>> 12.11% [guest/11399] [guest.kernel.kallsyms.11399] [g] irqtime_account_process_tick
>> 10.58% [guest/11399] [guest.kernel.kallsyms.11399] [g] run_timer_softirq
>> 6.95% [guest/11094] [guest.kernel.kallsyms.11094] [g] printk_needs_cpu
>> 6.50% [guest/11094] [guest.kernel.kallsyms.11094] [g] do_timer
>> 6.45% [guest/11399] [guest.kernel.kallsyms.11399] [g] idle_balance
>> 4.90% [guest/11094] [guest.kernel.kallsyms.11094] [g] native_read_tsc
>> ...
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Namhyung Kim <namhyung@gmail.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> ---
>> tools/perf/util/map.c | 24 ++++++++++++++++++++++++
>> tools/perf/util/map.h | 1 +
>> tools/perf/util/session.c | 1 +
>> 3 files changed, 26 insertions(+)
>>
>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>> index 641377e..da3411b 100644
>> --- a/tools/perf/util/map.c
>> +++ b/tools/perf/util/map.c
>> @@ -730,3 +730,27 @@ char *machine__mmap_name(struct machine *self, char *bf, size_t size)
>>
>> return bf;
>> }
>> +
>> +void machines__set_id_hdr_size(struct rb_root *self, u16 id_hdr_size)
>> +{
>> + struct rb_node *p;
>> + struct machine *machine;
>> +
>> + p = self->rb_node;
>> + while (p != NULL) {
>> + machine = rb_entry(p, struct machine, rb_node);
>> + machine->id_hdr_size = id_hdr_size;
>> + p = rb_next(p);
>> + }
>> +
>
> Looks like white-space damaged. :(

bleah, something on my dev server converted tabs to spaces. Will fix.

>
> The loop itself looks fine, or you might use this form:
>
> for (p = rb_first(self); p; p = rb_next(p))

Doesn't work - I still see machines not updated.
>
>
> Is there a something like rb_for_each_entry() ?

Didn't see one.

>
>
>> + p = self->rb_node;
>> + if (p)
>> + p = rb_prev(p);
>> + while (p != NULL) {
>> + machine = rb_entry(p, struct machine, rb_node);
>> + machine->id_hdr_size = id_hdr_size;
>> + p = rb_prev(p);
>> + }
>> +
>
> I don't see why this second loop is necessary?

To get the left half of the tree I believe. First loop walks the root
and all of the leaves to the right; this loop walks all of the leaves to
the left.

David


\
 
 \ /
  Last update: 2012-07-11 05:01    [W:0.053 / U:1.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site