* [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI
@ 2025-07-16 5:00 Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
` (8 more replies)
0 siblings, 9 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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.
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-v3' branch at
https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (8):
perf annotate: Rename to __hist_entry__tui_annotate()
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: 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
tools/perf/Documentation/perf-annotate.txt | 1 -
tools/perf/builtin-annotate.c | 5 -
tools/perf/ui/browsers/annotate.c | 66 +++++++--
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/util/annotate.c | 157 ++++++++++++++-------
tools/perf/util/annotate.h | 27 ++--
tools/perf/util/dso.h | 11 ++
tools/perf/util/hist.h | 12 +-
8 files changed, 191 insertions(+), 90 deletions(-)
--
2.50.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate()
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 20:16 ` Arnaldo Carvalho de Melo
2025-07-16 5:00 ` [PATCH v3 2/8] perf annotate: Remove __annotation_line__write() Namhyung Kim
` (7 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 17 +++++++----------
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/util/annotate.h | 12 ------------
tools/perf/util/hist.h | 12 +++++++-----
4 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 183902dac042ecb0..28ef146f29e8e742 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -27,6 +27,7 @@ struct annotate_browser {
struct rb_node *curr_hot;
struct annotation_line *selection;
struct arch *arch;
+ struct hist_entry *he;
bool searching_backwards;
char search_bf[128];
};
@@ -557,7 +558,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 +1033,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 +1041,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 +1060,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;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d26b925e3d7f46af..55455c49faf01891 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 c64254088fc77246..11ae738772ca4f61 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -712,8 +712,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);
@@ -741,9 +742,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.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/8] perf annotate: Remove __annotation_line__write()
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 3/8] perf annotate: Pass annotation_print_data to annotation_line__write() Namhyung Kim
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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 ++++++++++++++++----------------------
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 0dd475a744b6dfac..69ee83052396b15e 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1935,24 +1935,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,
+ 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;
@@ -1966,7 +1968,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) {
@@ -2115,17 +2118,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)
{
--
2.50.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/8] perf annotate: Pass annotation_print_data to annotation_line__write()
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 2/8] perf annotate: Remove __annotation_line__write() Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 4/8] perf annotate: Simplify width calculation in annotation_line__write() Namhyung Kim
` (5 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/annotate.c | 13 ++++++++++--
tools/perf/util/annotate.c | 35 +++++++++++++++++--------------
tools/perf/util/annotate.h | 15 +++++++++++--
3 files changed, 43 insertions(+), 20 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 28ef146f29e8e742..23bea5b165774ae7 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -28,6 +28,8 @@ struct annotate_browser {
struct annotation_line *selection;
struct arch *arch;
struct hist_entry *he;
+ struct debuginfo *dbg;
+ struct evsel *evsel;
bool searching_backwards;
char search_bf[128];
};
@@ -108,12 +110,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;
@@ -976,7 +984,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) {
@@ -1061,6 +1069,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 69ee83052396b15e..c21152710148b68c 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -765,15 +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;
- u64 start;
- int addr_fmt_width;
-};
-
static int
annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
struct annotation_options *opts, int printed,
@@ -1230,7 +1221,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;
@@ -1357,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 = {
@@ -1374,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;
}
@@ -1382,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;
@@ -1404,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;
@@ -1656,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;
@@ -1678,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);
@@ -1936,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,
- struct annotation_write_ops *wops)
+ struct annotation_write_ops *wops,
+ struct annotation_print_data *apd)
{
bool current_entry = wops->current_entry;
bool change_color = wops->change_color;
@@ -2114,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 0f640e4871744262..8fad464a870a2b8e 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;
+ u64 start;
+ int addr_fmt_width;
+};
+
void annotation_line__write(struct annotation_line *al, struct annotation *notes,
- struct annotation_write_ops *ops);
+ 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.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 4/8] perf annotate: Simplify width calculation in annotation_line__write()
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (2 preceding siblings ...)
2025-07-16 5:00 ` [PATCH v3 3/8] perf annotate: Pass annotation_print_data to annotation_line__write() Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI Namhyung Kim
` (4 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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 | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index c21152710148b68c..d69e406c1bc289cd 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,12 @@ 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, " ");
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 +2071,7 @@ 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);
+ obj__printf(obj, "%-*s", width - printed + 1, al->line);
} else {
u64 addr = al->offset;
int color = -1;
@@ -2112,9 +2114,11 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
if (change_color)
obj__set_color(obj, color);
+ width -= printed + 3;
+
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.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (3 preceding siblings ...)
2025-07-16 5:00 ` [PATCH v3 4/8] perf annotate: Simplify width calculation in annotation_line__write() Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 15:00 ` Ian Rogers
2025-07-23 16:04 ` Arnaldo Carvalho de Melo
2025-07-16 5:00 ` [PATCH v3 6/8] perf annotate: Add 'T' hot key to toggle data type display Namhyung Kim
` (3 subsequent siblings)
8 siblings, 2 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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.
Make disasm_line__write() return the number of printed characters so
that it can skip unnecessary operations when the screen is full.
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 | 61 +++++++++++++++++++---
4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
@@ -1101,6 +1102,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;
@@ -1111,6 +1115,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 d69e406c1bc289cd..06ddc7a9f58722a4 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;
}
@@ -1743,7 +1751,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 +1779,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);
}
static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
@@ -2116,11 +2124,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
width -= printed + 3;
- 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", bf);
+ width -= printed;
+
+ if (annotate_opts.code_with_type && apd->dbg) {
+ struct annotated_data_type *data_type;
+ int offset = 0;
+
+ data_type = __hist_entry__get_data_type(apd->he, apd->arch,
+ apd->dbg,
+ disasm_line(al),
+ &offset);
+ if (data_type && data_type != NO_TYPE) {
+ char member[256];
+
+ printed = scnprintf(bf, sizeof(bf),
+ "\t\t# data-type: %s",
+ data_type->self.type_name);
- obj__printf(obj, "%-*s", width, bf);
+ if (data_type != &stackop_type &&
+ data_type != &canary_type &&
+ sizeof(bf) > (size_t)printed) {
+ printed += scnprintf(bf + printed,
+ sizeof(bf) - printed,
+ " +%#x", offset);
+ }
+
+ if (annotated_data_type__get_member_name(data_type,
+ member,
+ sizeof(member),
+ offset) &&
+ sizeof(bf) > (size_t)printed) {
+ printed += scnprintf(bf + printed,
+ sizeof(bf) - printed,
+ " (%s)", member);
+ }
- (void)apd;
+ obj__printf(obj, "%-*s", width, bf);
+ } else {
+ obj__printf(obj, "%-*s", width, " ");
+ }
+ } else {
+ obj__printf(obj, "%-*s", width, " ");
+ }
}
}
--
2.50.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 6/8] perf annotate: Add 'T' hot key to toggle data type display
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (4 preceding siblings ...)
2025-07-16 5:00 ` [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 7/8] perf annotate: Show warning when debuginfo is not available Namhyung Kim
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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 cdee1969f3131a7c..4b059e0bafd33fcf 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -525,9 +525,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]" : "");
}
/*
@@ -901,7 +902,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);
@@ -1021,6 +1023,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 '>':
@@ -1115,8 +1123,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.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 7/8] perf annotate: Show warning when debuginfo is not available
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (5 preceding siblings ...)
2025-07-16 5:00 ` [PATCH v3 6/8] perf annotate: Add 'T' hot key to toggle data type display Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 8/8] perf annotate: Hide data-type for stack operation and canary Namhyung Kim
2025-07-16 15:02 ` [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Ian Rogers
8 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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 4b059e0bafd33fcf..2a4db5bdcdb7e9d8 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -804,6 +804,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)
@@ -834,6 +848,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);
@@ -1028,6 +1044,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 c87564471f9b01b1..42dadcc85a0e9150 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -282,6 +282,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;
@@ -342,6 +343,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.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 8/8] perf annotate: Hide data-type for stack operation and canary
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (6 preceding siblings ...)
2025-07-16 5:00 ` [PATCH v3 7/8] perf annotate: Show warning when debuginfo is not available Namhyung Kim
@ 2025-07-16 5:00 ` Namhyung Kim
2025-07-16 15:02 ` [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Ian Rogers
8 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 5:00 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
...
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 06ddc7a9f58722a4..6fc07971631ac8a3 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",
@@ -2138,7 +2149,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
apd->dbg,
disasm_line(al),
&offset);
- if (data_type && data_type != NO_TYPE) {
+ if (needs_type_info(data_type)) {
char member[256];
printed = scnprintf(bf, sizeof(bf),
--
2.50.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 5:00 ` [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI Namhyung Kim
@ 2025-07-16 15:00 ` Ian Rogers
2025-07-16 17:27 ` Namhyung Kim
2025-07-23 16:04 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 19+ messages in thread
From: Ian Rogers @ 2025-07-16 15:00 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> 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.
> Make disasm_line__write() return the number of printed characters so
> that it can skip unnecessary operations when the screen is full.
>
> 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 | 61 +++++++++++++++++++---
> 4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
> @@ -1101,6 +1102,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;
> @@ -1111,6 +1115,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 d69e406c1bc289cd..06ddc7a9f58722a4 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)));
This API looks unfortunate. A dso may have a long name (it'd be easier
to understand if this were called path rather than long name) or a
build ID. The API isn't the fault of this change, but I thought I'd
mention as we move toward greater use of build IDs.
Thanks,
Ian
> + }
> +
> 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;
> }
>
> @@ -1743,7 +1751,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 +1779,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);
> }
>
> static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
> @@ -2116,11 +2124,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>
> width -= printed + 3;
>
> - 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", bf);
> + width -= printed;
> +
> + if (annotate_opts.code_with_type && apd->dbg) {
> + struct annotated_data_type *data_type;
> + int offset = 0;
> +
> + data_type = __hist_entry__get_data_type(apd->he, apd->arch,
> + apd->dbg,
> + disasm_line(al),
> + &offset);
> + if (data_type && data_type != NO_TYPE) {
> + char member[256];
> +
> + printed = scnprintf(bf, sizeof(bf),
> + "\t\t# data-type: %s",
> + data_type->self.type_name);
>
> - obj__printf(obj, "%-*s", width, bf);
> + if (data_type != &stackop_type &&
> + data_type != &canary_type &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " +%#x", offset);
> + }
> +
> + if (annotated_data_type__get_member_name(data_type,
> + member,
> + sizeof(member),
> + offset) &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " (%s)", member);
> + }
>
> - (void)apd;
> + obj__printf(obj, "%-*s", width, bf);
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> }
>
> }
> --
> 2.50.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
` (7 preceding siblings ...)
2025-07-16 5:00 ` [PATCH v3 8/8] perf annotate: Hide data-type for stack operation and canary Namhyung Kim
@ 2025-07-16 15:02 ` Ian Rogers
8 siblings, 0 replies; 19+ messages in thread
From: Ian Rogers @ 2025-07-16 15:02 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> 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.
>
> 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-v3' branch at
> https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Thanks,
> Namhyung
For the series:
Reviewed-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> Namhyung Kim (8):
> perf annotate: Rename to __hist_entry__tui_annotate()
> 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: 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
>
> tools/perf/Documentation/perf-annotate.txt | 1 -
> tools/perf/builtin-annotate.c | 5 -
> tools/perf/ui/browsers/annotate.c | 66 +++++++--
> tools/perf/ui/browsers/hists.c | 2 +-
> tools/perf/util/annotate.c | 157 ++++++++++++++-------
> tools/perf/util/annotate.h | 27 ++--
> tools/perf/util/dso.h | 11 ++
> tools/perf/util/hist.h | 12 +-
> 8 files changed, 191 insertions(+), 90 deletions(-)
>
> --
> 2.50.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 15:00 ` Ian Rogers
@ 2025-07-16 17:27 ` Namhyung Kim
2025-07-16 20:42 ` Ian Rogers
0 siblings, 1 reply; 19+ messages in thread
From: Namhyung Kim @ 2025-07-16 17:27 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Wed, Jul 16, 2025 at 08:00:37AM -0700, Ian Rogers wrote:
> On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> 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.
> > Make disasm_line__write() return the number of printed characters so
> > that it can skip unnecessary operations when the screen is full.
> >
> > 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 | 61 +++++++++++++++++++---
> > 4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
> > @@ -1101,6 +1102,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;
> > @@ -1111,6 +1115,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 d69e406c1bc289cd..06ddc7a9f58722a4 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)));
>
> This API looks unfortunate. A dso may have a long name (it'd be easier
> to understand if this were called path rather than long name) or a
> build ID. The API isn't the fault of this change, but I thought I'd
> mention as we move toward greater use of build IDs.
Are you talking about build-id in MMAP2? I think it's build-id vs. (dev
major/minor + inode) and the long name should be available always.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate()
2025-07-16 5:00 ` [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
@ 2025-07-16 20:16 ` Arnaldo Carvalho de Melo
2025-07-17 0:55 ` Namhyung Kim
0 siblings, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-07-16 20:16 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Tue, Jul 15, 2025 at 10:00:47PM -0700, Namhyung Kim wrote:
> There are three different but similar functions for annotation on TUI.
Why the two initial __? Normally this is when its about work on some
data structure but the pointer to it isn't passed, but in this case the
first arg is a 'struct hist_entry *', so calling it
hist_entry__tui_annotate() would be right?
Well, there is already a hist_entry__tui_annotate(), that does some
term setup before calling this "new" __hist_entry__tui_annotate().
Looks confusing tho :-\
- Arnaldo
> 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.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/annotate.c | 17 +++++++----------
> tools/perf/ui/browsers/hists.c | 2 +-
> tools/perf/util/annotate.h | 12 ------------
> tools/perf/util/hist.h | 12 +++++++-----
> 4 files changed, 15 insertions(+), 28 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 183902dac042ecb0..28ef146f29e8e742 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -27,6 +27,7 @@ struct annotate_browser {
> struct rb_node *curr_hot;
> struct annotation_line *selection;
> struct arch *arch;
> + struct hist_entry *he;
> bool searching_backwards;
> char search_bf[128];
> };
> @@ -557,7 +558,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 +1033,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 +1041,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 +1060,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;
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index d26b925e3d7f46af..55455c49faf01891 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 c64254088fc77246..11ae738772ca4f61 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -712,8 +712,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);
> @@ -741,9 +742,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.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 17:27 ` Namhyung Kim
@ 2025-07-16 20:42 ` Ian Rogers
2025-07-23 16:10 ` Arnaldo Carvalho de Melo
2025-07-25 18:28 ` Namhyung Kim
0 siblings, 2 replies; 19+ messages in thread
From: Ian Rogers @ 2025-07-16 20:42 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Wed, Jul 16, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Jul 16, 2025 at 08:00:37AM -0700, Ian Rogers wrote:
> > On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> 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.
> > > Make disasm_line__write() return the number of printed characters so
> > > that it can skip unnecessary operations when the screen is full.
> > >
> > > 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 | 61 +++++++++++++++++++---
> > > 4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
> > > @@ -1101,6 +1102,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;
> > > @@ -1111,6 +1115,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 d69e406c1bc289cd..06ddc7a9f58722a4 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)));
> >
> > This API looks unfortunate. A dso may have a long name (it'd be easier
> > to understand if this were called path rather than long name) or a
> > build ID. The API isn't the fault of this change, but I thought I'd
> > mention as we move toward greater use of build IDs.
>
> Are you talking about build-id in MMAP2? I think it's build-id vs. (dev
> major/minor + inode) and the long name should be available always.
I'm thinking of work I'm doing with build IDs like (unmerged):
https://lore.kernel.org/lkml/20250628045017.1361563-1-irogers@google.com/
Even with mmap2 events the filename shouldn't be necessary as the
build ID should be preferred - if you profile remotely there may be a
file name/path collision on a local machine, but it should be unlikely
for a build ID collision. In those patches the dso_id is changed to
instead of considering inode numbers using build IDs when possible. It
was already the case that the name of a dso could be changed, which
affects sorting. In general I think we should move away from file
paths, inodes and the like as the build IDs will avoid races, work
across systems, etc. I think in this case we could add:
```
apd->dbg = dso__debuginfo(map__dso(apd->he->ms.map))))
```
or possibly just change `apd->dbg` to be the dso and grab the
debuginfo from the dso when needed. For now the dso__debuginfo would
be something like:
```
struct debuginfo *dso__debuginfo(struct dso *dso)
{
debuginfo__delete(dso->debuginfo);
dso->debuginfo = debuginfo__new(dso__long_name(dso));
return dso->debuginfo;
}
```
but in the future we can find the debuginfo off of the build ID and paths, etc.
Thanks,
Ian
> Thanks,
> Namhyung
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate()
2025-07-16 20:16 ` Arnaldo Carvalho de Melo
@ 2025-07-17 0:55 ` Namhyung Kim
0 siblings, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-17 0:55 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, welcome back!
On Wed, Jul 16, 2025 at 05:16:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Jul 15, 2025 at 10:00:47PM -0700, Namhyung Kim wrote:
> > There are three different but similar functions for annotation on TUI.
>
> Why the two initial __? Normally this is when its about work on some
> data structure but the pointer to it isn't passed, but in this case the
> first arg is a 'struct hist_entry *', so calling it
> hist_entry__tui_annotate() would be right?
>
> Well, there is already a hist_entry__tui_annotate(), that does some
> term setup before calling this "new" __hist_entry__tui_annotate().
>
> Looks confusing tho :-\
I thought it's a pattern to have a wrapper to deal with some boilerplate
code like this or locking, and have an internal function (with the "__"
prefix) to do the real work so that it can be called in a different
condition.
But I can rename if you don't feel comfortable with this. Any
suggestion?
Thanks,
Namhyung
>
> - Arnaldo
>
> > 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.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/ui/browsers/annotate.c | 17 +++++++----------
> > tools/perf/ui/browsers/hists.c | 2 +-
> > tools/perf/util/annotate.h | 12 ------------
> > tools/perf/util/hist.h | 12 +++++++-----
> > 4 files changed, 15 insertions(+), 28 deletions(-)
> >
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 183902dac042ecb0..28ef146f29e8e742 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -27,6 +27,7 @@ struct annotate_browser {
> > struct rb_node *curr_hot;
> > struct annotation_line *selection;
> > struct arch *arch;
> > + struct hist_entry *he;
> > bool searching_backwards;
> > char search_bf[128];
> > };
> > @@ -557,7 +558,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 +1033,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 +1041,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 +1060,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;
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index d26b925e3d7f46af..55455c49faf01891 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 c64254088fc77246..11ae738772ca4f61 100644
> > --- a/tools/perf/util/hist.h
> > +++ b/tools/perf/util/hist.h
> > @@ -712,8 +712,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);
> > @@ -741,9 +742,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.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 5:00 ` [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI Namhyung Kim
2025-07-16 15:00 ` Ian Rogers
@ 2025-07-23 16:04 ` Arnaldo Carvalho de Melo
2025-07-23 16:20 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-07-23 16:04 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Tue, Jul 15, 2025 at 10:00:51PM -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.
> Make disasm_line__write() return the number of printed characters so
> that it can skip unnecessary operations when the screen is full.
>
> 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 | 61 +++++++++++++++++++---
> 4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
> @@ -1101,6 +1102,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));
Some error checking here to tell the user if debuginfo isn't available
and hints on how to get it in place?
> +
> browser.b.width = notes->src->widths.max_line_len;
> browser.b.nr_entries = notes->src->nr_entries;
> browser.b.entries = ¬es->src->source;
> @@ -1111,6 +1115,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);
This is a local variable, so no need to zero browser.dbg after deleting
it, ok.
> 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 d69e406c1bc289cd..06ddc7a9f58722a4 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);
But here it would be good to nullify apd->dbg?
> +
> return 0;
> }
>
> @@ -1743,7 +1751,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 +1779,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);
> }
>
> static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
> @@ -2116,11 +2124,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>
> width -= printed + 3;
>
> - 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", bf);
> + width -= printed;
> +
> + if (annotate_opts.code_with_type && apd->dbg) {
> + struct annotated_data_type *data_type;
> + int offset = 0;
> +
> + data_type = __hist_entry__get_data_type(apd->he, apd->arch,
> + apd->dbg,
> + disasm_line(al),
> + &offset);
> + if (data_type && data_type != NO_TYPE) {
> + char member[256];
> +
> + printed = scnprintf(bf, sizeof(bf),
> + "\t\t# data-type: %s",
> + data_type->self.type_name);
>
> - obj__printf(obj, "%-*s", width, bf);
> + if (data_type != &stackop_type &&
> + data_type != &canary_type &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " +%#x", offset);
> + }
> +
> + if (annotated_data_type__get_member_name(data_type,
> + member,
> + sizeof(member),
> + offset) &&
> + sizeof(bf) > (size_t)printed) {
> + printed += scnprintf(bf + printed,
> + sizeof(bf) - printed,
> + " (%s)", member);
> + }
>
> - (void)apd;
> + obj__printf(obj, "%-*s", width, bf);
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> + } else {
> + obj__printf(obj, "%-*s", width, " ");
> + }
> }
>
> }
> --
> 2.50.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 20:42 ` Ian Rogers
@ 2025-07-23 16:10 ` Arnaldo Carvalho de Melo
2025-07-25 18:28 ` Namhyung Kim
1 sibling, 0 replies; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-07-23 16:10 UTC (permalink / raw)
To: Ian Rogers
Cc: Namhyung Kim, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Wed, Jul 16, 2025 at 01:42:13PM -0700, Ian Rogers wrote:
> affects sorting. In general I think we should move away from file
> paths, inodes and the like as the build IDs will avoid races, work
> across systems, etc. I think in this case we could add:
<SNIP>
> but in the future we can find the debuginfo off of the build ID and paths, etc.
Yeah, agreed, we better move to make build-id the primary means of
getting what is needed, when that isn't possible, fall back to
filenames.
Then we should check if the hostname in the perf.data header is the same
as the machine running to warn the user about that, other options are to
check the time the perf.data file was created to warn that its likely
updates took place, so take the results with a grain of salt, etc.
If the architecture or distro is different, things we can also see from
the perf.data files, outright refuse to use filenames :-)
- Arnaldo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-23 16:04 ` Arnaldo Carvalho de Melo
@ 2025-07-23 16:20 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-07-23 16:20 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, Jul 23, 2025 at 01:04:11PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Jul 15, 2025 at 10:00:51PM -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.
> > Make disasm_line__write() return the number of printed characters so
> > that it can skip unnecessary operations when the screen is full.
> >
> > 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 | 61 +++++++++++++++++++---
> > 4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
> > @@ -1101,6 +1102,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));
>
> Some error checking here to tell the user if debuginfo isn't available
> and hints on how to get it in place?
Ok, you did it later with:
commit 81d638a1b96ec04d7c41e163b5077419cf85e798 (HEAD)
Author: Namhyung Kim <namhyung@kernel.org>
Date: Tue Jul 15 22:00:53 2025 -0700
perf annotate: Show warning when debuginfo is not available
When user requests data-type annotation but no DWARF info is available,
show a warning message about it.
- Arnaldo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI
2025-07-16 20:42 ` Ian Rogers
2025-07-23 16:10 ` Arnaldo Carvalho de Melo
@ 2025-07-25 18:28 ` Namhyung Kim
1 sibling, 0 replies; 19+ messages in thread
From: Namhyung Kim @ 2025-07-25 18:28 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Wed, Jul 16, 2025 at 01:42:13PM -0700, Ian Rogers wrote:
> On Wed, Jul 16, 2025 at 10:27 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Wed, Jul 16, 2025 at 08:00:37AM -0700, Ian Rogers wrote:
> > > On Tue, Jul 15, 2025 at 10:01 PM Namhyung Kim <namhyung@kernel.org> 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.
> > > > Make disasm_line__write() return the number of printed characters so
> > > > that it can skip unnecessary operations when the screen is full.
> > > >
> > > > 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 | 61 +++++++++++++++++++---
> > > > 4 files changed, 61 insertions(+), 12 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 9833c2c82a2fee46..6debd725392db4a4 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 23bea5b165774ae7..cdee1969f3131a7c 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"
> > > > @@ -1101,6 +1102,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;
> > > > @@ -1111,6 +1115,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 d69e406c1bc289cd..06ddc7a9f58722a4 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)));
> > >
> > > This API looks unfortunate. A dso may have a long name (it'd be easier
> > > to understand if this were called path rather than long name) or a
> > > build ID. The API isn't the fault of this change, but I thought I'd
> > > mention as we move toward greater use of build IDs.
> >
> > Are you talking about build-id in MMAP2? I think it's build-id vs. (dev
> > major/minor + inode) and the long name should be available always.
>
> I'm thinking of work I'm doing with build IDs like (unmerged):
> https://lore.kernel.org/lkml/20250628045017.1361563-1-irogers@google.com/
> Even with mmap2 events the filename shouldn't be necessary as the
> build ID should be preferred - if you profile remotely there may be a
> file name/path collision on a local machine, but it should be unlikely
> for a build ID collision. In those patches the dso_id is changed to
> instead of considering inode numbers using build IDs when possible. It
> was already the case that the name of a dso could be changed, which
> affects sorting. In general I think we should move away from file
> paths, inodes and the like as the build IDs will avoid races, work
> across systems, etc. I think in this case we could add:
> ```
> apd->dbg = dso__debuginfo(map__dso(apd->he->ms.map))))
> ```
> or possibly just change `apd->dbg` to be the dso and grab the
> debuginfo from the dso when needed. For now the dso__debuginfo would
> be something like:
> ```
> struct debuginfo *dso__debuginfo(struct dso *dso)
> {
> debuginfo__delete(dso->debuginfo);
> dso->debuginfo = debuginfo__new(dso__long_name(dso));
> return dso->debuginfo;
> }
> ```
> but in the future we can find the debuginfo off of the build ID and paths, etc.
Looks good, will add this change on top.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-07-25 18:28 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-16 5:00 [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 1/8] perf annotate: Rename to __hist_entry__tui_annotate() Namhyung Kim
2025-07-16 20:16 ` Arnaldo Carvalho de Melo
2025-07-17 0:55 ` Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 2/8] perf annotate: Remove __annotation_line__write() Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 3/8] perf annotate: Pass annotation_print_data to annotation_line__write() Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 4/8] perf annotate: Simplify width calculation in annotation_line__write() Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 5/8] perf annotate: Add --code-with-type support for TUI Namhyung Kim
2025-07-16 15:00 ` Ian Rogers
2025-07-16 17:27 ` Namhyung Kim
2025-07-16 20:42 ` Ian Rogers
2025-07-23 16:10 ` Arnaldo Carvalho de Melo
2025-07-25 18:28 ` Namhyung Kim
2025-07-23 16:04 ` Arnaldo Carvalho de Melo
2025-07-23 16:20 ` Arnaldo Carvalho de Melo
2025-07-16 5:00 ` [PATCH v3 6/8] perf annotate: Add 'T' hot key to toggle data type display Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 7/8] perf annotate: Show warning when debuginfo is not available Namhyung Kim
2025-07-16 5:00 ` [PATCH v3 8/8] perf annotate: Hide data-type for stack operation and canary Namhyung Kim
2025-07-16 15:02 ` [PATCHSET v3 0/8] perf annotate: Support --code-with-type on TUI Ian Rogers
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).