From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>,
Kan Liang <kan.liang@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
Date: Wed, 20 Aug 2025 15:54:49 -0300 [thread overview]
Message-ID: <aKYZ-dhGJvWoIres@x1> (raw)
In-Reply-To: <20250816031635.25318-8-namhyung@kernel.org>
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 = ¬es->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, ¬es->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
next prev parent reply other threads:[~2025-08-20 18:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 01/12] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 02/12] perf annotate: Remove annotation_print_data.start Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 03/12] perf annotate: Remove __annotation_line__write() Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 04/12] perf annotate: Pass annotation_print_data to annotation_line__write() Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 05/12] perf annotate: Simplify width calculation in annotation_line__write() Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 06/12] perf annotate: Return printed number from disasm_line__write() Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI Namhyung Kim
2025-08-20 18:54 ` Arnaldo Carvalho de Melo [this message]
2025-08-20 23:38 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 08/12] perf annotate: Add 'T' hot key to toggle data type display Namhyung Kim
2025-08-20 21:13 ` Arnaldo Carvalho de Melo
2025-08-20 23:43 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 09/12] perf annotate: Show warning when debuginfo is not available Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 10/12] perf annotate: Hide data-type for stack operation and canary Namhyung Kim
2025-08-20 21:28 ` Arnaldo Carvalho de Melo
2025-08-20 21:30 ` Arnaldo Carvalho de Melo
2025-08-16 3:16 ` [PATCH v5 11/12] perf annotate: Add dso__debuginfo() helper Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 12/12] perf annotate: Use a hashmap to save type data Namhyung Kim
2025-08-20 21:37 ` Arnaldo Carvalho de Melo
2025-08-20 23:46 ` Namhyung Kim
2025-08-20 21:40 ` [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Arnaldo Carvalho de Melo
2025-08-20 21:43 ` Arnaldo Carvalho de Melo
2025-08-20 23:47 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aKYZ-dhGJvWoIres@x1 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).