linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
@ 2021-02-12 14:42 Martin Liška
  2021-02-12 20:34 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Martin Liška @ 2021-02-12 14:42 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users; +Cc: Arnaldo Carvalho de Melo

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, &notes->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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
  2021-02-12 14:42 [PATCH][RFC] perf annotate: show full line locations with 'k' in UI Martin Liška
@ 2021-02-12 20:34 ` Arnaldo Carvalho de Melo
  2021-02-15 12:34   ` Martin Liška
  0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-02-12 20:34 UTC (permalink / raw)
  To: Martin Liška; +Cc: linux-kernel, linux-perf-users

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, &notes->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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
  2021-02-12 20:34 ` Arnaldo Carvalho de Melo
@ 2021-02-15 12:34   ` Martin Liška
  2021-02-26 10:04     ` Martin Liška
  2021-03-06 14:31     ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 7+ messages in thread
From: Martin Liška @ 2021-02-15 12:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, linux-perf-users

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

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

[-- Attachment #2: 0001-perf-annotate-show-full-source-location-with-l-hotke.patch --]
[-- Type: text/x-patch, Size: 6290 bytes --]

From f85a5d7e66c3437b62622ebdb1cd690722d05c53 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
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 <mliska@suse.cz>
---
 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, &notes->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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
  2021-02-15 12:34   ` Martin Liška
@ 2021-02-26 10:04     ` Martin Liška
  2021-03-06 14:31     ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 7+ messages in thread
From: Martin Liška @ 2021-02-26 10:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, linux-perf-users

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
  2021-02-15 12:34   ` Martin Liška
  2021-02-26 10:04     ` Martin Liška
@ 2021-03-06 14:31     ` Arnaldo Carvalho de Melo
  2021-03-06 19:02       ` Martin Liška
  1 sibling, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-06 14:31 UTC (permalink / raw)
  To: Martin Liška; +Cc: linux-kernel, linux-perf-users

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 <mliska@suse.cz>
> 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 <mliska@suse.cz>
> ---
>  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, &notes->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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
  2021-03-06 14:31     ` Arnaldo Carvalho de Melo
@ 2021-03-06 19:02       ` Martin Liška
  2021-03-06 19:16         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Martin Liška @ 2021-03-06 19:02 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, linux-perf-users

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

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

[-- Attachment #2: 0001-perf-annotate-show-full-source-location-with-l-hotke.patch --]
[-- Type: text/x-patch, Size: 6405 bytes --]

From 8fb7db7722c481ee4d1e0de2d2dc884f25aa90a1 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
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 <mliska@suse.cz>
---
 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, &notes->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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH][RFC] perf annotate: show full line locations with 'k' in UI
  2021-03-06 19:02       ` Martin Liška
@ 2021-03-06 19:16         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-06 19:16 UTC (permalink / raw)
  To: Martin Liška; +Cc: linux-kernel, linux-perf-users

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 <mliska@suse.cz>
> 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 <mliska@suse.cz>
> ---
>  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, &notes->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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-06 19:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-12 14:42 [PATCH][RFC] perf annotate: show full line locations with 'k' in UI Martin Liška
2021-02-12 20:34 ` Arnaldo Carvalho de Melo
2021-02-15 12:34   ` Martin Liška
2021-02-26 10:04     ` Martin Liška
2021-03-06 14:31     ` Arnaldo Carvalho de Melo
2021-03-06 19:02       ` Martin Liška
2021-03-06 19:16         ` Arnaldo Carvalho de Melo

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