All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.