* [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI
@ 2025-08-16 3:16 Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 01/12] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
` (12 more replies)
0 siblings, 13 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
Hello,
The --code-with-type option is to show normal annotate output with type
information. It was supported only on --stdio, but this change adds it
to TUI as well.
Arnaldo, please tell me if you still don't like the
__hist_entry__tui_annotate() but I don't have a better idea for now.
v5 changes)
* use a copy of hist entry for perf top (Ian)
* split disasm_line__write() change (Ian)
* constify annotation_write_ops parameter (Ian)
* update printed length calculation (Ian)
* remove annotation_print_data.start
* add a hashmap to skip duplicate processing
v4 changes)
* add dso__debuginfo() helper (Ian)
v3 changes)
* hide stack operation and stack canary by default
v2 changes)
* use 'T' key to toggle data type display (Arnaldo)
* display '[Type]' in the title line when it's enabled (Arnaldo)
* show warning when debug info is not available (Arnaldo)
* fix a typo (Arnaldo)
Actually the command line option sets the default behavior and users can
change it by pressing 'T' key in the TUI annotate browser.
The code is also available at 'perf/annotate-code-type-tui-v5' branch at
https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (12):
perf annotate: Rename to __hist_entry__tui_annotate()
perf annotate: Remove annotation_print_data.start
perf annotate: Remove __annotation_line__write()
perf annotate: Pass annotation_print_data to annotation_line__write()
perf annotate: Simplify width calculation in annotation_line__write()
perf annotate: Return printed number from disasm_line__write()
perf annotate: Add --code-with-type support for TUI
perf annotate: Add 'T' hot key to toggle data type display
perf annotate: Show warning when debuginfo is not available
perf annotate: Hide data-type for stack operation and canary
perf annotate: Add dso__debuginfo() helper
perf annotate: Use a hashmap to save type data
tools/perf/Documentation/perf-annotate.txt | 1 -
tools/perf/builtin-annotate.c | 5 -
tools/perf/ui/browsers/annotate.c | 117 ++++++++++++--
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/util/annotate.c | 178 +++++++++++++++------
tools/perf/util/annotate.h | 29 ++--
tools/perf/util/dso.h | 21 +++
tools/perf/util/hist.h | 12 +-
8 files changed, 273 insertions(+), 92 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 01/12] perf annotate: Rename to __hist_entry__tui_annotate()
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
@ 2025-08-16 3:16 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 02/12] perf annotate: Remove annotation_print_data.start Namhyung Kim
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
There are three different but similar functions for annotation on TUI.
Rename it to __hist_entry__tui_annotate() and make sure it passes 'he'.
It's not used for now but it'll be needed for later use.
Also remove map_symbol__tui_annotate() which was a simple wrapper.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 40 +++++++++++++++++++++++--------
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/util/annotate.h | 12 ----------
tools/perf/util/hist.h | 12 ++++++----
4 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 183902dac042ecb0..4d5cbb86cbb6d506 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -12,6 +12,7 @@
#include "../../util/symbol.h"
#include "../../util/evsel.h"
#include "../../util/evlist.h"
+#include "../../util/thread.h"
#include <inttypes.h>
#include <linux/kernel.h>
#include <linux/string.h>
@@ -27,10 +28,18 @@ struct annotate_browser {
struct rb_node *curr_hot;
struct annotation_line *selection;
struct arch *arch;
+ /*
+ * perf top can delete hist_entry anytime. Callers should make sure
+ * its lifetime.
+ */
+ struct hist_entry *he;
bool searching_backwards;
char search_bf[128];
};
+/* A copy of target hist_entry for perf top. */
+static struct hist_entry annotate_he;
+
static inline struct annotation *browser__annotation(struct ui_browser *browser)
{
struct map_symbol *ms = browser->priv;
@@ -557,7 +566,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
target_ms.map = ms->map;
target_ms.sym = dl->ops.target.sym;
annotation__unlock(notes);
- symbol__tui_annotate(&target_ms, evsel, hbt);
+ __hist_entry__tui_annotate(browser->he, &target_ms, evsel, hbt);
sym_title(ms->sym, ms->map, title, sizeof(title), annotate_opts.percent_type);
ui_browser__show_title(&browser->b, title);
return true;
@@ -1032,12 +1041,6 @@ static int annotate_browser__run(struct annotate_browser *browser,
return key;
}
-int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt)
-{
- return symbol__tui_annotate(ms, evsel, hbt);
-}
-
int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
struct hist_browser_timer *hbt)
{
@@ -1046,11 +1049,12 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
SLang_init_tty(0, 0, 0);
SLtty_set_suspend_state(true);
- return map_symbol__tui_annotate(&he->ms, evsel, hbt);
+ return __hist_entry__tui_annotate(he, &he->ms, evsel, hbt);
}
-int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt)
+int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
+ struct evsel *evsel,
+ struct hist_browser_timer *hbt)
{
struct symbol *sym = ms->sym;
struct annotation *notes = symbol__annotation(sym);
@@ -1064,6 +1068,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
.priv = ms,
.use_navkeypressed = true,
},
+ .he = he,
};
struct dso *dso;
int ret = -1, err;
@@ -1093,6 +1098,16 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
}
}
+ /* Copy necessary information when it's called from perf top */
+ if (hbt != NULL && he != &annotate_he) {
+ annotate_he.hists = he->hists;
+ annotate_he.thread = thread__get(he->thread);
+ annotate_he.cpumode = he->cpumode;
+ map_symbol__copy(&annotate_he.ms, ms);
+
+ browser.he = &annotate_he;
+ }
+
ui_helpline__push("Press ESC to exit");
browser.b.width = notes->src->widths.max_line_len;
@@ -1108,5 +1123,10 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
if (not_annotated && !notes->src->tried_source)
annotated_source__purge(notes->src);
+ if (hbt != NULL && he != &annotate_he) {
+ thread__zput(annotate_he.thread);
+ map_symbol__exit(&annotate_he.ms);
+ }
+
return ret;
}
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d9d3fb44477ac6d5..487c0b08c0038710 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2484,8 +2484,8 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
else
evsel = hists_to_evsel(browser->hists);
- err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
he = hist_browser__selected_entry(browser);
+ err = __hist_entry__tui_annotate(he, &act->ms, evsel, browser->hbt);
/*
* offer option to annotate the other branch source or target
* (if they exists) when returning from annotate
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 8b5131d257b01e3e..0f640e4871744262 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -471,18 +471,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel);
int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel);
int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel);
-#ifdef HAVE_SLANG_SUPPORT
-int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt);
-#else
-static inline int symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
- struct evsel *evsel __maybe_unused,
- struct hist_browser_timer *hbt __maybe_unused)
-{
- return 0;
-}
-#endif
-
void annotation_options__init(void);
void annotation_options__exit(void);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 70438d03ca9c33b1..c64005278687cb02 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -713,8 +713,9 @@ struct block_hist {
#include "../ui/keysyms.h"
void attr_to_script(char *buf, struct perf_event_attr *attr);
-int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
- struct hist_browser_timer *hbt);
+int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
+ struct evsel *evsel,
+ struct hist_browser_timer *hbt);
int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
struct hist_browser_timer *hbt);
@@ -742,9 +743,10 @@ int evlist__tui_browse_hists(struct evlist *evlist __maybe_unused,
{
return 0;
}
-static inline int map_symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
- struct evsel *evsel __maybe_unused,
- struct hist_browser_timer *hbt __maybe_unused)
+static inline int __hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
+ struct map_symbol *ms __maybe_unused,
+ struct evsel *evsel __maybe_unused,
+ struct hist_browser_timer *hbt __maybe_unused)
{
return 0;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 02/12] perf annotate: Remove annotation_print_data.start
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 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 03/12] perf annotate: Remove __annotation_line__write() Namhyung Kim
` (10 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
The start field is to control whether the output shows full address or
offset from the function start. But actually it can be changed
dynamically in annotation__toggle_full_addr(). The informaiton should
be available through struct annotation. Let's use it directly.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 0dd475a744b6dfac..b699e14102b0587a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -770,7 +770,6 @@ struct annotation_print_data {
struct evsel *evsel;
struct arch *arch;
struct debuginfo *dbg;
- u64 start;
int addr_fmt_width;
};
@@ -845,7 +844,7 @@ annotation_line__print(struct annotation_line *al, struct annotation_print_data
printf(" : ");
- disasm_line__print(dl, apd->start, apd->addr_fmt_width);
+ disasm_line__print(dl, notes->src->start, apd->addr_fmt_width);
if (opts->code_with_type && apd->dbg) {
struct annotated_data_type *data_type;
@@ -1230,7 +1229,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
struct annotation_print_data apd = {
.he = he,
.evsel = evsel,
- .start = map__rip_2objdump(map, sym->start),
};
int printed = 2, queue_len = 0;
int more = 0;
@@ -1267,7 +1265,7 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
symbol__annotate_hits(sym, evsel);
apd.addr_fmt_width = annotated_source__addr_fmt_width(¬es->src->source,
- apd.start);
+ notes->src->start);
evsel__get_arch(evsel, &apd.arch);
apd.dbg = debuginfo__new(filename);
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 03/12] perf annotate: Remove __annotation_line__write()
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 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 04/12] perf annotate: Pass annotation_print_data to annotation_line__write() Namhyung Kim
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
Get rid of the internal function and convert function arguments into
local variables if they are used more than once.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 46 ++++++++++++++++----------------------
tools/perf/util/annotate.h | 2 +-
2 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b699e14102b0587a..7929f108e35b4e65 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1933,24 +1933,26 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
return -ENOMEM;
}
-static void __annotation_line__write(struct annotation_line *al, struct annotation *notes,
- bool first_line, bool current_entry, bool change_color, int width,
- void *obj, unsigned int percent_type,
- int (*obj__set_color)(void *obj, int color),
- void (*obj__set_percent_color)(void *obj, double percent, bool current),
- int (*obj__set_jumps_percent_color)(void *obj, int nr, bool current),
- void (*obj__printf)(void *obj, const char *fmt, ...),
- void (*obj__write_graph)(void *obj, int graph))
-
-{
- double percent_max = annotation_line__max_percent(al, percent_type);
- int pcnt_width = annotation__pcnt_width(notes),
- cycles_width = annotation__cycles_width(notes);
+void annotation_line__write(struct annotation_line *al, struct annotation *notes,
+ const struct annotation_write_ops *wops)
+{
+ bool current_entry = wops->current_entry;
+ bool change_color = wops->change_color;
+ double percent_max = annotation_line__max_percent(al, annotate_opts.percent_type);
+ int width = wops->width;
+ int pcnt_width = annotation__pcnt_width(notes);
+ int cycles_width = annotation__cycles_width(notes);
bool show_title = false;
char bf[256];
int printed;
-
- if (first_line && (al->offset == -1 || percent_max == 0.0)) {
+ void *obj = wops->obj;
+ int (*obj__set_color)(void *obj, int color) = wops->set_color;
+ void (*obj__set_percent_color)(void *obj, double percent, bool current) = wops->set_percent_color;
+ int (*obj__set_jumps_percent_color)(void *obj, int nr, bool current) = wops->set_jumps_percent_color;
+ void (*obj__printf)(void *obj, const char *fmt, ...) = wops->printf;
+ void (*obj__write_graph)(void *obj, int graph) = wops->write_graph;
+
+ if (wops->first_line && (al->offset == -1 || percent_max == 0.0)) {
if (notes->branch && al->cycles) {
if (al->cycles->ipc == 0.0 && al->cycles->avg == 0)
show_title = true;
@@ -1964,7 +1966,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
for (i = 0; i < al->data_nr; i++) {
double percent;
- percent = annotation_data__percent(&al->data[i], percent_type);
+ percent = annotation_data__percent(&al->data[i],
+ annotate_opts.percent_type);
obj__set_percent_color(obj, percent, current_entry);
if (symbol_conf.show_total_period) {
@@ -2113,17 +2116,6 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
}
-void annotation_line__write(struct annotation_line *al, struct annotation *notes,
- struct annotation_write_ops *wops)
-{
- __annotation_line__write(al, notes, wops->first_line, wops->current_entry,
- wops->change_color, wops->width, wops->obj,
- annotate_opts.percent_type,
- wops->set_color, wops->set_percent_color,
- wops->set_jumps_percent_color, wops->printf,
- wops->write_graph);
-}
-
int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
struct arch **parch)
{
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 0f640e4871744262..2e28a24fa13a736a 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -200,7 +200,7 @@ struct annotation_write_ops {
};
void annotation_line__write(struct annotation_line *al, struct annotation *notes,
- struct annotation_write_ops *ops);
+ const struct annotation_write_ops *ops);
int __annotation__scnprintf_samples_period(struct annotation *notes,
char *bf, size_t size,
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 04/12] perf annotate: Pass annotation_print_data to annotation_line__write()
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (2 preceding siblings ...)
2025-08-16 3:16 ` [PATCH v5 03/12] perf annotate: Remove __annotation_line__write() Namhyung Kim
@ 2025-08-16 3:16 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 05/12] perf annotate: Simplify width calculation in annotation_line__write() Namhyung Kim
` (8 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
It will be used for data type display later.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 13 ++++++++++--
tools/perf/util/annotate.c | 33 ++++++++++++++++++-------------
tools/perf/util/annotate.h | 15 ++++++++++++--
3 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4d5cbb86cbb6d506..3e8b111e3f12b030 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -33,6 +33,8 @@ struct annotate_browser {
* its lifetime.
*/
struct hist_entry *he;
+ struct debuginfo *dbg;
+ struct evsel *evsel;
bool searching_backwards;
char search_bf[128];
};
@@ -116,12 +118,18 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
.printf = annotate_browser__printf,
.write_graph = annotate_browser__write_graph,
};
+ struct annotation_print_data apd = {
+ .he = ab->he,
+ .arch = ab->arch,
+ .evsel = ab->evsel,
+ .dbg = ab->dbg,
+ };
/* The scroll bar isn't being used */
if (!browser->navkeypressed)
ops.width += 1;
- annotation_line__write(al, notes, &ops);
+ annotation_line__write(al, notes, &ops, &apd);
if (ops.current_entry)
ab->selection = al;
@@ -984,7 +992,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
continue;
}
case 'P':
- map_symbol__annotation_dump(ms, evsel);
+ map_symbol__annotation_dump(ms, evsel, browser->he);
continue;
case 't':
if (symbol_conf.show_total_period) {
@@ -1069,6 +1077,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
.use_navkeypressed = true,
},
.he = he,
+ .evsel = evsel,
};
struct dso *dso;
int ret = -1, err;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 7929f108e35b4e65..2544d83a52a0a596 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -765,14 +765,6 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
struct debuginfo *dbg, struct disasm_line *dl,
int *type_offset);
-struct annotation_print_data {
- struct hist_entry *he;
- struct evsel *evsel;
- struct arch *arch;
- struct debuginfo *dbg;
- int addr_fmt_width;
-};
-
static int
annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
struct annotation_options *opts, int printed,
@@ -1355,7 +1347,8 @@ static void FILE__write_graph(void *fp, int graph)
fputs(s, fp);
}
-static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
+static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
+ struct annotation_print_data *apd)
{
struct annotation *notes = symbol__annotation(sym);
struct annotation_write_ops wops = {
@@ -1372,7 +1365,7 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
list_for_each_entry(al, ¬es->src->source, node) {
if (annotation_line__filter(al))
continue;
- annotation_line__write(al, notes, &wops);
+ annotation_line__write(al, notes, &wops, apd);
fputc('\n', fp);
wops.first_line = false;
}
@@ -1380,13 +1373,18 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
return 0;
}
-int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel)
+int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel,
+ struct hist_entry *he)
{
const char *ev_name = evsel__name(evsel);
char buf[1024];
char *filename;
int err = -1;
FILE *fp;
+ struct annotation_print_data apd = {
+ .he = he,
+ .evsel = evsel,
+ };
if (asprintf(&filename, "%s.annotation", ms->sym->name) < 0)
return -1;
@@ -1402,7 +1400,7 @@ int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel)
fprintf(fp, "%s() %s\nEvent: %s\n\n",
ms->sym->name, dso__long_name(map__dso(ms->map)), ev_name);
- symbol__annotate_fprintf2(ms->sym, fp);
+ symbol__annotate_fprintf2(ms->sym, fp, &apd);
fclose(fp);
err = 0;
@@ -1654,6 +1652,10 @@ int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
struct symbol *sym = ms->sym;
struct rb_root source_line = RB_ROOT;
struct hists *hists = evsel__hists(evsel);
+ struct annotation_print_data apd = {
+ .he = he,
+ .evsel = evsel,
+ };
char buf[1024];
int err;
@@ -1676,7 +1678,7 @@ int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
hists__scnprintf_title(hists, buf, sizeof(buf));
fprintf(stdout, "%s, [percent: %s]\n%s() %s\n",
buf, percent_type_str(annotate_opts.percent_type), sym->name, dso__long_name(dso));
- symbol__annotate_fprintf2(sym, stdout);
+ symbol__annotate_fprintf2(sym, stdout, &apd);
annotated_source__purge(symbol__annotation(sym)->src);
@@ -1934,7 +1936,8 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
}
void annotation_line__write(struct annotation_line *al, struct annotation *notes,
- const struct annotation_write_ops *wops)
+ const struct annotation_write_ops *wops,
+ struct annotation_print_data *apd)
{
bool current_entry = wops->current_entry;
bool change_color = wops->change_color;
@@ -2112,6 +2115,8 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
obj__printf(obj, "%-*s", width - pcnt_width - cycles_width - 3 - printed, bf);
+
+ (void)apd;
}
}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 2e28a24fa13a736a..86e858f5bf173152 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -199,8 +199,18 @@ struct annotation_write_ops {
void (*write_graph)(void *obj, int graph);
};
+struct annotation_print_data {
+ struct hist_entry *he;
+ struct evsel *evsel;
+ struct arch *arch;
+ struct debuginfo *dbg;
+ /* It'll be set in hist_entry__annotate_printf() */
+ int addr_fmt_width;
+};
+
void annotation_line__write(struct annotation_line *al, struct annotation *notes,
- const struct annotation_write_ops *ops);
+ const struct annotation_write_ops *ops,
+ struct annotation_print_data *apd);
int __annotation__scnprintf_samples_period(struct annotation *notes,
char *bf, size_t size,
@@ -463,7 +473,8 @@ void symbol__annotate_zero_histogram(struct symbol *sym, struct evsel *evsel);
void symbol__annotate_decay_histogram(struct symbol *sym, struct evsel *evsel);
void annotated_source__purge(struct annotated_source *as);
-int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel);
+int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel,
+ struct hist_entry *he);
bool ui__has_annotation(void);
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 05/12] perf annotate: Simplify width calculation in annotation_line__write()
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (3 preceding siblings ...)
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 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 06/12] perf annotate: Return printed number from disasm_line__write() Namhyung Kim
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
The width is updated after each part is printed. It can skip the output
processing if the total printed size is bigger than the width.
No function changes intended.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 2544d83a52a0a596..6389292ad8f95d89 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1993,6 +1993,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
symbol_conf.show_nr_samples ? "Samples" : "Percent");
}
}
+ width -= pcnt_width;
if (notes->branch) {
if (al->cycles && al->cycles->ipc)
@@ -2056,11 +2057,13 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
obj__printf(obj, "%*s", ANNOTATION__AVG_IPC_WIDTH, bf);
}
}
+ width -= cycles_width;
obj__printf(obj, " ");
+ width -= 1;
if (!*al->line)
- obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
+ obj__printf(obj, "%-*s", width, " ");
else if (al->offset == -1) {
if (al->line_nr && annotate_opts.show_linenr)
printed = scnprintf(bf, sizeof(bf), "%-*d ",
@@ -2069,7 +2072,8 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
printed = scnprintf(bf, sizeof(bf), "%-*s ",
notes->src->widths.addr, " ");
obj__printf(obj, bf);
- obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
+ width -= printed;
+ obj__printf(obj, "%-*s", width, al->line);
} else {
u64 addr = al->offset;
int color = -1;
@@ -2112,9 +2116,11 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
if (change_color)
obj__set_color(obj, color);
+ width -= printed;
+
disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
- obj__printf(obj, "%-*s", width - pcnt_width - cycles_width - 3 - printed, bf);
+ obj__printf(obj, "%-*s", width, bf);
(void)apd;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 06/12] perf annotate: Return printed number from disasm_line__write()
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (4 preceding siblings ...)
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 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI Namhyung Kim
` (6 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
Like other print functions, make disasm_line__write() return the number
of printed characters. It'll be used to skip unnecessary operations
when the screen is full.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 6389292ad8f95d89..698bc4f559e83043 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1743,7 +1743,7 @@ static double annotation_line__max_percent(struct annotation_line *al,
return percent_max;
}
-static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
+static int disasm_line__write(struct disasm_line *dl, struct annotation *notes,
void *obj, char *bf, size_t size,
void (*obj__printf)(void *obj, const char *fmt, ...),
void (*obj__write_graph)(void *obj, int graph))
@@ -1771,8 +1771,8 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
obj__printf(obj, " ");
}
- disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
- notes->src->widths.max_ins_name);
+ return disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
+ notes->src->widths.max_ins_name) + 2;
}
static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (5 preceding siblings ...)
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 ` Namhyung Kim
2025-08-20 18:54 ` Arnaldo Carvalho de Melo
2025-08-16 3:16 ` [PATCH v5 08/12] perf annotate: Add 'T' hot key to toggle data type display Namhyung Kim
` (5 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
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 = ¬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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 08/12] perf annotate: Add 'T' hot key to toggle data type display
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (6 preceding siblings ...)
2025-08-16 3:16 ` [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI Namhyung Kim
@ 2025-08-16 3:16 ` Namhyung Kim
2025-08-20 21:13 ` Arnaldo Carvalho de Melo
2025-08-16 3:16 ` [PATCH v5 09/12] perf annotate: Show warning when debuginfo is not available Namhyung Kim
` (4 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
Support data type display with a key press so that users can toggle the
output dynamically on TUI. Also display "[Type]" in the title line if
it's enabled.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index e5b35336f0d33d7e..dfe869c20e35da77 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -533,9 +533,10 @@ static void ui_browser__init_asm_mode(struct ui_browser *browser)
static int sym_title(struct symbol *sym, struct map *map, char *title,
size_t sz, int percent_type)
{
- return snprintf(title, sz, "%s %s [Percent: %s]", sym->name,
+ return snprintf(title, sz, "%s %s [Percent: %s] %s", sym->name,
dso__long_name(map__dso(map)),
- percent_type_str(percent_type));
+ percent_type_str(percent_type),
+ annotate_opts.code_with_type ? "[Type]" : "");
}
/*
@@ -909,7 +910,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
"b Toggle percent base [period/hits]\n"
"B Branch counter abbr list (Optional)\n"
"? Search string backwards\n"
- "f Toggle showing offsets to full address\n");
+ "f Toggle showing offsets to full address\n"
+ "T Toggle data type display\n");
continue;
case 'r':
script_browse(NULL, NULL);
@@ -1029,6 +1031,12 @@ static int annotate_browser__run(struct annotate_browser *browser,
case 'f':
annotation__toggle_full_addr(notes, ms);
continue;
+ case 'T':
+ annotate_opts.code_with_type ^= 1;
+ if (browser->dbg == NULL)
+ browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
+ annotate_browser__show(&browser->b, title, help);
+ continue;
case K_LEFT:
case '<':
case '>':
@@ -1133,8 +1141,7 @@ 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);
+ debuginfo__delete(browser.dbg);
if (not_annotated && !notes->src->tried_source)
annotated_source__purge(notes->src);
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 09/12] perf annotate: Show warning when debuginfo is not available
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (7 preceding siblings ...)
2025-08-16 3:16 ` [PATCH v5 08/12] perf annotate: Add 'T' hot key to toggle data type display Namhyung Kim
@ 2025-08-16 3:16 ` Namhyung Kim
2025-08-16 3:16 ` [PATCH v5 10/12] perf annotate: Hide data-type for stack operation and canary Namhyung Kim
` (3 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
When user requests data-type annotation but no DWARF info is available,
show a warning message about it.
Warning:
DWARF debuginfo not found.
Data-type in this DSO will not be displayed.
Please make sure to have debug information.
Press any key...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 17 +++++++++++++++++
tools/perf/util/dso.h | 11 +++++++++++
2 files changed, 28 insertions(+)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index dfe869c20e35da77..e3e7004f32251666 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -812,6 +812,20 @@ static int annotate__scnprintf_title(struct hists *hists, char *bf, size_t size)
return printed;
}
+static void annotate_browser__debuginfo_warning(struct annotate_browser *browser)
+{
+ struct map_symbol *ms = browser->b.priv;
+ struct dso *dso = map__dso(ms->map);
+
+ if (browser->dbg == NULL && annotate_opts.code_with_type &&
+ !dso__debuginfo_warned(dso)) {
+ ui__warning("DWARF debuginfo not found.\n\n"
+ "Data-type in this DSO will not be displayed.\n"
+ "Please make sure to have debug information.");
+ dso__set_debuginfo_warned(dso);
+ }
+}
+
static int annotate_browser__run(struct annotate_browser *browser,
struct evsel *evsel,
struct hist_browser_timer *hbt)
@@ -842,6 +856,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
annotation_br_cntr_abbr_list(&br_cntr_text, evsel, false);
+ annotate_browser__debuginfo_warning(browser);
+
while (1) {
key = ui_browser__run(&browser->b, delay_secs);
@@ -1036,6 +1052,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
if (browser->dbg == NULL)
browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
annotate_browser__show(&browser->b, title, help);
+ annotate_browser__debuginfo_warning(browser);
continue;
case K_LEFT:
case '<':
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 3457d713d3c56df6..7df1673f08d3ddb4 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -299,6 +299,7 @@ DECLARE_RC_STRUCT(dso) {
u8 hit:1;
u8 annotate_warned:1;
u8 auxtrace_warned:1;
+ u8 debuginfo_warned:1;
u8 short_name_allocated:1;
u8 long_name_allocated:1;
u8 is_64_bit:1;
@@ -362,6 +363,16 @@ static inline void dso__set_annotate_warned(struct dso *dso)
RC_CHK_ACCESS(dso)->annotate_warned = 1;
}
+static inline bool dso__debuginfo_warned(const struct dso *dso)
+{
+ return RC_CHK_ACCESS(dso)->debuginfo_warned;
+}
+
+static inline void dso__set_debuginfo_warned(struct dso *dso)
+{
+ RC_CHK_ACCESS(dso)->debuginfo_warned = 1;
+}
+
static inline bool dso__auxtrace_warned(const struct dso *dso)
{
return RC_CHK_ACCESS(dso)->auxtrace_warned;
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 10/12] perf annotate: Hide data-type for stack operation and canary
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (8 preceding siblings ...)
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 ` Namhyung Kim
2025-08-20 21:28 ` Arnaldo Carvalho de Melo
2025-08-16 3:16 ` [PATCH v5 11/12] perf annotate: Add dso__debuginfo() helper Namhyung Kim
` (2 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
It's mostly unnecessary to print when it has no actual type information
like in the stack operations and canary. Let's have them if -v option
is given.
Before:
$ perf annotate --code-with-type
...
: 0 0xd640 <_dl_relocate_object>:
0.00 : 0: endbr64
0.00 : 4: pushq %rbp # data-type: (stack operation)
0.00 : 5: movq %rsp, %rbp
0.00 : 8: pushq %r15 # data-type: (stack operation)
0.00 : a: pushq %r14 # data-type: (stack operation)
0.00 : c: pushq %r13 # data-type: (stack operation)
0.00 : e: pushq %r12 # data-type: (stack operation)
0.00 : 10: pushq %rbx # data-type: (stack operation)
0.00 : 11: subq $0xf8, %rsp
...
0.00 : d4: testl %eax, %eax
0.00 : d6: jne 0xf424
0.00 : dc: movq 0xf0(%r14), %rbx # data-type: struct link_map +0xf0
0.00 : e3: testq %rbx, %rbx
0.00 : e6: jne 0xf2dd
0.00 : ec: cmpq $0, 0xf8(%r14) # data-type: struct link_map +0xf8
...
After:
: 0 0xd640 <_dl_relocate_object>:
0.00 : 0: endbr64
0.00 : 4: pushq %rbp
0.00 : 5: movq %rsp, %rbp
0.00 : 8: pushq %r15
0.00 : a: pushq %r14
0.00 : c: pushq %r13
0.00 : e: pushq %r12
0.00 : 10: pushq %rbx
0.00 : 11: subq $0xf8, %rsp
...
0.00 : d4: testl %eax, %eax
0.00 : d6: jne 0xf424
0.00 : dc: movq 0xf0(%r14), %rbx # data-type: struct link_map +0xf0
0.00 : e3: testq %rbx, %rbx
0.00 : e6: jne 0xf2dd
0.00 : ec: cmpq $0, 0xf8(%r14) # data-type: struct link_map +0xf8
...
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 99e976d254493de2..ea68b32da7ce584a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -765,6 +765,17 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
struct debuginfo *dbg, struct disasm_line *dl,
int *type_offset);
+static bool needs_type_info(struct annotated_data_type *data_type)
+{
+ if (data_type == NULL || data_type == NO_TYPE)
+ return false;
+
+ if (verbose)
+ return true;
+
+ return (data_type != &stackop_type) && (data_type != &canary_type);
+}
+
static int
annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
struct annotation_options *opts, int printed,
@@ -844,7 +855,7 @@ annotation_line__print(struct annotation_line *al, struct annotation_print_data
data_type = __hist_entry__get_data_type(apd->he, apd->arch,
apd->dbg, dl, &offset);
- if (data_type && data_type != NO_TYPE) {
+ if (needs_type_info(data_type)) {
char buf[4096];
printf("\t\t# data-type: %s",
@@ -1958,7 +1969,7 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
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)
+ if (!needs_type_info(data_type))
return 1;
printed = scnprintf(buf, len, "\t\t# data-type: %s", data_type->self.type_name);
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 11/12] perf annotate: Add dso__debuginfo() helper
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (9 preceding siblings ...)
2025-08-16 3:16 ` [PATCH v5 10/12] perf annotate: Hide data-type for stack operation and canary Namhyung Kim
@ 2025-08-16 3:16 ` 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:40 ` [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Arnaldo Carvalho de Melo
12 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
It'd be great if it can get the correct debug information using DSO
build-Id not just the path name. Instead of adding new callsites of
debuginfo__new(), let's add dso__debuginfo() which can hide the access
using the pathname and help the future conversion.
Suggested-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 4 ++--
tools/perf/util/annotate.c | 6 +++---
tools/perf/util/dso.h | 10 ++++++++++
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index e3e7004f32251666..9aa3c1ba22f52789 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -1050,7 +1050,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
case 'T':
annotate_opts.code_with_type ^= 1;
if (browser->dbg == NULL)
- browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
+ browser->dbg = dso__debuginfo(map__dso(ms->map));
annotate_browser__show(&browser->b, title, help);
annotate_browser__debuginfo_warning(browser);
continue;
@@ -1146,7 +1146,7 @@ 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.dbg = dso__debuginfo(dso);
browser.b.width = notes->src->widths.max_line_len;
browser.b.nr_entries = notes->src->nr_entries;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ea68b32da7ce584a..bea3457a00632fd7 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1270,7 +1270,7 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
apd.addr_fmt_width = annotated_source__addr_fmt_width(¬es->src->source,
notes->src->start);
evsel__get_arch(evsel, &apd.arch);
- apd.dbg = debuginfo__new(filename);
+ apd.dbg = dso__debuginfo(dso);
list_for_each_entry(pos, ¬es->src->source, node) {
int err;
@@ -1375,7 +1375,7 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
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)));
+ apd->dbg = dso__debuginfo(map__dso(apd->he->ms.map));
}
list_for_each_entry(al, ¬es->src->source, node) {
@@ -2882,7 +2882,7 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
di_cache.dso = dso__get(map__dso(ms->map));
debuginfo__delete(di_cache.dbg);
- di_cache.dbg = debuginfo__new(dso__long_name(di_cache.dso));
+ di_cache.dbg = dso__debuginfo(di_cache.dso);
}
if (di_cache.dbg == NULL) {
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 7df1673f08d3ddb4..fd8e95de77f78dfc 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -10,6 +10,7 @@
#include <stdio.h>
#include <linux/bitops.h>
#include "build-id.h"
+#include "debuginfo.h"
#include "mutex.h"
#include <internal/rc_check.h>
@@ -914,4 +915,13 @@ u64 dso__findnew_global_type(struct dso *dso, u64 addr, u64 offset);
bool perf_pid_map_tid(const char *dso_name, int *tid);
bool is_perf_pid_map_name(const char *dso_name);
+/*
+ * In the future, we may get debuginfo using build-ID (w/o path).
+ * Add this helper is for the smooth conversion.
+ */
+static inline struct debuginfo *dso__debuginfo(struct dso *dso)
+{
+ return debuginfo__new(dso__long_name(dso));
+}
+
#endif /* __PERF_DSO */
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v5 12/12] perf annotate: Use a hashmap to save type data
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (10 preceding siblings ...)
2025-08-16 3:16 ` [PATCH v5 11/12] perf annotate: Add dso__debuginfo() helper Namhyung Kim
@ 2025-08-16 3:16 ` Namhyung Kim
2025-08-20 21:37 ` Arnaldo Carvalho de Melo
2025-08-20 21:40 ` [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Arnaldo Carvalho de Melo
12 siblings, 1 reply; 24+ messages in thread
From: Namhyung Kim @ 2025-08-16 3:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
It can slowdown annotation browser if objdump is processing large DWARF
data. Let's add a hashmap to save the data type info for each line.
Note that this is needed for TUI only because stdio only processes each
line once. TUI will display the same line whenever it refreshes the
screen.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 34 ++++++++++++++++++++++++++++++-
tools/perf/util/annotate.c | 27 ++++++++++++++++++++++--
tools/perf/util/annotate.h | 2 ++
3 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 9aa3c1ba22f52789..9677a3763a290a3d 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -6,6 +6,7 @@
#include "../../util/debug.h"
#include "../../util/debuginfo.h"
#include "../../util/dso.h"
+#include "../../util/hashmap.h"
#include "../../util/hist.h"
#include "../../util/sort.h"
#include "../../util/map.h"
@@ -15,6 +16,7 @@
#include "../../util/evlist.h"
#include "../../util/thread.h"
#include <inttypes.h>
+#include <linux/err.h>
#include <linux/kernel.h>
#include <linux/string.h>
#include <linux/zalloc.h>
@@ -36,6 +38,7 @@ struct annotate_browser {
struct hist_entry *he;
struct debuginfo *dbg;
struct evsel *evsel;
+ struct hashmap *type_hash;
bool searching_backwards;
char search_bf[128];
};
@@ -43,6 +46,16 @@ struct annotate_browser {
/* A copy of target hist_entry for perf top. */
static struct hist_entry annotate_he;
+static size_t type_hash(long key, void *ctx __maybe_unused)
+{
+ return key;
+}
+
+static bool type_equal(long key1, long key2, void *ctx __maybe_unused)
+{
+ return key1 == key2;
+}
+
static inline struct annotation *browser__annotation(struct ui_browser *browser)
{
struct map_symbol *ms = browser->priv;
@@ -130,6 +143,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
if (!browser->navkeypressed)
ops.width += 1;
+ if (!IS_ERR_OR_NULL(ab->type_hash))
+ apd.type_hash = ab->type_hash;
+
annotation_line__write(al, notes, &ops, &apd);
if (ops.current_entry)
@@ -1051,6 +1067,10 @@ static int annotate_browser__run(struct annotate_browser *browser,
annotate_opts.code_with_type ^= 1;
if (browser->dbg == NULL)
browser->dbg = dso__debuginfo(map__dso(ms->map));
+ if (browser->type_hash == NULL) {
+ browser->type_hash = hashmap__new(type_hash, type_equal,
+ /*ctx=*/NULL);
+ }
annotate_browser__show(&browser->b, title, help);
annotate_browser__debuginfo_warning(browser);
continue;
@@ -1145,8 +1165,10 @@ 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)
+ if (annotate_opts.code_with_type) {
browser.dbg = dso__debuginfo(dso);
+ browser.type_hash = hashmap__new(type_hash, type_equal, /*ctx=*/NULL);
+ }
browser.b.width = notes->src->widths.max_line_len;
browser.b.nr_entries = notes->src->nr_entries;
@@ -1159,6 +1181,16 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
ret = annotate_browser__run(&browser, evsel, hbt);
debuginfo__delete(browser.dbg);
+
+ if (!IS_ERR_OR_NULL(browser.type_hash)) {
+ struct hashmap_entry *cur;
+ size_t bkt;
+
+ hashmap__for_each_entry(browser.type_hash, cur, bkt)
+ free(cur->pvalue);
+ hashmap__free(browser.type_hash);
+ }
+
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 bea3457a00632fd7..77414e04d99bb4f2 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1954,11 +1954,17 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
return -ENOMEM;
}
+struct type_hash_entry {
+ struct annotated_data_type *type;
+ int offset;
+};
+
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;
+ struct annotated_data_type *data_type = NULL;
+ struct type_hash_entry *entry = NULL;
char member[256];
int offset = 0;
int printed;
@@ -1968,7 +1974,24 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
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 (apd->type_hash) {
+ hashmap__find(apd->type_hash, dl->al.offset, &entry);
+ if (entry != NULL) {
+ data_type = entry->type;
+ offset = entry->offset;
+ }
+ }
+ if (data_type == NULL)
+ data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
+ if (apd->type_hash && entry == NULL) {
+ entry = malloc(sizeof(*entry));
+ if (entry != NULL) {
+ entry->type = data_type;
+ entry->offset = offset;
+ hashmap__add(apd->type_hash, dl->al.offset, entry);
+ }
+ }
+
if (!needs_type_info(data_type))
return 1;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 86e858f5bf173152..eaf6c8aa7f473959 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -204,6 +204,8 @@ struct annotation_print_data {
struct evsel *evsel;
struct arch *arch;
struct debuginfo *dbg;
+ /* save data type info keyed by al->offset */
+ struct hashmap *type_hash;
/* It'll be set in hist_entry__annotate_printf() */
int addr_fmt_width;
};
--
2.50.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
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
2025-08-20 23:38 ` Namhyung Kim
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 18:54 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 08/12] perf annotate: Add 'T' hot key to toggle data type display
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
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 21:13 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Aug 15, 2025 at 08:16:31PM -0700, Namhyung Kim wrote:
> Support data type display with a key press so that users can toggle the
> output dynamically on TUI. Also display "[Type]" in the title line if
> it's enabled.
Testing here I see:
0.81 │ mov 0x9c(%r8),%r11d # data-type: struct cfs_rq +0x9c
If I ask for source code by pressing 's':
│ delta += sa->period_contrib; ▒
0.97 │ mov 0x9c(%r8),%r11d # data-type: struct cfs_rq +0x9c
So it is the 'period_contrib' field of 'struct cfs_rq', humm, not:
root@number:~# pahole -E --hex cfs_rq | grep 0x9c -B 10 -A10
struct sched_entity * next; /* 0x60 0x8 */
/* XXX 24 bytes hole, try to pack */
/* --- cacheline 2 boundary (128 bytes) --- */
struct sched_avg {
/* typedef u64 -> __u64 */ long long unsigned int last_update_time; /* 0x80 0x8 */
/* typedef u64 -> __u64 */ long long unsigned int load_sum; /* 0x88 0x8 */
/* typedef u64 -> __u64 */ long long unsigned int runnable_sum; /* 0x90 0x8 */
/* typedef u32 -> __u32 */ unsigned int util_sum; /* 0x98 0x4 */
/* typedef u32 -> __u32 */ unsigned int period_contrib; /* 0x9c 0x4 */
long unsigned int load_avg; /* 0xa0 0x8 */
long unsigned int runnable_avg; /* 0xa8 0x8 */
long unsigned int util_avg; /* 0xb0 0x8 */
unsigned int util_est; /* 0xb8 0x4 */
} avg __attribute__((__aligned__(64))); /* 0x80 0x40 */
/* XXX last struct has 4 bytes of padding */
/* --- cacheline 3 boundary (192 bytes) --- */
struct {
root@number:~#
So it is in a subtype and probably this is an improvement to be made,
right?
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/annotate.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index e5b35336f0d33d7e..dfe869c20e35da77 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -533,9 +533,10 @@ static void ui_browser__init_asm_mode(struct ui_browser *browser)
> static int sym_title(struct symbol *sym, struct map *map, char *title,
> size_t sz, int percent_type)
> {
> - return snprintf(title, sz, "%s %s [Percent: %s]", sym->name,
> + return snprintf(title, sz, "%s %s [Percent: %s] %s", sym->name,
> dso__long_name(map__dso(map)),
> - percent_type_str(percent_type));
> + percent_type_str(percent_type),
> + annotate_opts.code_with_type ? "[Type]" : "");
> }
>
> /*
> @@ -909,7 +910,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
> "b Toggle percent base [period/hits]\n"
> "B Branch counter abbr list (Optional)\n"
> "? Search string backwards\n"
> - "f Toggle showing offsets to full address\n");
> + "f Toggle showing offsets to full address\n"
> + "T Toggle data type display\n");
> continue;
> case 'r':
> script_browse(NULL, NULL);
> @@ -1029,6 +1031,12 @@ static int annotate_browser__run(struct annotate_browser *browser,
> case 'f':
> annotation__toggle_full_addr(notes, ms);
> continue;
> + case 'T':
> + annotate_opts.code_with_type ^= 1;
> + if (browser->dbg == NULL)
> + browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> + annotate_browser__show(&browser->b, title, help);
> + continue;
> case K_LEFT:
> case '<':
> case '>':
> @@ -1133,8 +1141,7 @@ 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);
> + debuginfo__delete(browser.dbg);
> if (not_annotated && !notes->src->tried_source)
> annotated_source__purge(notes->src);
>
> --
> 2.50.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 10/12] perf annotate: Hide data-type for stack operation and canary
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
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 21:28 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Aug 15, 2025 at 08:16:33PM -0700, Namhyung Kim wrote:
> It's mostly unnecessary to print when it has no actual type information
> like in the stack operations and canary. Let's have them if -v option
> is given.
In the main 'report' TUI the 'V' key is wired to bumping the verbose
level, wiring it up in the annotation view seems in order :-)
- Arnaldo
> Before:
> $ perf annotate --code-with-type
> ...
> : 0 0xd640 <_dl_relocate_object>:
> 0.00 : 0: endbr64
> 0.00 : 4: pushq %rbp # data-type: (stack operation)
> 0.00 : 5: movq %rsp, %rbp
> 0.00 : 8: pushq %r15 # data-type: (stack operation)
> 0.00 : a: pushq %r14 # data-type: (stack operation)
> 0.00 : c: pushq %r13 # data-type: (stack operation)
> 0.00 : e: pushq %r12 # data-type: (stack operation)
> 0.00 : 10: pushq %rbx # data-type: (stack operation)
> 0.00 : 11: subq $0xf8, %rsp
> ...
> 0.00 : d4: testl %eax, %eax
> 0.00 : d6: jne 0xf424
> 0.00 : dc: movq 0xf0(%r14), %rbx # data-type: struct link_map +0xf0
> 0.00 : e3: testq %rbx, %rbx
> 0.00 : e6: jne 0xf2dd
> 0.00 : ec: cmpq $0, 0xf8(%r14) # data-type: struct link_map +0xf8
> ...
>
> After:
> : 0 0xd640 <_dl_relocate_object>:
> 0.00 : 0: endbr64
> 0.00 : 4: pushq %rbp
> 0.00 : 5: movq %rsp, %rbp
> 0.00 : 8: pushq %r15
> 0.00 : a: pushq %r14
> 0.00 : c: pushq %r13
> 0.00 : e: pushq %r12
> 0.00 : 10: pushq %rbx
> 0.00 : 11: subq $0xf8, %rsp
> ...
> 0.00 : d4: testl %eax, %eax
> 0.00 : d6: jne 0xf424
> 0.00 : dc: movq 0xf0(%r14), %rbx # data-type: struct link_map +0xf0
> 0.00 : e3: testq %rbx, %rbx
> 0.00 : e6: jne 0xf2dd
> 0.00 : ec: cmpq $0, 0xf8(%r14) # data-type: struct link_map +0xf8
> ...
>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/annotate.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 99e976d254493de2..ea68b32da7ce584a 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -765,6 +765,17 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
> struct debuginfo *dbg, struct disasm_line *dl,
> int *type_offset);
>
> +static bool needs_type_info(struct annotated_data_type *data_type)
> +{
> + if (data_type == NULL || data_type == NO_TYPE)
> + return false;
> +
> + if (verbose)
> + return true;
> +
> + return (data_type != &stackop_type) && (data_type != &canary_type);
> +}
> +
> static int
> annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
> struct annotation_options *opts, int printed,
> @@ -844,7 +855,7 @@ annotation_line__print(struct annotation_line *al, struct annotation_print_data
>
> data_type = __hist_entry__get_data_type(apd->he, apd->arch,
> apd->dbg, dl, &offset);
> - if (data_type && data_type != NO_TYPE) {
> + if (needs_type_info(data_type)) {
> char buf[4096];
>
> printf("\t\t# data-type: %s",
> @@ -1958,7 +1969,7 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
> 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)
> + if (!needs_type_info(data_type))
> return 1;
>
> printed = scnprintf(buf, len, "\t\t# data-type: %s", data_type->self.type_name);
> --
> 2.50.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 10/12] perf annotate: Hide data-type for stack operation and canary
2025-08-20 21:28 ` Arnaldo Carvalho de Melo
@ 2025-08-20 21:30 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 21:30 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Wed, Aug 20, 2025 at 06:28:53PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 15, 2025 at 08:16:33PM -0700, Namhyung Kim wrote:
> > It's mostly unnecessary to print when it has no actual type information
> > like in the stack operations and canary. Let's have them if -v option
> > is given.
>
> In the main 'report' TUI the 'V' key is wired to bumping the verbose
> level, wiring it up in the annotation view seems in order :-)
So using just 'perf top', then pressing 'V' and then going into the
annotation gets the verbose level needed to show these stack operations
in the annotate view :-)
- Arnaldo
> > Before:
> > $ perf annotate --code-with-type
> > ...
> > : 0 0xd640 <_dl_relocate_object>:
> > 0.00 : 0: endbr64
> > 0.00 : 4: pushq %rbp # data-type: (stack operation)
> > 0.00 : 5: movq %rsp, %rbp
> > 0.00 : 8: pushq %r15 # data-type: (stack operation)
> > 0.00 : a: pushq %r14 # data-type: (stack operation)
> > 0.00 : c: pushq %r13 # data-type: (stack operation)
> > 0.00 : e: pushq %r12 # data-type: (stack operation)
> > 0.00 : 10: pushq %rbx # data-type: (stack operation)
> > 0.00 : 11: subq $0xf8, %rsp
> > ...
> > 0.00 : d4: testl %eax, %eax
> > 0.00 : d6: jne 0xf424
> > 0.00 : dc: movq 0xf0(%r14), %rbx # data-type: struct link_map +0xf0
> > 0.00 : e3: testq %rbx, %rbx
> > 0.00 : e6: jne 0xf2dd
> > 0.00 : ec: cmpq $0, 0xf8(%r14) # data-type: struct link_map +0xf8
> > ...
> >
> > After:
> > : 0 0xd640 <_dl_relocate_object>:
> > 0.00 : 0: endbr64
> > 0.00 : 4: pushq %rbp
> > 0.00 : 5: movq %rsp, %rbp
> > 0.00 : 8: pushq %r15
> > 0.00 : a: pushq %r14
> > 0.00 : c: pushq %r13
> > 0.00 : e: pushq %r12
> > 0.00 : 10: pushq %rbx
> > 0.00 : 11: subq $0xf8, %rsp
> > ...
> > 0.00 : d4: testl %eax, %eax
> > 0.00 : d6: jne 0xf424
> > 0.00 : dc: movq 0xf0(%r14), %rbx # data-type: struct link_map +0xf0
> > 0.00 : e3: testq %rbx, %rbx
> > 0.00 : e6: jne 0xf2dd
> > 0.00 : ec: cmpq $0, 0xf8(%r14) # data-type: struct link_map +0xf8
> > ...
> >
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/annotate.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 99e976d254493de2..ea68b32da7ce584a 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -765,6 +765,17 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
> > struct debuginfo *dbg, struct disasm_line *dl,
> > int *type_offset);
> >
> > +static bool needs_type_info(struct annotated_data_type *data_type)
> > +{
> > + if (data_type == NULL || data_type == NO_TYPE)
> > + return false;
> > +
> > + if (verbose)
> > + return true;
> > +
> > + return (data_type != &stackop_type) && (data_type != &canary_type);
> > +}
> > +
> > static int
> > annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
> > struct annotation_options *opts, int printed,
> > @@ -844,7 +855,7 @@ annotation_line__print(struct annotation_line *al, struct annotation_print_data
> >
> > data_type = __hist_entry__get_data_type(apd->he, apd->arch,
> > apd->dbg, dl, &offset);
> > - if (data_type && data_type != NO_TYPE) {
> > + if (needs_type_info(data_type)) {
> > char buf[4096];
> >
> > printf("\t\t# data-type: %s",
> > @@ -1958,7 +1969,7 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
> > 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)
> > + if (!needs_type_info(data_type))
> > return 1;
> >
> > printed = scnprintf(buf, len, "\t\t# data-type: %s", data_type->self.type_name);
> > --
> > 2.50.1
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 12/12] perf annotate: Use a hashmap to save type data
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
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 21:37 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Aug 15, 2025 at 08:16:35PM -0700, Namhyung Kim wrote:
> It can slowdown annotation browser if objdump is processing large DWARF
> data. Let's add a hashmap to save the data type info for each line.
>
> Note that this is needed for TUI only because stdio only processes each
> line once. TUI will display the same line whenever it refreshes the
> screen.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/annotate.c | 34 ++++++++++++++++++++++++++++++-
> tools/perf/util/annotate.c | 27 ++++++++++++++++++++++--
> tools/perf/util/annotate.h | 2 ++
> 3 files changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 9aa3c1ba22f52789..9677a3763a290a3d 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -6,6 +6,7 @@
> #include "../../util/debug.h"
> #include "../../util/debuginfo.h"
> #include "../../util/dso.h"
> +#include "../../util/hashmap.h"
> #include "../../util/hist.h"
> #include "../../util/sort.h"
> #include "../../util/map.h"
> @@ -15,6 +16,7 @@
> #include "../../util/evlist.h"
> #include "../../util/thread.h"
> #include <inttypes.h>
> +#include <linux/err.h>
> #include <linux/kernel.h>
> #include <linux/string.h>
> #include <linux/zalloc.h>
> @@ -36,6 +38,7 @@ struct annotate_browser {
> struct hist_entry *he;
> struct debuginfo *dbg;
> struct evsel *evsel;
> + struct hashmap *type_hash;
> bool searching_backwards;
> char search_bf[128];
> };
> @@ -43,6 +46,16 @@ struct annotate_browser {
> /* A copy of target hist_entry for perf top. */
> static struct hist_entry annotate_he;
>
> +static size_t type_hash(long key, void *ctx __maybe_unused)
> +{
> + return key;
> +}
> +
> +static bool type_equal(long key1, long key2, void *ctx __maybe_unused)
> +{
> + return key1 == key2;
> +}
> +
> static inline struct annotation *browser__annotation(struct ui_browser *browser)
> {
> struct map_symbol *ms = browser->priv;
> @@ -130,6 +143,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
> if (!browser->navkeypressed)
> ops.width += 1;
>
> + if (!IS_ERR_OR_NULL(ab->type_hash))
> + apd.type_hash = ab->type_hash;
> +
> annotation_line__write(al, notes, &ops, &apd);
>
> if (ops.current_entry)
> @@ -1051,6 +1067,10 @@ static int annotate_browser__run(struct annotate_browser *browser,
> annotate_opts.code_with_type ^= 1;
> if (browser->dbg == NULL)
> browser->dbg = dso__debuginfo(map__dso(ms->map));
> + if (browser->type_hash == NULL) {
> + browser->type_hash = hashmap__new(type_hash, type_equal,
> + /*ctx=*/NULL);
> + }
> annotate_browser__show(&browser->b, title, help);
> annotate_browser__debuginfo_warning(browser);
> continue;
> @@ -1145,8 +1165,10 @@ 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)
> + if (annotate_opts.code_with_type) {
> browser.dbg = dso__debuginfo(dso);
> + browser.type_hash = hashmap__new(type_hash, type_equal, /*ctx=*/NULL);
> + }
>
> browser.b.width = notes->src->widths.max_line_len;
> browser.b.nr_entries = notes->src->nr_entries;
> @@ -1159,6 +1181,16 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> ret = annotate_browser__run(&browser, evsel, hbt);
>
> debuginfo__delete(browser.dbg);
> +
> + if (!IS_ERR_OR_NULL(browser.type_hash)) {
> + struct hashmap_entry *cur;
> + size_t bkt;
> +
> + hashmap__for_each_entry(browser.type_hash, cur, bkt)
> + free(cur->pvalue);
zfree(&cur->pvalue);
> + hashmap__free(browser.type_hash);
> + }
> +
> 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 bea3457a00632fd7..77414e04d99bb4f2 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1954,11 +1954,17 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
> return -ENOMEM;
> }
>
> +struct type_hash_entry {
> + struct annotated_data_type *type;
> + int offset;
> +};
> +
> 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;
> + struct annotated_data_type *data_type = NULL;
> + struct type_hash_entry *entry = NULL;
> char member[256];
> int offset = 0;
> int printed;
> @@ -1968,7 +1974,24 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
> 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 (apd->type_hash) {
> + hashmap__find(apd->type_hash, dl->al.offset, &entry);
> + if (entry != NULL) {
> + data_type = entry->type;
> + offset = entry->offset;
> + }
> + }
> + if (data_type == NULL)
> + data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
add space?
> + if (apd->type_hash && entry == NULL) {
> + entry = malloc(sizeof(*entry));
Is the 'entry' variable needed anywhere else? Not, so could be declared
here to save a line at the start of the function. Or is it used in a
later patch outside of this scope?
> + if (entry != NULL) {
> + entry->type = data_type;
> + entry->offset = offset;
> + hashmap__add(apd->type_hash, dl->al.offset, entry);
> + }
> + }
> +
> if (!needs_type_info(data_type))
> return 1;
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 86e858f5bf173152..eaf6c8aa7f473959 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -204,6 +204,8 @@ struct annotation_print_data {
> struct evsel *evsel;
> struct arch *arch;
> struct debuginfo *dbg;
> + /* save data type info keyed by al->offset */
> + struct hashmap *type_hash;
> /* It'll be set in hist_entry__annotate_printf() */
> int addr_fmt_width;
> };
> --
> 2.50.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI
2025-08-16 3:16 [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (11 preceding siblings ...)
2025-08-16 3:16 ` [PATCH v5 12/12] perf annotate: Use a hashmap to save type data Namhyung Kim
@ 2025-08-20 21:40 ` Arnaldo Carvalho de Melo
2025-08-20 21:43 ` Arnaldo Carvalho de Melo
12 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 21:40 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Aug 15, 2025 at 08:16:23PM -0700, Namhyung Kim wrote:
>
> Hello,
>
> The --code-with-type option is to show normal annotate output with type
> information. It was supported only on --stdio, but this change adds it
> to TUI as well.
>
> Arnaldo, please tell me if you still don't like the
> __hist_entry__tui_annotate() but I don't have a better idea for now.
Not a problem, I tested everything, all seems to work as advertised.
The minor suggestions I made I can do while merging, if you don't mind
and agree with them.
Please let me know.
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
- Arnaldo
> v5 changes)
> * use a copy of hist entry for perf top (Ian)
> * split disasm_line__write() change (Ian)
> * constify annotation_write_ops parameter (Ian)
> * update printed length calculation (Ian)
> * remove annotation_print_data.start
> * add a hashmap to skip duplicate processing
>
> v4 changes)
> * add dso__debuginfo() helper (Ian)
>
> v3 changes)
> * hide stack operation and stack canary by default
>
> v2 changes)
> * use 'T' key to toggle data type display (Arnaldo)
> * display '[Type]' in the title line when it's enabled (Arnaldo)
> * show warning when debug info is not available (Arnaldo)
> * fix a typo (Arnaldo)
>
> Actually the command line option sets the default behavior and users can
> change it by pressing 'T' key in the TUI annotate browser.
>
> The code is also available at 'perf/annotate-code-type-tui-v5' branch at
> https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Thanks,
> Namhyung
>
>
> Namhyung Kim (12):
> perf annotate: Rename to __hist_entry__tui_annotate()
> perf annotate: Remove annotation_print_data.start
> perf annotate: Remove __annotation_line__write()
> perf annotate: Pass annotation_print_data to annotation_line__write()
> perf annotate: Simplify width calculation in annotation_line__write()
> perf annotate: Return printed number from disasm_line__write()
> perf annotate: Add --code-with-type support for TUI
> perf annotate: Add 'T' hot key to toggle data type display
> perf annotate: Show warning when debuginfo is not available
> perf annotate: Hide data-type for stack operation and canary
> perf annotate: Add dso__debuginfo() helper
> perf annotate: Use a hashmap to save type data
>
> tools/perf/Documentation/perf-annotate.txt | 1 -
> tools/perf/builtin-annotate.c | 5 -
> tools/perf/ui/browsers/annotate.c | 117 ++++++++++++--
> tools/perf/ui/browsers/hists.c | 2 +-
> tools/perf/util/annotate.c | 178 +++++++++++++++------
> tools/perf/util/annotate.h | 29 ++--
> tools/perf/util/dso.h | 21 +++
> tools/perf/util/hist.h | 12 +-
> 8 files changed, 273 insertions(+), 92 deletions(-)
>
> --
> 2.50.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI
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
0 siblings, 1 reply; 24+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-08-20 21:43 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Wed, Aug 20, 2025 at 06:40:53PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 15, 2025 at 08:16:23PM -0700, Namhyung Kim wrote:
> >
> > Hello,
> >
> > The --code-with-type option is to show normal annotate output with type
> > information. It was supported only on --stdio, but this change adds it
> > to TUI as well.
> >
> > Arnaldo, please tell me if you still don't like the
> > __hist_entry__tui_annotate() but I don't have a better idea for now.
>
> Not a problem, I tested everything, all seems to work as advertised.
>
> The minor suggestions I made I can do while merging, if you don't mind
> and agree with them.
I mean the scope of a variable, a missing line before an if line, not
the other more involved one as the alignment of the type comments, etc.
:-)
- Arnaldo
> Please let me know.
>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> - Arnaldo
>
> > v5 changes)
> > * use a copy of hist entry for perf top (Ian)
> > * split disasm_line__write() change (Ian)
> > * constify annotation_write_ops parameter (Ian)
> > * update printed length calculation (Ian)
> > * remove annotation_print_data.start
> > * add a hashmap to skip duplicate processing
> >
> > v4 changes)
> > * add dso__debuginfo() helper (Ian)
> >
> > v3 changes)
> > * hide stack operation and stack canary by default
> >
> > v2 changes)
> > * use 'T' key to toggle data type display (Arnaldo)
> > * display '[Type]' in the title line when it's enabled (Arnaldo)
> > * show warning when debug info is not available (Arnaldo)
> > * fix a typo (Arnaldo)
> >
> > Actually the command line option sets the default behavior and users can
> > change it by pressing 'T' key in the TUI annotate browser.
> >
> > The code is also available at 'perf/annotate-code-type-tui-v5' branch at
> > https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >
> > Thanks,
> > Namhyung
> >
> >
> > Namhyung Kim (12):
> > perf annotate: Rename to __hist_entry__tui_annotate()
> > perf annotate: Remove annotation_print_data.start
> > perf annotate: Remove __annotation_line__write()
> > perf annotate: Pass annotation_print_data to annotation_line__write()
> > perf annotate: Simplify width calculation in annotation_line__write()
> > perf annotate: Return printed number from disasm_line__write()
> > perf annotate: Add --code-with-type support for TUI
> > perf annotate: Add 'T' hot key to toggle data type display
> > perf annotate: Show warning when debuginfo is not available
> > perf annotate: Hide data-type for stack operation and canary
> > perf annotate: Add dso__debuginfo() helper
> > perf annotate: Use a hashmap to save type data
> >
> > tools/perf/Documentation/perf-annotate.txt | 1 -
> > tools/perf/builtin-annotate.c | 5 -
> > tools/perf/ui/browsers/annotate.c | 117 ++++++++++++--
> > tools/perf/ui/browsers/hists.c | 2 +-
> > tools/perf/util/annotate.c | 178 +++++++++++++++------
> > tools/perf/util/annotate.h | 29 ++--
> > tools/perf/util/dso.h | 21 +++
> > tools/perf/util/hist.h | 12 +-
> > 8 files changed, 273 insertions(+), 92 deletions(-)
> >
> > --
> > 2.50.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI
2025-08-20 18:54 ` Arnaldo Carvalho de Melo
@ 2025-08-20 23:38 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-20 23:38 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 08/12] perf annotate: Add 'T' hot key to toggle data type display
2025-08-20 21:13 ` Arnaldo Carvalho de Melo
@ 2025-08-20 23:43 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-20 23:43 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Wed, Aug 20, 2025 at 06:13:15PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 15, 2025 at 08:16:31PM -0700, Namhyung Kim wrote:
> > Support data type display with a key press so that users can toggle the
> > output dynamically on TUI. Also display "[Type]" in the title line if
> > it's enabled.
>
> Testing here I see:
>
> 0.81 │ mov 0x9c(%r8),%r11d # data-type: struct cfs_rq +0x9c
>
> If I ask for source code by pressing 's':
>
> │ delta += sa->period_contrib; ▒
> 0.97 │ mov 0x9c(%r8),%r11d # data-type: struct cfs_rq +0x9c
>
> So it is the 'period_contrib' field of 'struct cfs_rq', humm, not:
>
> root@number:~# pahole -E --hex cfs_rq | grep 0x9c -B 10 -A10
> struct sched_entity * next; /* 0x60 0x8 */
>
> /* XXX 24 bytes hole, try to pack */
>
> /* --- cacheline 2 boundary (128 bytes) --- */
> struct sched_avg {
> /* typedef u64 -> __u64 */ long long unsigned int last_update_time; /* 0x80 0x8 */
> /* typedef u64 -> __u64 */ long long unsigned int load_sum; /* 0x88 0x8 */
> /* typedef u64 -> __u64 */ long long unsigned int runnable_sum; /* 0x90 0x8 */
> /* typedef u32 -> __u32 */ unsigned int util_sum; /* 0x98 0x4 */
> /* typedef u32 -> __u32 */ unsigned int period_contrib; /* 0x9c 0x4 */
> long unsigned int load_avg; /* 0xa0 0x8 */
> long unsigned int runnable_avg; /* 0xa8 0x8 */
> long unsigned int util_avg; /* 0xb0 0x8 */
> unsigned int util_est; /* 0xb8 0x4 */
> } avg __attribute__((__aligned__(64))); /* 0x80 0x40 */
>
> /* XXX last struct has 4 bytes of padding */
>
> /* --- cacheline 3 boundary (192 bytes) --- */
> struct {
> root@number:~#
>
> So it is in a subtype and probably this is an improvement to be made,
> right?
It seems %r8 contains a pointer to cfs_rq and there's a variable for sa
(sched_avg) but it actually uses the same register as it's embeded. I
think DWARF has correct location expression to find the containing data
type and the output looks ok. In general, I'd prefer seeing outer types.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 12/12] perf annotate: Use a hashmap to save type data
2025-08-20 21:37 ` Arnaldo Carvalho de Melo
@ 2025-08-20 23:46 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-20 23:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Wed, Aug 20, 2025 at 06:37:18PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 15, 2025 at 08:16:35PM -0700, Namhyung Kim wrote:
> > It can slowdown annotation browser if objdump is processing large DWARF
> > data. Let's add a hashmap to save the data type info for each line.
> >
> > Note that this is needed for TUI only because stdio only processes each
> > line once. TUI will display the same line whenever it refreshes the
> > screen.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/ui/browsers/annotate.c | 34 ++++++++++++++++++++++++++++++-
> > tools/perf/util/annotate.c | 27 ++++++++++++++++++++++--
> > tools/perf/util/annotate.h | 2 ++
> > 3 files changed, 60 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 9aa3c1ba22f52789..9677a3763a290a3d 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -6,6 +6,7 @@
> > #include "../../util/debug.h"
> > #include "../../util/debuginfo.h"
> > #include "../../util/dso.h"
> > +#include "../../util/hashmap.h"
> > #include "../../util/hist.h"
> > #include "../../util/sort.h"
> > #include "../../util/map.h"
> > @@ -15,6 +16,7 @@
> > #include "../../util/evlist.h"
> > #include "../../util/thread.h"
> > #include <inttypes.h>
> > +#include <linux/err.h>
> > #include <linux/kernel.h>
> > #include <linux/string.h>
> > #include <linux/zalloc.h>
> > @@ -36,6 +38,7 @@ struct annotate_browser {
> > struct hist_entry *he;
> > struct debuginfo *dbg;
> > struct evsel *evsel;
> > + struct hashmap *type_hash;
> > bool searching_backwards;
> > char search_bf[128];
> > };
> > @@ -43,6 +46,16 @@ struct annotate_browser {
> > /* A copy of target hist_entry for perf top. */
> > static struct hist_entry annotate_he;
> >
> > +static size_t type_hash(long key, void *ctx __maybe_unused)
> > +{
> > + return key;
> > +}
> > +
> > +static bool type_equal(long key1, long key2, void *ctx __maybe_unused)
> > +{
> > + return key1 == key2;
> > +}
> > +
> > static inline struct annotation *browser__annotation(struct ui_browser *browser)
> > {
> > struct map_symbol *ms = browser->priv;
> > @@ -130,6 +143,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
> > if (!browser->navkeypressed)
> > ops.width += 1;
> >
> > + if (!IS_ERR_OR_NULL(ab->type_hash))
> > + apd.type_hash = ab->type_hash;
> > +
> > annotation_line__write(al, notes, &ops, &apd);
> >
> > if (ops.current_entry)
> > @@ -1051,6 +1067,10 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > annotate_opts.code_with_type ^= 1;
> > if (browser->dbg == NULL)
> > browser->dbg = dso__debuginfo(map__dso(ms->map));
> > + if (browser->type_hash == NULL) {
> > + browser->type_hash = hashmap__new(type_hash, type_equal,
> > + /*ctx=*/NULL);
> > + }
> > annotate_browser__show(&browser->b, title, help);
> > annotate_browser__debuginfo_warning(browser);
> > continue;
> > @@ -1145,8 +1165,10 @@ 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)
> > + if (annotate_opts.code_with_type) {
> > browser.dbg = dso__debuginfo(dso);
> > + browser.type_hash = hashmap__new(type_hash, type_equal, /*ctx=*/NULL);
> > + }
> >
> > browser.b.width = notes->src->widths.max_line_len;
> > browser.b.nr_entries = notes->src->nr_entries;
> > @@ -1159,6 +1181,16 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > ret = annotate_browser__run(&browser, evsel, hbt);
> >
> > debuginfo__delete(browser.dbg);
> > +
> > + if (!IS_ERR_OR_NULL(browser.type_hash)) {
> > + struct hashmap_entry *cur;
> > + size_t bkt;
> > +
> > + hashmap__for_each_entry(browser.type_hash, cur, bkt)
> > + free(cur->pvalue);
>
> zfree(&cur->pvalue);
Yep. :)
>
> > + hashmap__free(browser.type_hash);
> > + }
> > +
> > 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 bea3457a00632fd7..77414e04d99bb4f2 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -1954,11 +1954,17 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
> > return -ENOMEM;
> > }
> >
> > +struct type_hash_entry {
> > + struct annotated_data_type *type;
> > + int offset;
> > +};
> > +
> > 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;
> > + struct annotated_data_type *data_type = NULL;
> > + struct type_hash_entry *entry = NULL;
> > char member[256];
> > int offset = 0;
> > int printed;
> > @@ -1968,7 +1974,24 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
> > 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 (apd->type_hash) {
> > + hashmap__find(apd->type_hash, dl->al.offset, &entry);
> > + if (entry != NULL) {
> > + data_type = entry->type;
> > + offset = entry->offset;
> > + }
> > + }
> > + if (data_type == NULL)
> > + data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
>
>
> add space?
Sure.
>
> > + if (apd->type_hash && entry == NULL) {
> > + entry = malloc(sizeof(*entry));
>
> Is the 'entry' variable needed anywhere else? Not, so could be declared
> here to save a line at the start of the function. Or is it used in a
> later patch outside of this scope?
It was used in the earlier block to look up an existing entry.
Thanks,
Namhyung
>
> > + if (entry != NULL) {
> > + entry->type = data_type;
> > + entry->offset = offset;
> > + hashmap__add(apd->type_hash, dl->al.offset, entry);
> > + }
> > + }
> > +
> > if (!needs_type_info(data_type))
> > return 1;
> >
> > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> > index 86e858f5bf173152..eaf6c8aa7f473959 100644
> > --- a/tools/perf/util/annotate.h
> > +++ b/tools/perf/util/annotate.h
> > @@ -204,6 +204,8 @@ struct annotation_print_data {
> > struct evsel *evsel;
> > struct arch *arch;
> > struct debuginfo *dbg;
> > + /* save data type info keyed by al->offset */
> > + struct hashmap *type_hash;
> > /* It'll be set in hist_entry__annotate_printf() */
> > int addr_fmt_width;
> > };
> > --
> > 2.50.1
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHSET v5 00/12] perf annotate: Support --code-with-type on TUI
2025-08-20 21:43 ` Arnaldo Carvalho de Melo
@ 2025-08-20 23:47 ` Namhyung Kim
0 siblings, 0 replies; 24+ messages in thread
From: Namhyung Kim @ 2025-08-20 23:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Wed, Aug 20, 2025 at 06:43:03PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Aug 20, 2025 at 06:40:53PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Fri, Aug 15, 2025 at 08:16:23PM -0700, Namhyung Kim wrote:
> > >
> > > Hello,
> > >
> > > The --code-with-type option is to show normal annotate output with type
> > > information. It was supported only on --stdio, but this change adds it
> > > to TUI as well.
> > >
> > > Arnaldo, please tell me if you still don't like the
> > > __hist_entry__tui_annotate() but I don't have a better idea for now.
> >
> > Not a problem, I tested everything, all seems to work as advertised.
> >
> > The minor suggestions I made I can do while merging, if you don't mind
> > and agree with them.
>
> I mean the scope of a variable, a missing line before an if line, not
> the other more involved one as the alignment of the type comments, etc.
> :-)
I've left comments in each thread.
>
> - Arnaldo
>
> > Please let me know.
> >
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Thanks for your review and tests!
Namhyung
> >
> > - Arnaldo
> >
> > > v5 changes)
> > > * use a copy of hist entry for perf top (Ian)
> > > * split disasm_line__write() change (Ian)
> > > * constify annotation_write_ops parameter (Ian)
> > > * update printed length calculation (Ian)
> > > * remove annotation_print_data.start
> > > * add a hashmap to skip duplicate processing
> > >
> > > v4 changes)
> > > * add dso__debuginfo() helper (Ian)
> > >
> > > v3 changes)
> > > * hide stack operation and stack canary by default
> > >
> > > v2 changes)
> > > * use 'T' key to toggle data type display (Arnaldo)
> > > * display '[Type]' in the title line when it's enabled (Arnaldo)
> > > * show warning when debug info is not available (Arnaldo)
> > > * fix a typo (Arnaldo)
> > >
> > > Actually the command line option sets the default behavior and users can
> > > change it by pressing 'T' key in the TUI annotate browser.
> > >
> > > The code is also available at 'perf/annotate-code-type-tui-v5' branch at
> > > https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >
> > > Thanks,
> > > Namhyung
> > >
> > >
> > > Namhyung Kim (12):
> > > perf annotate: Rename to __hist_entry__tui_annotate()
> > > perf annotate: Remove annotation_print_data.start
> > > perf annotate: Remove __annotation_line__write()
> > > perf annotate: Pass annotation_print_data to annotation_line__write()
> > > perf annotate: Simplify width calculation in annotation_line__write()
> > > perf annotate: Return printed number from disasm_line__write()
> > > perf annotate: Add --code-with-type support for TUI
> > > perf annotate: Add 'T' hot key to toggle data type display
> > > perf annotate: Show warning when debuginfo is not available
> > > perf annotate: Hide data-type for stack operation and canary
> > > perf annotate: Add dso__debuginfo() helper
> > > perf annotate: Use a hashmap to save type data
> > >
> > > tools/perf/Documentation/perf-annotate.txt | 1 -
> > > tools/perf/builtin-annotate.c | 5 -
> > > tools/perf/ui/browsers/annotate.c | 117 ++++++++++++--
> > > tools/perf/ui/browsers/hists.c | 2 +-
> > > tools/perf/util/annotate.c | 178 +++++++++++++++------
> > > tools/perf/util/annotate.h | 29 ++--
> > > tools/perf/util/dso.h | 21 +++
> > > tools/perf/util/hist.h | 12 +-
> > > 8 files changed, 273 insertions(+), 92 deletions(-)
> > >
> > > --
> > > 2.50.1
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-08-20 23:47 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
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).