Hello.
Sometimes it's handy to display also a filename of a source location, mainly because
source lines can come from different files. I extended 'k' hotkey and one can now see:
1) no source lines:
1.31 │ ↓ je 130 ▒
│ return iterative_hash_object (arg, val); ▒
│ ▒
│ if (!TYPE_P (arg)) ▒
│ movzwl (%rdi),%edx ▒
18.95 │ lea tree_code_type,%rbx ▒
│ movzwl %dx,%ecx ▒
│ mov %rdx,%rax ▒
│ cmpl $0x2,(%rbx,%rcx,4) ▒
11.76 │ ↓ jne 5f ▒
0.65 │ ↓ jmp c0 ▒
│ nop ▒
│ iterative_hash_template_arg(tree_node*, unsigned int) [clone .localalias]: ▒
│ tree_operand_length (const_tree node) ▒
│ { ▒
│ if (VL_EXP_CLASS_P (node)) ▒
│ return VL_EXP_OPERAND_LENGTH (node); ▒
│ else ▒
│ return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
│ 40: lea tree_code_length,%rdx ▒
2) only source lines displayed
1.31 │ ↓ je 130 ▒
│1774 return iterative_hash_object (arg, val); ▒
│ ▒
│1776 if (!TYPE_P (arg)) ▒
│ movzwl (%rdi),%edx ▒
18.95 │ lea tree_code_type,%rbx ▒
│ movzwl %dx,%ecx ▒
│ mov %rdx,%rax ▒
│ cmpl $0x2,(%rbx,%rcx,4) ▒
11.76 │ ↓ jne 5f ▒
0.65 │ ↓ jmp c0 ▒
│ nop ▒
│1785 iterative_hash_template_arg(tree_node*, unsigned int) [clone .localalias]: ▒
│3837 tree_operand_length (const_tree node) ▒
│3838 { ▒
│3839 if (VL_EXP_CLASS_P (node)) ▒
│3840 return VL_EXP_OPERAND_LENGTH (node); ▒
│3841 else ▒
│3842 return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
│ 40: lea tree_code_length,%rdx ▒
full source locations displayed.
1.31 │ ↓ je 130 ▒
│/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 return iterative_hash_object (arg, val); ▒
│ ▒
│/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 if (!TYPE_P (arg)) ▒
│ movzwl (%rdi),%edx ▒
18.95 │ lea tree_code_type,%rbx ▒
│ movzwl %dx,%ecx ▒
│ mov %rdx,%rax ▒
│ cmpl $0x2,(%rbx,%rcx,4) ▒
11.76 │ ↓ jne 5f ▒
0.65 │ ↓ jmp c0 ▒
│ nop ▒
│/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 iterative_hash_template_arg(tree_node*, unsigned int) [▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 tree_operand_length (const_tree node) ▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 { ▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 if (VL_EXP_CLASS_P (node)) ▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 return VL_EXP_OPERAND_LENGTH (node); ▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 else ▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
│ 40: lea tree_code_length,%rdx ▒
Thoughts?
Thanks,
Martin
---
tools/perf/ui/browsers/annotate.c | 11 +++++++++--
tools/perf/util/annotate.c | 24 ++++++++++++++++++------
tools/perf/util/annotate.h | 2 ++
3 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index bd77825fd5a1..ca09736e14a3 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -746,7 +746,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
"t Circulate percent, total period, samples view\n"
"c Show min/max cycle\n"
"/ Search string\n"
- "k Toggle line numbers\n"
+ "k Circulate line numbers, full location, no source lines\n"
"P Print to [symbol_name].annotation file.\n"
"r Run available scripts\n"
"p Toggle percent type [local/global]\n"
@@ -758,7 +758,14 @@ static int annotate_browser__run(struct annotate_browser *browser,
annotate_browser__show(&browser->b, title, help);
continue;
case 'k':
- notes->options->show_linenr = !notes->options->show_linenr;
+ if (notes->options->show_fileloc) {
+ notes->options->show_fileloc = false;
+ notes->options->show_linenr = false;
+ }
+ else if (notes->options->show_linenr)
+ notes->options->show_fileloc = true;
+ else
+ notes->options->show_linenr = true;
break;
case 'H':
nd = browser->curr_hot;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index e3eae646be3e..32e5926d587f 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1159,6 +1159,7 @@ struct annotate_args {
s64 offset;
char *line;
int line_nr;
+ char *fileloc;
};
static void annotation_line__init(struct annotation_line *al,
@@ -1168,6 +1169,7 @@ static void annotation_line__init(struct annotation_line *al,
al->offset = args->offset;
al->line = strdup(args->line);
al->line_nr = args->line_nr;
+ al->fileloc = args->fileloc;
al->data_nr = nr;
}
@@ -1480,7 +1482,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
*/
static int symbol__parse_objdump_line(struct symbol *sym,
struct annotate_args *args,
- char *parsed_line, int *line_nr)
+ char *parsed_line, int *line_nr, char **fileloc)
{
struct map *map = args->ms.map;
struct annotation *notes = symbol__annotation(sym);
@@ -1492,6 +1494,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
/* /filename:linenr ? Save line number and ignore. */
if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
*line_nr = atoi(parsed_line + match[1].rm_so);
+ *fileloc = strdup(parsed_line);
return 0;
}
@@ -1511,6 +1514,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
args->offset = offset;
args->line = parsed_line;
args->line_nr = *line_nr;
+ args->fileloc = *fileloc;
args->ms.sym = sym;
dl = disasm_line__new(args);
@@ -1805,6 +1809,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
args->offset = -1;
args->line = strdup(srcline);
args->line_nr = 0;
+ args->fileloc = NULL;
args->ms.sym = sym;
dl = disasm_line__new(args);
if (dl) {
@@ -1816,6 +1821,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
args->offset = pc;
args->line = buf + prev_buf_size;
args->line_nr = 0;
+ args->fileloc = NULL;
args->ms.sym = sym;
dl = disasm_line__new(args);
if (dl)
@@ -1850,6 +1856,7 @@ symbol__disassemble_bpf_image(struct symbol *sym,
args->offset = -1;
args->line = strdup("to be implemented");
args->line_nr = 0;
+ args->fileloc = NULL;
dl = disasm_line__new(args);
if (dl)
annotation_line__add(&dl->al, ¬es->src->source);
@@ -1931,6 +1938,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
bool delete_extract = false;
bool decomp = false;
int lineno = 0;
+ char *fileloc = strdup("");
int nline;
char *line;
size_t line_len;
@@ -2058,7 +2066,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
* See disasm_line__new() and struct disasm_line::line_nr.
*/
if (symbol__parse_objdump_line(sym, args, expanded_line,
- &lineno) < 0)
+ &lineno, &fileloc) < 0)
break;
nline++;
}
@@ -3004,10 +3012,14 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
if (!*al->line)
obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
else if (al->offset == -1) {
- if (al->line_nr && notes->options->show_linenr)
- printed = scnprintf(bf, sizeof(bf), "%-*d ", notes->widths.addr + 1, al->line_nr);
- else
- printed = scnprintf(bf, sizeof(bf), "%-*s ", notes->widths.addr, " ");
+ if (al->line_nr) {
+ if (notes->options->show_fileloc)
+ printed = scnprintf(bf, sizeof(bf), "%s ", al->fileloc);
+ else if (notes->options->show_linenr)
+ printed = scnprintf(bf, sizeof(bf), "%-*d ", notes->widths.addr + 1, al->line_nr);
+ else
+ printed = scnprintf(bf, sizeof(bf), "%-*s ", notes->widths.addr, " ");
+ }
obj__printf(obj, bf);
obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
} else {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 096cdaf21b01..3757416bcf46 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -84,6 +84,7 @@ struct annotation_options {
print_lines,
full_path,
show_linenr,
+ show_fileloc,
show_nr_jumps,
show_minmax_cycle,
show_asm_raw,
@@ -136,6 +137,7 @@ struct annotation_line {
s64 offset;
char *line;
int line_nr;
+ char *fileloc;
int jump_sources;
float ipc;
u64 cycles;
--
2.30.0
Em Fri, Feb 12, 2021 at 03:42:47PM +0100, Martin Liška escreveu:
> Hello.
>
> Sometimes it's handy to display also a filename of a source location, mainly because
> source lines can come from different files. I extended 'k' hotkey and one can now see:
>
> 1) no source lines:
>
> 1.31 │ ↓ je 130 ▒
> │ return iterative_hash_object (arg, val); ▒
> │ ▒
> │ if (!TYPE_P (arg)) ▒
> │ movzwl (%rdi),%edx ▒
> 18.95 │ lea tree_code_type,%rbx ▒
> │ movzwl %dx,%ecx ▒
> │ mov %rdx,%rax ▒
> │ cmpl $0x2,(%rbx,%rcx,4) ▒
> 11.76 │ ↓ jne 5f ▒
> 0.65 │ ↓ jmp c0 ▒
> │ nop ▒
> │ iterative_hash_template_arg(tree_node*, unsigned int) [clone .localalias]: ▒
> │ tree_operand_length (const_tree node) ▒
> │ { ▒
> │ if (VL_EXP_CLASS_P (node)) ▒
> │ return VL_EXP_OPERAND_LENGTH (node); ▒
> │ else ▒
> │ return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
> │ 40: lea tree_code_length,%rdx ▒
>
> 2) only source lines displayed
>
> 1.31 │ ↓ je 130 ▒
> │1774 return iterative_hash_object (arg, val); ▒
> │ ▒
> │1776 if (!TYPE_P (arg)) ▒
> │ movzwl (%rdi),%edx ▒
> 18.95 │ lea tree_code_type,%rbx ▒
> │ movzwl %dx,%ecx ▒
> │ mov %rdx,%rax ▒
> │ cmpl $0x2,(%rbx,%rcx,4) ▒
> 11.76 │ ↓ jne 5f ▒
> 0.65 │ ↓ jmp c0 ▒
> │ nop ▒
> │1785 iterative_hash_template_arg(tree_node*, unsigned int) [clone .localalias]: ▒
> │3837 tree_operand_length (const_tree node) ▒
> │3838 { ▒
> │3839 if (VL_EXP_CLASS_P (node)) ▒
> │3840 return VL_EXP_OPERAND_LENGTH (node); ▒
> │3841 else ▒
> │3842 return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
> │ 40: lea tree_code_length,%rdx ▒
>
> full source locations displayed.
>
> 1.31 │ ↓ je 130 ▒
> │/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 return iterative_hash_object (arg, val); ▒
> │ ▒
> │/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 if (!TYPE_P (arg)) ▒
> │ movzwl (%rdi),%edx ▒
> 18.95 │ lea tree_code_type,%rbx ▒
> │ movzwl %dx,%ecx ▒
> │ mov %rdx,%rax ▒
> │ cmpl $0x2,(%rbx,%rcx,4) ▒
> 11.76 │ ↓ jne 5f ▒
> 0.65 │ ↓ jmp c0 ▒
> │ nop ▒
> │/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 iterative_hash_template_arg(tree_node*, unsigned int) [▒
> │/home/marxin/Programming/gcc/gcc/tree.h:3837 tree_operand_length (const_tree node) ▒
> │/home/marxin/Programming/gcc/gcc/tree.h:3837 { ▒
> │/home/marxin/Programming/gcc/gcc/tree.h:3837 if (VL_EXP_CLASS_P (node)) ▒
> │/home/marxin/Programming/gcc/gcc/tree.h:3837 return VL_EXP_OPERAND_LENGTH (node); ▒
> │/home/marxin/Programming/gcc/gcc/tree.h:3837 else ▒
> │/home/marxin/Programming/gcc/gcc/tree.h:3837 return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
> │ 40: lea tree_code_length,%rdx ▒
It is valuable, I just think we should try to limit the number of
columns used by the source code line, perhaps something like:
0.65 │ ↓ jmp c0 ▒
│ nop ▒
│/home/marxin/Programming/gcc/gcc/cp/pt.c:1774 ▒
│ iterative_hash_template_arg(tree_node*, unsigned int) [ ▒
│/home/marxin/Programming/gcc/gcc/tree.h:3837 ▒
│ tree_operand_length (const_tree node) ▒
│ { ▒
│ if (VL_EXP_CLASS_P (node)) ▒
│ return VL_EXP_OPERAND_LENGTH (node); ▒
│ else ▒
│ return TREE_CODE_LENGTH (TREE_CODE (node)); ▒
│ 40: lea tree_code_length,%rdx ▒
Another idea is to, when requested, reserve one line at the bottom to
show what is the source code file:line for where the TUI cursor is, i.e.
you press down/up and the line under the cursor has its source file:line
shown at the second (from bottom to top) line in the screen.
- Arnaldo
> Thoughts?
> Thanks,
> Martin
>
> ---
> tools/perf/ui/browsers/annotate.c | 11 +++++++++--
> tools/perf/util/annotate.c | 24 ++++++++++++++++++------
> tools/perf/util/annotate.h | 2 ++
> 3 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index bd77825fd5a1..ca09736e14a3 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -746,7 +746,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> "t Circulate percent, total period, samples view\n"
> "c Show min/max cycle\n"
> "/ Search string\n"
> - "k Toggle line numbers\n"
> + "k Circulate line numbers, full location, no source lines\n"
> "P Print to [symbol_name].annotation file.\n"
> "r Run available scripts\n"
> "p Toggle percent type [local/global]\n"
> @@ -758,7 +758,14 @@ static int annotate_browser__run(struct annotate_browser *browser,
> annotate_browser__show(&browser->b, title, help);
> continue;
> case 'k':
> - notes->options->show_linenr = !notes->options->show_linenr;
> + if (notes->options->show_fileloc) {
> + notes->options->show_fileloc = false;
> + notes->options->show_linenr = false;
> + }
> + else if (notes->options->show_linenr)
> + notes->options->show_fileloc = true;
> + else
> + notes->options->show_linenr = true;
> break;
> case 'H':
> nd = browser->curr_hot;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index e3eae646be3e..32e5926d587f 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1159,6 +1159,7 @@ struct annotate_args {
> s64 offset;
> char *line;
> int line_nr;
> + char *fileloc;
> };
> static void annotation_line__init(struct annotation_line *al,
> @@ -1168,6 +1169,7 @@ static void annotation_line__init(struct annotation_line *al,
> al->offset = args->offset;
> al->line = strdup(args->line);
> al->line_nr = args->line_nr;
> + al->fileloc = args->fileloc;
> al->data_nr = nr;
> }
> @@ -1480,7 +1482,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
> */
> static int symbol__parse_objdump_line(struct symbol *sym,
> struct annotate_args *args,
> - char *parsed_line, int *line_nr)
> + char *parsed_line, int *line_nr, char **fileloc)
> {
> struct map *map = args->ms.map;
> struct annotation *notes = symbol__annotation(sym);
> @@ -1492,6 +1494,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
> /* /filename:linenr ? Save line number and ignore. */
> if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
> *line_nr = atoi(parsed_line + match[1].rm_so);
> + *fileloc = strdup(parsed_line);
> return 0;
> }
> @@ -1511,6 +1514,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
> args->offset = offset;
> args->line = parsed_line;
> args->line_nr = *line_nr;
> + args->fileloc = *fileloc;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> @@ -1805,6 +1809,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
> args->offset = -1;
> args->line = strdup(srcline);
> args->line_nr = 0;
> + args->fileloc = NULL;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> if (dl) {
> @@ -1816,6 +1821,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
> args->offset = pc;
> args->line = buf + prev_buf_size;
> args->line_nr = 0;
> + args->fileloc = NULL;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> if (dl)
> @@ -1850,6 +1856,7 @@ symbol__disassemble_bpf_image(struct symbol *sym,
> args->offset = -1;
> args->line = strdup("to be implemented");
> args->line_nr = 0;
> + args->fileloc = NULL;
> dl = disasm_line__new(args);
> if (dl)
> annotation_line__add(&dl->al, ¬es->src->source);
> @@ -1931,6 +1938,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> bool delete_extract = false;
> bool decomp = false;
> int lineno = 0;
> + char *fileloc = strdup("");
> int nline;
> char *line;
> size_t line_len;
> @@ -2058,7 +2066,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> * See disasm_line__new() and struct disasm_line::line_nr.
> */
> if (symbol__parse_objdump_line(sym, args, expanded_line,
> - &lineno) < 0)
> + &lineno, &fileloc) < 0)
> break;
> nline++;
> }
> @@ -3004,10 +3012,14 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> if (!*al->line)
> obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
> else if (al->offset == -1) {
> - if (al->line_nr && notes->options->show_linenr)
> - printed = scnprintf(bf, sizeof(bf), "%-*d ", notes->widths.addr + 1, al->line_nr);
> - else
> - printed = scnprintf(bf, sizeof(bf), "%-*s ", notes->widths.addr, " ");
> + if (al->line_nr) {
> + if (notes->options->show_fileloc)
> + printed = scnprintf(bf, sizeof(bf), "%s ", al->fileloc);
> + else if (notes->options->show_linenr)
> + printed = scnprintf(bf, sizeof(bf), "%-*d ", notes->widths.addr + 1, al->line_nr);
> + else
> + printed = scnprintf(bf, sizeof(bf), "%-*s ", notes->widths.addr, " ");
> + }
> obj__printf(obj, bf);
> obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
> } else {
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 096cdaf21b01..3757416bcf46 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -84,6 +84,7 @@ struct annotation_options {
> print_lines,
> full_path,
> show_linenr,
> + show_fileloc,
> show_nr_jumps,
> show_minmax_cycle,
> show_asm_raw,
> @@ -136,6 +137,7 @@ struct annotation_line {
> s64 offset;
> char *line;
> int line_nr;
> + char *fileloc;
> int jump_sources;
> float ipc;
> u64 cycles;
> --
> 2.30.0
>
--
- Arnaldo
On 2/12/21 9:34 PM, Arnaldo Carvalho de Melo wrote:
> Another idea is to, when requested, reserve one line at the bottom to
> show what is the source codefile:line for where the TUI cursor is, i.e.
> you press down/up and the line under the cursor has its sourcefile:line
> shown at the second (from bottom to top) line in the screen.
Hello.
I decided to use the footer bar and a full location is displayed when 'l'
hokey is pressed. I think it's quite rare feature, so on demand footer
line usage should be an appropriate place.
Thoughts?
Thanks,
Martin
May I please ping this?
Thanks
On 2/15/21 1:34 PM, Martin Liška wrote:
> On 2/12/21 9:34 PM, Arnaldo Carvalho de Melo wrote:
>> Another idea is to, when requested, reserve one line at the bottom to
>> show what is the source codefile:line for where the TUI cursor is, i.e.
>> you press down/up and the line under the cursor has its sourcefile:line
>> shown at the second (from bottom to top) line in the screen.
>
> Hello.
>
> I decided to use the footer bar and a full location is displayed when 'l'
> hokey is pressed. I think it's quite rare feature, so on demand footer
> line usage should be an appropriate place.
>
> Thoughts?
> Thanks,
> Martin
Em Mon, Feb 15, 2021 at 01:34:29PM +0100, Martin Liška escreveu:
> On 2/12/21 9:34 PM, Arnaldo Carvalho de Melo wrote:
> > Another idea is to, when requested, reserve one line at the bottom to
> > show what is the source codefile:line for where the TUI cursor is, i.e.
> > you press down/up and the line under the cursor has its sourcefile:line
> > shown at the second (from bottom to top) line in the screen.
>
> Hello.
>
> I decided to use the footer bar and a full location is displayed when 'l'
> hokey is pressed. I think it's quite rare feature, so on demand footer
> line usage should be an appropriate place.
>
> Thoughts?
> Thanks,
> Martin
I'm trying to test this with 'perf top', then 'A' on a kernel symbol on
this system:
[root@five ~]# ls -la /usr/lib/debug/lib/modules/5.10.19-200.fc33.x86_64/vmlinux
-rwxr-xr-x. 1 root root 827665640 Feb 26 13:40 /usr/lib/debug/lib/modules/5.10.19-200.fc33.x86_64/vmlinux
[root@five ~]# uname -r
5.10.19-200.fc33.x86_64
[root@five ~]#
the annotate TUI finds the right vmlinux, matching the build-id of the
running kernel, so it should find the line loc, right?
But its not working, I get:
'No source file location.'
Its a fedora 33 system:
[root@five ~]# rpm -qa | grep kernel-debuginfo
kernel-debuginfo-common-x86_64-5.10.10-200.fc33.x86_64
kernel-debuginfo-5.10.10-200.fc33.x86_64
kernel-debuginfo-common-x86_64-5.10.13-200.fc33.x86_64
kernel-debuginfo-5.10.13-200.fc33.x86_64
kernel-debuginfo-common-x86_64-5.10.19-200.fc33.x86_64
kernel-debuginfo-5.10.19-200.fc33.x86_64
[root@five ~]# uname -a
Linux five 5.10.19-200.fc33.x86_64 #1 SMP Fri Feb 26 16:21:30 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
[root@five ~]#
I see, it works only when pressing on source code lines:
Source file location: /usr/src/debug/kernel-5.10.fc33/linux-5.10.19-200.fc33.x86_64/./arch/x86/include/asm/msr.h:205
So we better improve that message? I.e. 'press on source code lines', or
'enable showing source code lines to get line number'?
- Arnaldo
> From f85a5d7e66c3437b62622ebdb1cd690722d05c53 Mon Sep 17 00:00:00 2001
> From: Martin Liska <[email protected]>
> Date: Mon, 15 Feb 2021 12:34:46 +0100
> Subject: [PATCH] perf annotate: show full source location with 'l' hotkey
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Right now, when Line numbers are displayed, one can't easily
> find a source file that the line corresponds to.
>
> When a source line is selected and 'l' is pressed, full source file
> location is displayed in perf UI footer line.
>
> Signed-off-by: Martin Liška <[email protected]>
> ---
> tools/perf/ui/browsers/annotate.c | 23 +++++++++++++++++++++--
> tools/perf/util/annotate.c | 12 ++++++++++--
> tools/perf/util/annotate.h | 2 ++
> 3 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index bd77825fd5a1..adf5ce513438 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -381,6 +381,23 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> return true;
> }
>
> +#define SYM_TITLE_MAX_SIZE (PATH_MAX + 64)
> +
> +static void annotate_browser__show_full_location(struct ui_browser *browser)
> +{
> + struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
> + struct disasm_line *cursor = disasm_line(ab->selection);
> + struct annotation_line *al = &cursor->al;
> +
> + if (al->offset != -1 || al->fileloc == NULL) {
> + ui_helpline__puts("No source file location.");
> + } else {
> + char help_line[SYM_TITLE_MAX_SIZE];
> + sprintf (help_line, "Source file location: %s", al->fileloc);
> + ui_helpline__puts(help_line);
> + }
> +}
> +
> static void ui_browser__init_asm_mode(struct ui_browser *browser)
> {
> struct annotation *notes = browser__annotation(browser);
> @@ -388,8 +405,6 @@ static void ui_browser__init_asm_mode(struct ui_browser *browser)
> browser->nr_entries = notes->nr_asm_entries;
> }
>
> -#define SYM_TITLE_MAX_SIZE (PATH_MAX + 64)
> -
> static int sym_title(struct symbol *sym, struct map *map, char *title,
> size_t sz, int percent_type)
> {
> @@ -747,6 +762,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> "c Show min/max cycle\n"
> "/ Search string\n"
> "k Toggle line numbers\n"
> + "l Show full source file location\n"
> "P Print to [symbol_name].annotation file.\n"
> "r Run available scripts\n"
> "p Toggle percent type [local/global]\n"
> @@ -760,6 +776,9 @@ static int annotate_browser__run(struct annotate_browser *browser,
> case 'k':
> notes->options->show_linenr = !notes->options->show_linenr;
> break;
> + case 'l':
> + annotate_browser__show_full_location (&browser->b);
> + continue;
> case 'H':
> nd = browser->curr_hot;
> break;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index e3eae646be3e..e4b0c21362d8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1159,6 +1159,7 @@ struct annotate_args {
> s64 offset;
> char *line;
> int line_nr;
> + char *fileloc;
> };
>
> static void annotation_line__init(struct annotation_line *al,
> @@ -1168,6 +1169,7 @@ static void annotation_line__init(struct annotation_line *al,
> al->offset = args->offset;
> al->line = strdup(args->line);
> al->line_nr = args->line_nr;
> + al->fileloc = args->fileloc;
> al->data_nr = nr;
> }
>
> @@ -1480,7 +1482,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
> */
> static int symbol__parse_objdump_line(struct symbol *sym,
> struct annotate_args *args,
> - char *parsed_line, int *line_nr)
> + char *parsed_line, int *line_nr, char **fileloc)
> {
> struct map *map = args->ms.map;
> struct annotation *notes = symbol__annotation(sym);
> @@ -1492,6 +1494,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
> /* /filename:linenr ? Save line number and ignore. */
> if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
> *line_nr = atoi(parsed_line + match[1].rm_so);
> + *fileloc = strdup(parsed_line);
> return 0;
> }
>
> @@ -1511,6 +1514,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
> args->offset = offset;
> args->line = parsed_line;
> args->line_nr = *line_nr;
> + args->fileloc = *fileloc;
> args->ms.sym = sym;
>
> dl = disasm_line__new(args);
> @@ -1805,6 +1809,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
> args->offset = -1;
> args->line = strdup(srcline);
> args->line_nr = 0;
> + args->fileloc = NULL;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> if (dl) {
> @@ -1816,6 +1821,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
> args->offset = pc;
> args->line = buf + prev_buf_size;
> args->line_nr = 0;
> + args->fileloc = NULL;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> if (dl)
> @@ -1850,6 +1856,7 @@ symbol__disassemble_bpf_image(struct symbol *sym,
> args->offset = -1;
> args->line = strdup("to be implemented");
> args->line_nr = 0;
> + args->fileloc = NULL;
> dl = disasm_line__new(args);
> if (dl)
> annotation_line__add(&dl->al, ¬es->src->source);
> @@ -1931,6 +1938,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> bool delete_extract = false;
> bool decomp = false;
> int lineno = 0;
> + char *fileloc = NULL;
> int nline;
> char *line;
> size_t line_len;
> @@ -2058,7 +2066,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> * See disasm_line__new() and struct disasm_line::line_nr.
> */
> if (symbol__parse_objdump_line(sym, args, expanded_line,
> - &lineno) < 0)
> + &lineno, &fileloc) < 0)
> break;
> nline++;
> }
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 096cdaf21b01..3757416bcf46 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -84,6 +84,7 @@ struct annotation_options {
> print_lines,
> full_path,
> show_linenr,
> + show_fileloc,
> show_nr_jumps,
> show_minmax_cycle,
> show_asm_raw,
> @@ -136,6 +137,7 @@ struct annotation_line {
> s64 offset;
> char *line;
> int line_nr;
> + char *fileloc;
> int jump_sources;
> float ipc;
> u64 cycles;
> --
> 2.30.0
>
--
- Arnaldo
On 3/6/21 3:31 PM, Arnaldo Carvalho de Melo wrote:
> I see, it works only when pressing on source code lines:
Hey.
Yes, I forgot to explicitly mention that.
>
> Source file location: /usr/src/debug/kernel-5.10.fc33/linux-5.10.19-200.fc33.x86_64/./arch/x86/include/asm/msr.h:205
>
> So we better improve that message? I.e. 'press on source code lines', or
> 'enable showing source code lines to get line number'?
Fixed that in the attached patch. I got inspiration from 's' hotkey which prints the following warning:
"Only available for assembly lines.".
Thanks,
Martin
Em Sat, Mar 06, 2021 at 08:02:24PM +0100, Martin Liška escreveu:
> On 3/6/21 3:31 PM, Arnaldo Carvalho de Melo wrote:
> > I see, it works only when pressing on source code lines:
>
> Hey.
>
> Yes, I forgot to explicitly mention that.
Some requests, send the mail inline, not as an attachment, so that I can
use scripts that will extract the Message-ID, add it as a Link: tag,
etc.
Also something Ingo asked me since the dawn of times and I grew used to:
[PATCH][RFC] perf annotate: show full line locations with 'k' in
[PATCH][RFC] perf annotate: Show full line locations with 'k' in
Capitalize that :-)
Also please base your work on my perf/core branch, as I had to do
adjustments to one of the patch hunks.
> > Source file location: /usr/src/debug/kernel-5.10.fc33/linux-5.10.19-200.fc33.x86_64/./arch/x86/include/asm/msr.h:205
> >
> > So we better improve that message? I.e. 'press on source code lines', or
> > 'enable showing source code lines to get line number'?
>
> Fixed that in the attached patch. I got inspiration from 's' hotkey which prints the following warning:
> "Only available for assembly lines.".
Thanks, testing now.
- Arnaldo
> Thanks,
> Martin
> From 8fb7db7722c481ee4d1e0de2d2dc884f25aa90a1 Mon Sep 17 00:00:00 2001
> From: Martin Liska <[email protected]>
> Date: Mon, 15 Feb 2021 12:34:46 +0100
> Subject: [PATCH] perf annotate: show full source location with 'l' hotkey
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Right now, when Line numbers are displayed, one can't easily
> find a source file that the line corresponds to.
>
> When a source line is selected and 'l' is pressed, full source file
> location is displayed in perf UI footer line. The hotkey works
> only for source code lines.
>
> Signed-off-by: Martin Liška <[email protected]>
> ---
> tools/perf/ui/browsers/annotate.c | 25 +++++++++++++++++++++++--
> tools/perf/util/annotate.c | 12 ++++++++++--
> tools/perf/util/annotate.h | 2 ++
> 3 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index bd77825fd5a1..cf60ba59b903 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -381,6 +381,25 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> return true;
> }
>
> +#define SYM_TITLE_MAX_SIZE (PATH_MAX + 64)
> +
> +static void annotate_browser__show_full_location(struct ui_browser *browser)
> +{
> + struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
> + struct disasm_line *cursor = disasm_line(ab->selection);
> + struct annotation_line *al = &cursor->al;
> +
> + if (al->offset != -1)
> + ui_helpline__puts("Only available for source code lines.");
> + else if (al->fileloc == NULL)
> + ui_helpline__puts("No source file location.");
> + else {
> + char help_line[SYM_TITLE_MAX_SIZE];
> + sprintf (help_line, "Source file location: %s", al->fileloc);
> + ui_helpline__puts(help_line);
> + }
> +}
> +
> static void ui_browser__init_asm_mode(struct ui_browser *browser)
> {
> struct annotation *notes = browser__annotation(browser);
> @@ -388,8 +407,6 @@ static void ui_browser__init_asm_mode(struct ui_browser *browser)
> browser->nr_entries = notes->nr_asm_entries;
> }
>
> -#define SYM_TITLE_MAX_SIZE (PATH_MAX + 64)
> -
> static int sym_title(struct symbol *sym, struct map *map, char *title,
> size_t sz, int percent_type)
> {
> @@ -747,6 +764,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> "c Show min/max cycle\n"
> "/ Search string\n"
> "k Toggle line numbers\n"
> + "l Show full source file location\n"
> "P Print to [symbol_name].annotation file.\n"
> "r Run available scripts\n"
> "p Toggle percent type [local/global]\n"
> @@ -760,6 +778,9 @@ static int annotate_browser__run(struct annotate_browser *browser,
> case 'k':
> notes->options->show_linenr = !notes->options->show_linenr;
> break;
> + case 'l':
> + annotate_browser__show_full_location (&browser->b);
> + continue;
> case 'H':
> nd = browser->curr_hot;
> break;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index e3eae646be3e..e4b0c21362d8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1159,6 +1159,7 @@ struct annotate_args {
> s64 offset;
> char *line;
> int line_nr;
> + char *fileloc;
> };
>
> static void annotation_line__init(struct annotation_line *al,
> @@ -1168,6 +1169,7 @@ static void annotation_line__init(struct annotation_line *al,
> al->offset = args->offset;
> al->line = strdup(args->line);
> al->line_nr = args->line_nr;
> + al->fileloc = args->fileloc;
> al->data_nr = nr;
> }
>
> @@ -1480,7 +1482,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
> */
> static int symbol__parse_objdump_line(struct symbol *sym,
> struct annotate_args *args,
> - char *parsed_line, int *line_nr)
> + char *parsed_line, int *line_nr, char **fileloc)
> {
> struct map *map = args->ms.map;
> struct annotation *notes = symbol__annotation(sym);
> @@ -1492,6 +1494,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
> /* /filename:linenr ? Save line number and ignore. */
> if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
> *line_nr = atoi(parsed_line + match[1].rm_so);
> + *fileloc = strdup(parsed_line);
> return 0;
> }
>
> @@ -1511,6 +1514,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
> args->offset = offset;
> args->line = parsed_line;
> args->line_nr = *line_nr;
> + args->fileloc = *fileloc;
> args->ms.sym = sym;
>
> dl = disasm_line__new(args);
> @@ -1805,6 +1809,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
> args->offset = -1;
> args->line = strdup(srcline);
> args->line_nr = 0;
> + args->fileloc = NULL;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> if (dl) {
> @@ -1816,6 +1821,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
> args->offset = pc;
> args->line = buf + prev_buf_size;
> args->line_nr = 0;
> + args->fileloc = NULL;
> args->ms.sym = sym;
> dl = disasm_line__new(args);
> if (dl)
> @@ -1850,6 +1856,7 @@ symbol__disassemble_bpf_image(struct symbol *sym,
> args->offset = -1;
> args->line = strdup("to be implemented");
> args->line_nr = 0;
> + args->fileloc = NULL;
> dl = disasm_line__new(args);
> if (dl)
> annotation_line__add(&dl->al, ¬es->src->source);
> @@ -1931,6 +1938,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> bool delete_extract = false;
> bool decomp = false;
> int lineno = 0;
> + char *fileloc = NULL;
> int nline;
> char *line;
> size_t line_len;
> @@ -2058,7 +2066,7 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> * See disasm_line__new() and struct disasm_line::line_nr.
> */
> if (symbol__parse_objdump_line(sym, args, expanded_line,
> - &lineno) < 0)
> + &lineno, &fileloc) < 0)
> break;
> nline++;
> }
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 096cdaf21b01..3757416bcf46 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -84,6 +84,7 @@ struct annotation_options {
> print_lines,
> full_path,
> show_linenr,
> + show_fileloc,
> show_nr_jumps,
> show_minmax_cycle,
> show_asm_raw,
> @@ -136,6 +137,7 @@ struct annotation_line {
> s64 offset;
> char *line;
> int line_nr;
> + char *fileloc;
> int jump_sources;
> float ipc;
> u64 cycles;
> --
> 2.30.1
>
--
- Arnaldo