[PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI

Namhyung Kim posted 12 patches 1 month, 2 weeks ago
[PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
Posted by Namhyung Kim 1 month, 2 weeks ago
Until now, the --code-with-type option is available only on stdio.
But it was an artifical limitation because of an implemention issue.

Implement the same logic in annotation_line__write() for stdio2/TUI
and remove the limitation and update the man page.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-annotate.txt |  1 -
 tools/perf/builtin-annotate.c              |  5 ---
 tools/perf/ui/browsers/annotate.c          |  6 +++
 tools/perf/util/annotate.c                 | 47 ++++++++++++++++++++--
 4 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
index 46090c5b42b4762f..547f1a2680185e3c 100644
--- a/tools/perf/Documentation/perf-annotate.txt
+++ b/tools/perf/Documentation/perf-annotate.txt
@@ -170,7 +170,6 @@ include::itrace.txt[]
 
 --code-with-type::
 	Show data type info in code annotation (for memory instructions only).
-	Currently it only works with --stdio option.
 
 
 SEE ALSO
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 5d57d2913f3d9a33..646f43b0f7c4c9b0 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
 		symbol_conf.annotate_data_sample = true;
 	} else if (annotate_opts.code_with_type) {
 		symbol_conf.annotate_data_member = true;
-
-		if (!annotate.use_stdio) {
-			pr_err("--code-with-type only works with --stdio.\n");
-			goto out_delete;
-		}
 	}
 
 	setup_browser(true);
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 3e8b111e3f12b030..e5b35336f0d33d7e 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -4,6 +4,7 @@
 #include "../ui.h"
 #include "../../util/annotate.h"
 #include "../../util/debug.h"
+#include "../../util/debuginfo.h"
 #include "../../util/dso.h"
 #include "../../util/hist.h"
 #include "../../util/sort.h"
@@ -1119,6 +1120,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
 
 	ui_helpline__push("Press ESC to exit");
 
+	if (annotate_opts.code_with_type)
+		browser.dbg = debuginfo__new(dso__long_name(dso));
+
 	browser.b.width = notes->src->widths.max_line_len;
 	browser.b.nr_entries = notes->src->nr_entries;
 	browser.b.entries = &notes->src->source;
@@ -1129,6 +1133,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
 
 	ret = annotate_browser__run(&browser, evsel, hbt);
 
+	if (annotate_opts.code_with_type)
+		debuginfo__delete(browser.dbg);
 	if (not_annotated && !notes->src->tried_source)
 		annotated_source__purge(notes->src);
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 698bc4f559e83043..99e976d254493de2 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
 	};
 	struct annotation_line *al;
 
+	if (annotate_opts.code_with_type) {
+		evsel__get_arch(apd->evsel, &apd->arch);
+		apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
+	}
+
 	list_for_each_entry(al, &notes->src->source, node) {
 		if (annotation_line__filter(al))
 			continue;
@@ -1370,6 +1375,9 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
 		wops.first_line = false;
 	}
 
+	if (annotate_opts.code_with_type)
+		debuginfo__delete(apd->dbg);
+
 	return 0;
 }
 
@@ -1935,6 +1943,36 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
 	return -ENOMEM;
 }
 
+static int disasm_line__snprint_type_info(struct disasm_line *dl,
+					  char *buf, int len,
+					  struct annotation_print_data *apd)
+{
+	struct annotated_data_type *data_type;
+	char member[256];
+	int offset = 0;
+	int printed;
+
+	scnprintf(buf, len, " ");
+
+	if (!annotate_opts.code_with_type || apd->dbg == NULL)
+		return 1;
+
+	data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
+	if (data_type == NULL || data_type == NO_TYPE)
+		return 1;
+
+	printed = scnprintf(buf, len, "\t\t# data-type: %s", data_type->self.type_name);
+
+	if (data_type != &stackop_type && data_type != &canary_type && len > printed)
+		printed += scnprintf(buf + printed, len - printed, " +%#x", offset);
+
+	if (annotated_data_type__get_member_name(data_type, member, sizeof(member), offset) &&
+	    len > printed) {
+		printed += scnprintf(buf + printed, len - printed, " (%s)", member);
+	}
+	return printed;
+}
+
 void annotation_line__write(struct annotation_line *al, struct annotation *notes,
 			    const struct annotation_write_ops *wops,
 			    struct annotation_print_data *apd)
@@ -2118,11 +2156,14 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 
 		width -= printed;
 
