From: "Jin, Yao" <yao.jin@linux.intel.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Cc: jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com,
alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org,
ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols
Date: Thu, 19 Mar 2020 08:49:38 +0800 [thread overview]
Message-ID: <49849512-c6a7-22ab-5c93-03fd78ad82d6@linux.intel.com> (raw)
In-Reply-To: <20200318154206.GG11531@kernel.org>
On 3/18/2020 11:42 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 27, 2020 at 12:39:38PM +0800, Jin Yao escreveu:
>> For perf report on stripped binaries it is currently impossible to do
>> annotation. The annotation state is all tied to symbols, but there are
>> either no symbols, or symbols are not covering all the code.
>>
>> We should support the annotation functionality even without symbols.
>>
>> This patch fakes a symbol and the symbol name is the string of address.
>> After that, we just follow current annotation working flow.
>>
>> For example,
>>
>> 1. perf report
>>
>> Overhead Command Shared Object Symbol
>> 20.67% div libc-2.27.so [.] __random_r
>> 17.29% div libc-2.27.so [.] __random
>> 10.59% div div [.] 0x0000000000000628
>> 9.25% div div [.] 0x0000000000000612
>> 6.11% div div [.] 0x0000000000000645
>>
>> 2. Select the line of "10.59% div div [.] 0x0000000000000628" and ENTER.
>>
>> Annotate 0x0000000000000628
>> Zoom into div thread
>> Zoom into div DSO (use the 'k' hotkey to zoom directly into the kernel)
>> Browse map details
>> Run scripts for samples of symbol [0x0000000000000628]
>> Run scripts for all samples
>> Switch to another data file in PWD
>> Exit
>>
>> 3. Select the "Annotate 0x0000000000000628" and ENTER.
>>
>> Percent│
>> │
>> │
>> │ Disassembly of section .text:
>> │
>> │ 0000000000000628 <.text+0x68>:
>> │ divsd %xmm4,%xmm0
>> │ divsd %xmm3,%xmm1
>> │ movsd (%rsp),%xmm2
>> │ addsd %xmm1,%xmm0
>> │ addsd %xmm2,%xmm0
>> │ movsd %xmm0,(%rsp)
>>
>> Now we can see the dump of object starting from 0x628.
>
> Testing this I noticed this discrepancy when using 'o' in the annotate
> view to see the address columns:
>
> Samples: 10K of event 'cycles', 4000 Hz, Event count (approx.): 7738221585
> 0x0000000000ea8b97 /usr/libexec/gcc/x86_64-redhat-linux/9/cc1 [Percent: local period]
> Percent│ ▒
> │ ▒
> │ ▒
> │ Disassembly of section .text: ▒
> │ ◆
> │ 00000000012a8b97 <linemap_get_expansion_line@@Base+0x227>: ▒
> │12a8b97: cmp %rax,(%rdi) ▒
> │12a8b9a: ↓ je 12a8ba0 <linemap_get_expansion_line@@Base+0x230> ▒
> │12a8b9c: xor %eax,%eax ▒
> │12a8b9e: ← retq ▒
> │12a8b9f: nop ▒
> │12a8ba0: mov 0x8(%rsi),%edx
>
>
>
> See that 0x0000000000ea8b97 != 12a8b97
>
> How can we explain that?
>
> - Arnaldo
>
Not very clear about the reason. Actually the patch doesn't change the
annotate view, it only fakes a dummy symbol.
I want to debug it but I'm failed to reproduce this issue on my machine. :(
Thanks
Jin Yao
>> v5:
>> ---
>> Remove the hotkey 'a' implementation from this patch. It
>> will be moved to a separate patch.
>>
>> v4:
>> ---
>> 1. Support the hotkey 'a'. When we press 'a' on address,
>> now it supports the annotation.
>>
>> 2. Change the patch title from
>> "Support interactive annotation of code without symbols" to
>> "perf report: Support interactive annotation of code without symbols"
>>
>> v3:
>> ---
>> Keep just the ANNOTATION_DUMMY_LEN, and remove the
>> opts->annotate_dummy_len since it's the "maybe in future
>> we will provide" feature.
>>
>> v2:
>> ---
>> Fix a crash issue when annotating an address in "unknown" object.
>>
>> The steps to reproduce this issue:
>>
>> perf record -e cycles:u ls
>> perf report
>>
>> 75.29% ls ld-2.27.so [.] do_lookup_x
>> 23.64% ls ld-2.27.so [.] __GI___tunables_init
>> 1.04% ls [unknown] [k] 0xffffffff85c01210
>> 0.03% ls ld-2.27.so [.] _start
>>
>> When annotating 0xffffffff85c01210, the crash happens.
>>
>> v2 adds checking for ms->map in add_annotate_opt(). If the object is
>> "unknown", ms->map is NULL.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>> tools/perf/ui/browsers/hists.c | 43 +++++++++++++++++++++++++++++-----
>> tools/perf/util/annotate.h | 1 +
>> 2 files changed, 38 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index f36dee499320..2f07680559c4 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -2465,13 +2465,41 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
>> return 0;
>> }
>>
>> +static struct symbol *new_annotate_sym(u64 addr, struct map *map)
>> +{
>> + struct symbol *sym;
>> + struct annotated_source *src;
>> + char name[64];
>> +
>> + snprintf(name, sizeof(name), "%-#.*lx", BITS_PER_LONG / 4, addr);
>> +
>> + sym = symbol__new(addr, ANNOTATION_DUMMY_LEN, 0, 0, name);
>> + if (sym) {
>> + src = symbol__hists(sym, 1);
>> + if (!src) {
>> + symbol__delete(sym);
>> + return NULL;
>> + }
>> +
>> + dso__insert_symbol(map->dso, sym);
>> + }
>> +
>> + return sym;
>> +}
>> +
>> static int
>> add_annotate_opt(struct hist_browser *browser __maybe_unused,
>> struct popup_action *act, char **optstr,
>> - struct map_symbol *ms)
>> + struct map_symbol *ms,
>> + u64 addr)
>> {
>> - if (ms->sym == NULL || ms->map->dso->annotate_warned ||
>> - symbol__annotation(ms->sym)->src == NULL)
>> + if (!ms->map || !ms->map->dso || ms->map->dso->annotate_warned)
>> + return 0;
>> +
>> + if (!ms->sym)
>> + ms->sym = new_annotate_sym(addr, ms->map);
>> +
>> + if (ms->sym == NULL || symbol__annotation(ms->sym)->src == NULL)
>> return 0;
>>
>> if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
>> @@ -3219,17 +3247,20 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>> nr_options += add_annotate_opt(browser,
>> &actions[nr_options],
>> &options[nr_options],
>> - &bi->from.ms);
>> + &bi->from.ms,
>> + bi->from.al_addr);
>> if (bi->to.ms.sym != bi->from.ms.sym)
>> nr_options += add_annotate_opt(browser,
>> &actions[nr_options],
>> &options[nr_options],
>> - &bi->to.ms);
>> + &bi->to.ms,
>> + bi->to.al_addr);
>> } else {
>> nr_options += add_annotate_opt(browser,
>> &actions[nr_options],
>> &options[nr_options],
>> - browser->selection);
>> + browser->selection,
>> + browser->he_selection->ip);
>> }
>> skip_annotation:
>> nr_options += add_thread_opt(browser, &actions[nr_options],
>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>> index 455403e8fede..c8463c48adf2 100644
>> --- a/tools/perf/util/annotate.h
>> +++ b/tools/perf/util/annotate.h
>> @@ -74,6 +74,7 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2);
>> #define ANNOTATION__CYCLES_WIDTH 6
>> #define ANNOTATION__MINMAX_CYCLES_WIDTH 19
>> #define ANNOTATION__AVG_IPC_WIDTH 36
>> +#define ANNOTATION_DUMMY_LEN 256
>>
>> struct annotation_options {
>> bool hide_src_code,
>> --
>> 2.17.1
>>
>
next prev parent reply other threads:[~2020-03-19 0:49 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-27 4:39 [PATCH v5 0/3] perf report: Support annotation of code without symbols Jin Yao
2020-02-27 4:39 ` [PATCH v5 1/3] perf util: Print al_addr when symbol is not found Jin Yao
2020-04-04 8:42 ` [tip: perf/urgent] perf report: " tip-bot2 for Jin Yao
2020-02-27 4:39 ` [PATCH v5 2/3] perf report: Support interactive annotation of code without symbols Jin Yao
2020-03-18 15:42 ` Arnaldo Carvalho de Melo
2020-03-18 15:43 ` Arnaldo Carvalho de Melo
2020-03-18 15:46 ` Arnaldo Carvalho de Melo
2020-03-19 1:11 ` Jin, Yao
2020-03-19 0:56 ` Jin, Yao
2020-03-19 0:49 ` Jin, Yao [this message]
2020-04-04 8:42 ` [tip: perf/urgent] " tip-bot2 for Jin Yao
2020-02-27 4:39 ` [PATCH v5 3/3] perf report: Support hotkey 'a' on address for annotation Jin Yao
2020-04-04 8:42 ` [tip: perf/urgent] perf report/top TUI: Support hotkey 'a' for annotation of unresolved addresses tip-bot2 for Jin Yao
2020-03-02 4:08 ` [PATCH v5 0/3] perf report: Support annotation of code without symbols Ravi Bangoria
2020-03-02 12:51 ` Jiri Olsa
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=49849512-c6a7-22ab-5c93-03fd78ad82d6@linux.intel.com \
--to=yao.jin@linux.intel.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=arnaldo.melo@gmail.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
/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;
as well as URLs for NNTP newsgroup(s).