| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aKYZ-dhGJvWoIres@x1>
Date: Wed, 20 Aug 2025 15:54:49 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for
TUI
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@...nel.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
Powered by blists - more mailing lists