-		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
+		printed = disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf),
+					     obj__printf, obj__write_graph);
 
-		obj__printf(obj, "%-*s", width, bf);
+		obj__printf(obj, "%s", bf);
+		width -= printed;
 
-		(void)apd;
+		disasm_line__snprint_type_info(disasm_line(al), bf, sizeof(bf), apd);
+		obj__printf(obj, "%-*s", width, bf);
 	}
 
 }
-- 
2.50.1
Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
Posted by Arnaldo Carvalho de Melo 1 month, 2 weeks ago
On Fri, Aug 15, 2025 at 08:16:30PM -0700, Namhyung Kim wrote:
> Until now, the --code-with-type option is available only on stdio.
> But it was an artifical limitation because of an implemention issue.
> 
> Implement the same logic in annotation_line__write() for stdio2/TUI
> and remove the limitation and update the man page.

Samples: 54K of event 'cycles:P', 4000 Hz, Event count (approx.): 25749717559
poll_idle  /usr/lib/debug/lib/modules/6.15.9-201.fc42.x86_64/vmlinux [Percent: local period]
Percent │      mov     %rdi,%rbx                                                                                                                                        ▒
        │    → call    local_clock_noinstr              # data-type: (stack operation)                                                                                  ▒
        │      andb    $0xfb,(%rbx)             # data-type: struct cpuidle_device +0 (registered:1)                                                                    ▒
        │      mov     %rax,%rbp                                                                                                                                        ▒
        │    → call    *0x1342a22(%rip)        # ffffffff8384f778 <pv_ops+0xf8>         # data-type: (stack operation)                                                  ▒
        │      mov     current_task,%r14                # data-type: struct task_struct* +0                                                                             ▒
        │      lock    orb     $0x20,0x2(%r14)          # data-type: struct task_struct +0x2 (thread_info.flags)                                                        ▒
        │      mov     (%r14),%rax              # data-type: struct task_struct +0 (thread_info.flags)                                                                  ▒
        │      test    $0x8,%al                                                                                                                                         ▒
        │    ↓ jne     6d                                                                                                                                               ▒
        │      mov     %r12,%rdi                                                                                                                                        ▒
        │      mov     %rbx,%rsi                                                                                                                                        ▒
        │    → call    cpuidle_poll_time                # data-type: (stack operation)                                                                                  ▒
        │      mov     %rax,%r12                                                                                                                                        ▒
        │49:   mov     $0xc9,%eax                                                                                                                                       ▒
   0.75 │4e:   mov     (%r14),%rdx              # data-type: struct task_struct +0 (thread_info.flags)                                                                  ▒
   0.42 │      and     $0x8,%edx                                                                                                                                        ▒
        │    ↓ jne     6d                                                                                                                                               ▒
  97.81 │      pause                                                                                                                                                    ▒
   0.70 │      sub     $0x1,%eax                                                                                                                                        ▒
   0.04 │    ↑ jne     4e                                                                                                                                               ▒
   0.14 │    → call    local_clock_noinstr              # data-type: (stack operation)                                                                                  ▒
   0.01 │      sub     %rbp,%rax                                                                                                                                        ▒
   0.05 │      cmp     %rax,%r12                                                                                                                                        ▒
        │    ↑ jae     49                                                                                                                                               ▒
        │      orb     $0x4,(%rbx)              # data-type: struct cpuidle_device +0 (registered:1)                                                                    ▒
        │6d: → call    *0x13429cd(%rip)        # ffffffff8384f770 <pv_ops+0xf0>         # data-type: (stack operation)                                                  ▒
        │      lock    andb    $0xdf,0x2(%r14)          # data-type: struct task_struct +0x2 (thread_info.flags)                                                        ▒
        │      mov     (%r14),%rax              # data-type: struct task_struct +0 (thread_info.flags) 


Some suggestions:

Align the # data-type: annotations.

Remove the "data-type: " part, just uses space and it should be obvious
what that type refers to.

At some point, if we can do it super cheaply, maybe BTF, do this by
default.

The 'T' hotkey looks great, but perhaps we should have more visibility
for it? Like what I suggested in:

https://lore.kernel.org/all/aBvx-J-ISifmw0NS@google.com/T/#u

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-annotate.txt |  1 -
>  tools/perf/builtin-annotate.c              |  5 ---
>  tools/perf/ui/browsers/annotate.c          |  6 +++
>  tools/perf/util/annotate.c                 | 47 ++++++++++++++++++++--
>  4 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> index 46090c5b42b4762f..547f1a2680185e3c 100644
> --- a/tools/perf/Documentation/perf-annotate.txt
> +++ b/tools/perf/Documentation/perf-annotate.txt
> @@ -170,7 +170,6 @@ include::itrace.txt[]
>  
>  --code-with-type::
>  	Show data type info in code annotation (for memory instructions only).
> -	Currently it only works with --stdio option.
>  
>  
>  SEE ALSO
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 5d57d2913f3d9a33..646f43b0f7c4c9b0 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
>  		symbol_conf.annotate_data_sample = true;
>  	} else if (annotate_opts.code_with_type) {
>  		symbol_conf.annotate_data_member = true;
> -
> -		if (!annotate.use_stdio) {
> -			pr_err("--code-with-type only works with --stdio.\n");
> -			goto out_delete;
> -		}
>  	}
>  
>  	setup_browser(true);
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 3e8b111e3f12b030..e5b35336f0d33d7e 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -4,6 +4,7 @@
>  #include "../ui.h"
>  #include "../../util/annotate.h"
>  #include "../../util/debug.h"
> +#include "../../util/debuginfo.h"
>  #include "../../util/dso.h"
>  #include "../../util/hist.h"
>  #include "../../util/sort.h"
> @@ -1119,6 +1120,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>  
>  	ui_helpline__push("Press ESC to exit");
>  
> +	if (annotate_opts.code_with_type)
> +		browser.dbg = debuginfo__new(dso__long_name(dso));
> +
>  	browser.b.width = notes->src->widths.max_line_len;
>  	browser.b.nr_entries = notes->src->nr_entries;
>  	browser.b.entries = &notes->src->source;
> @@ -1129,6 +1133,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>  
>  	ret = annotate_browser__run(&browser, evsel, hbt);
>  
> +	if (annotate_opts.code_with_type)
> +		debuginfo__delete(browser.dbg);
>  	if (not_annotated && !notes->src->tried_source)
>  		annotated_source__purge(notes->src);
>  
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 698bc4f559e83043..99e976d254493de2 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
>  	};
>  	struct annotation_line *al;
>  
> +	if (annotate_opts.code_with_type) {
> +		evsel__get_arch(apd->evsel, &apd->arch);
> +		apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
> +	}
> +
>  	list_for_each_entry(al, &notes->src->source, node) {
>  		if (annotation_line__filter(al))
>  			continue;
> @@ -1370,6 +1375,9 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
>  		wops.first_line = false;
>  	}
>  
> +	if (annotate_opts.code_with_type)
> +		debuginfo__delete(apd->dbg);
> +
>  	return 0;
>  }
>  
> @@ -1935,6 +1943,36 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
>  	return -ENOMEM;
>  }
>  
> +static int disasm_line__snprint_type_info(struct disasm_line *dl,
> +					  char *buf, int len,
> +					  struct annotation_print_data *apd)
> +{
> +	struct annotated_data_type *data_type;
> +	char member[256];
> +	int offset = 0;
> +	int printed;
> +
> +	scnprintf(buf, len, " ");
> +
> +	if (!annotate_opts.code_with_type || apd->dbg == NULL)
> +		return 1;
> +
> +	data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
> +	if (data_type == NULL || data_type == NO_TYPE)
> +		return 1;
> +
> +	printed = scnprintf(buf, len, "\t\t# data-type: %s", data_type->self.type_name);
> +
> +	if (data_type != &stackop_type && data_type != &canary_type && len > printed)
> +		printed += scnprintf(buf + printed, len - printed, " +%#x", offset);
> +
> +	if (annotated_data_type__get_member_name(data_type, member, sizeof(member), offset) &&
> +	    len > printed) {
> +		printed += scnprintf(buf + printed, len - printed, " (%s)", member);
> +	}
> +	return printed;
> +}
> +
>  void annotation_line__write(struct annotation_line *al, struct annotation *notes,
>  			    const struct annotation_write_ops *wops,
>  			    struct annotation_print_data *apd)
> @@ -2118,11 +2156,14 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>  
>  		width -= printed;
>  
> -		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
> +		printed = disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf),
> +					     obj__printf, obj__write_graph);
>  
> -		obj__printf(obj, "%-*s", width, bf);
> +		obj__printf(obj, "%s", bf);
> +		width -= printed;
>  
> -		(void)apd;
> +		disasm_line__snprint_type_info(disasm_line(al), bf, sizeof(bf), apd);
> +		obj__printf(obj, "%-*s", width, bf);
>  	}
>  
>  }
> -- 
> 2.50.1
Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
Posted by Namhyung Kim 1 month, 2 weeks ago
Hi Arnaldo,

On Wed, Aug 20, 2025 at 03:54:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 15, 2025 at 08:16:30PM -0700, Namhyung Kim wrote:
> > Until now, the --code-with-type option is available only on stdio.
> > But it was an artifical limitation because of an implemention issue.
> > 
> > Implement the same logic in annotation_line__write() for stdio2/TUI
> > and remove the limitation and update the man page.
> 
> Samples: 54K of event 'cycles:P', 4000 Hz, Event count (approx.): 25749717559
> poll_idle  /usr/lib/debug/lib/modules/6.15.9-201.fc42.x86_64/vmlinux [Percent: local period]
> Percent │      mov     %rdi,%rbx                                                                                                                                        ▒
>         │    → call    local_clock_noinstr              # data-type: (stack operation)                                                                                  ▒
>         │      andb    $0xfb,(%rbx)             # data-type: struct cpuidle_device +0 (registered:1)                                                                    ▒
>         │      mov     %rax,%rbp                                                                                                                                        ▒
>         │    → call    *0x1342a22(%rip)        # ffffffff8384f778 <pv_ops+0xf8>         # data-type: (stack operation)                                                  ▒
>         │      mov     current_task,%r14                # data-type: struct task_struct* +0                                                                             ▒
>         │      lock    orb     $0x20,0x2(%r14)          # data-type: struct task_struct +0x2 (thread_info.flags)                                                        ▒
>         │      mov     (%r14),%rax              # data-type: struct task_struct +0 (thread_info.flags)                                                                  ▒
>         │      test    $0x8,%al                                                                                                                                         ▒
>         │    ↓ jne     6d                                                                                                                                               ▒
>         │      mov     %r12,%rdi                                                                                                                                        ▒
>         │      mov     %rbx,%rsi                                                                                                                                        ▒
>         │    → call    cpuidle_poll_time                # data-type: (stack operation)                                                                                  ▒
>         │      mov     %rax,%r12                                                                                                                                        ▒
>         │49:   mov     $0xc9,%eax                                                                                                                                       ▒
>    0.75 │4e:   mov     (%r14),%rdx              # data-type: struct task_struct +0 (thread_info.flags)                                                                  ▒
>    0.42 │      and     $0x8,%edx                                                                                                                                        ▒
>         │    ↓ jne     6d                                                                                                                                               ▒
>   97.81 │      pause                                                                                                                                                    ▒
>    0.70 │      sub     $0x1,%eax                                                                                                                                        ▒
>    0.04 │    ↑ jne     4e                                                                                                                                               ▒
>    0.14 │    → call    local_clock_noinstr              # data-type: (stack operation)                                                                                  ▒
>    0.01 │      sub     %rbp,%rax                                                                                                                                        ▒
>    0.05 │      cmp     %rax,%r12                                                                                                                                        ▒
>         │    ↑ jae     49                                                                                                                                               ▒
>         │      orb     $0x4,(%rbx)              # data-type: struct cpuidle_device +0 (registered:1)                                                                    ▒
>         │6d: → call    *0x13429cd(%rip)        # ffffffff8384f770 <pv_ops+0xf0>         # data-type: (stack operation)                                                  ▒
>         │      lock    andb    $0xdf,0x2(%r14)          # data-type: struct task_struct +0x2 (thread_info.flags)                                                        ▒
>         │      mov     (%r14),%rax              # data-type: struct task_struct +0 (thread_info.flags) 
> 
> 
> Some suggestions:
> 
> Align the # data-type: annotations.

I was thinking about this but then probably it needs to align to the
longest line like call instruction.  It can be annoying if you are
using a small terminal.

> 
> Remove the "data-type: " part, just uses space and it should be obvious
> what that type refers to.

I think it's better to have some kind of signature to find the info
deterministically as I need to extract them in a script.

> 
> At some point, if we can do it super cheaply, maybe BTF, do this by
> default.

BTF doesn't have variables and locations so it's impossible to match
instructions to types currently.

> 
> The 'T' hotkey looks great, but perhaps we should have more visibility
> for it? Like what I suggested in:
> 
> https://lore.kernel.org/all/aBvx-J-ISifmw0NS@google.com/T/#u

Yep, in the next patch. :)

Thanks,
Namhyung