* [PATCH v3] perf annotate: Fix source code annotate with objdump
@ 2025-06-25 23:03 Namhyung Kim
2025-06-26 15:23 ` Ian Rogers
2025-06-27 18:53 ` Namhyung Kim
0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2025-06-25 23:03 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
Recently it uses llvm and capstone to speed up annotation or disassembly
of instructions. But they don't support source code view yet. Until it
fixed, we can force to use objdump for source code annotation.
To prevent performance loss, it's disabled by default and turned it on
when user requests it in TUI by pressing 's' key.
Link: https://lore.kernel.org/r/20250608004613.238291-1-namhyung@kernel.org
Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
v3)
* show warning when there's no source code
tools/perf/ui/browsers/annotate.c | 86 +++++++++++++++++++++++++++++--
tools/perf/util/annotate.c | 2 +
tools/perf/util/annotate.h | 1 +
tools/perf/util/disasm.c | 7 +++
4 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index ab776b1ed2d5b4ba..183902dac042ecb0 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -345,6 +345,23 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
browser->curr_hot = rb_last(&browser->entries);
}
+static struct annotation_line *annotate_browser__find_new_asm_line(
+ struct annotate_browser *browser,
+ int idx_asm)
+{
+ struct annotation_line *al;
+ struct list_head *head = browser->b.entries;
+
+ /* find an annotation line in the new list with the same idx_asm */
+ list_for_each_entry(al, head, node) {
+ if (al->idx_asm == idx_asm)
+ return al;
+ }
+
+ /* There are no asm lines */
+ return NULL;
+}
+
static struct annotation_line *annotate_browser__find_next_asm_line(
struct annotate_browser *browser,
struct annotation_line *al)
@@ -368,7 +385,31 @@ static struct annotation_line *annotate_browser__find_next_asm_line(
return NULL;
}
-static bool annotate_browser__toggle_source(struct annotate_browser *browser)
+static bool annotation__has_source(struct annotation *notes)
+{
+ struct annotation_line *al;
+ bool found_asm = false;
+
+ /* Let's skip the first non-asm lines which present regardless of source. */
+ list_for_each_entry(al, ¬es->src->source, node) {
+ if (al->offset >= 0) {
+ found_asm = true;
+ break;
+ }
+ }
+
+ if (found_asm) {
+ /* After assembly lines, any line without offset means source. */
+ list_for_each_entry_continue(al, ¬es->src->source, node) {
+ if (al->offset == -1)
+ return true;
+ }
+ }
+ return false;
+}
+
+static bool annotate_browser__toggle_source(struct annotate_browser *browser,
+ struct evsel *evsel)
{
struct annotation *notes = browser__annotation(&browser->b);
struct annotation_line *al;
@@ -377,6 +418,39 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
browser->b.seek(&browser->b, offset, SEEK_CUR);
al = list_entry(browser->b.top, struct annotation_line, node);
+ if (!annotate_opts.annotate_src)
+ annotate_opts.annotate_src = true;
+
+ /*
+ * It's about to get source code annotation for the first time.
+ * Drop the existing annotation_lines and get the new one with source.
+ * And then move to the original line at the same asm index.
+ */
+ if (annotate_opts.hide_src_code && !notes->src->tried_source) {
+ struct map_symbol *ms = browser->b.priv;
+ int orig_idx_asm = al->idx_asm;
+
+ /* annotate again with source code info */
+ annotate_opts.hide_src_code = false;
+ annotated_source__purge(notes->src);
+ symbol__annotate2(ms, evsel, &browser->arch);
+ annotate_opts.hide_src_code = true;
+
+ /* should be after annotated_source__purge() */
+ notes->src->tried_source = true;
+
+ if (!annotation__has_source(notes))
+ ui__warning("Annotation has no source code.");
+
+ browser->b.entries = ¬es->src->source;
+ al = annotate_browser__find_new_asm_line(browser, orig_idx_asm);
+ if (unlikely(al == NULL)) {
+ al = list_first_entry(¬es->src->source,
+ struct annotation_line, node);
+ }
+ browser->b.seek(&browser->b, al->idx_asm, SEEK_SET);
+ }
+
if (annotate_opts.hide_src_code) {
if (al->idx_asm < offset)
offset = al->idx;
@@ -833,7 +907,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
nd = browser->curr_hot;
break;
case 's':
- if (annotate_browser__toggle_source(browser))
+ if (annotate_browser__toggle_source(browser, evsel))
ui_helpline__puts(help);
annotate__scnprintf_title(hists, title, sizeof(title));
annotate_browser__show(&browser->b, title, help);
@@ -1011,6 +1085,12 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
return -1;
}
+
+ if (!annotate_opts.hide_src_code) {
+ notes->src->tried_source = true;
+ if (!annotation__has_source(notes))
+ ui__warning("Annotation has no source code.");
+ }
}
ui_helpline__push("Press ESC to exit");
@@ -1025,7 +1105,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
ret = annotate_browser__run(&browser, evsel, hbt);
- if(not_annotated)
+ if (not_annotated && !notes->src->tried_source)
annotated_source__purge(notes->src);
return ret;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 264a212b47df850c..0dd475a744b6dfac 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1451,6 +1451,7 @@ void annotated_source__purge(struct annotated_source *as)
list_del_init(&al->node);
disasm_line__free(disasm_line(al));
}
+ as->tried_source = false;
}
static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
@@ -2280,6 +2281,7 @@ void annotation_options__init(void)
opt->annotate_src = true;
opt->offset_level = ANNOTATION__OFFSET_JUMP_TARGETS;
opt->percent_type = PERCENT_PERIOD_LOCAL;
+ opt->hide_src_code = true;
opt->hide_src_code_on_title = true;
}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index bbb89b32f398b3c9..8b5131d257b01e3e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -294,6 +294,7 @@ struct annotated_source {
int nr_entries;
int nr_asm_entries;
int max_jump_sources;
+ bool tried_source;
u64 start;
struct {
u8 addr;
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 8f0eb56c6fc66a96..ff475a239f4b0db4 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -2284,6 +2284,13 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
}
}
+ /* FIXME: LLVM and CAPSTONE should support source code */
+ if (options->annotate_src && !options->hide_src_code) {
+ err = symbol__disassemble_objdump(symfs_filename, sym, args);
+ if (err == 0)
+ goto out_remove_tmp;
+ }
+
err = -1;
for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
enum perf_disassembler dis = options->disassemblers[i];
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] perf annotate: Fix source code annotate with objdump
2025-06-25 23:03 [PATCH v3] perf annotate: Fix source code annotate with objdump Namhyung Kim
@ 2025-06-26 15:23 ` Ian Rogers
2025-06-26 17:50 ` Namhyung Kim
2025-06-27 18:53 ` Namhyung Kim
1 sibling, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2025-06-26 15:23 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, Jun 25, 2025 at 4:03 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Recently it uses llvm and capstone to speed up annotation or disassembly
> of instructions. But they don't support source code view yet. Until it
> fixed, we can force to use objdump for source code annotation.
>
> To prevent performance loss, it's disabled by default and turned it on
> when user requests it in TUI by pressing 's' key.
>
> Link: https://lore.kernel.org/r/20250608004613.238291-1-namhyung@kernel.org
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Can we move this series forward:
https://lore.kernel.org/lkml/20250417230740.86048-1-irogers@google.com/
There was some build testing by Arnaldo but then things became
stalled. I'm guessing I'll need to send a rebase.
Thanks,
Ian
> ---
> v3)
> * show warning when there's no source code
>
> tools/perf/ui/browsers/annotate.c | 86 +++++++++++++++++++++++++++++--
> tools/perf/util/annotate.c | 2 +
> tools/perf/util/annotate.h | 1 +
> tools/perf/util/disasm.c | 7 +++
> 4 files changed, 93 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index ab776b1ed2d5b4ba..183902dac042ecb0 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -345,6 +345,23 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> browser->curr_hot = rb_last(&browser->entries);
> }
>
> +static struct annotation_line *annotate_browser__find_new_asm_line(
> + struct annotate_browser *browser,
> + int idx_asm)
> +{
> + struct annotation_line *al;
> + struct list_head *head = browser->b.entries;
> +
> + /* find an annotation line in the new list with the same idx_asm */
> + list_for_each_entry(al, head, node) {
> + if (al->idx_asm == idx_asm)
> + return al;
> + }
> +
> + /* There are no asm lines */
> + return NULL;
> +}
> +
> static struct annotation_line *annotate_browser__find_next_asm_line(
> struct annotate_browser *browser,
> struct annotation_line *al)
> @@ -368,7 +385,31 @@ static struct annotation_line *annotate_browser__find_next_asm_line(
> return NULL;
> }
>
> -static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> +static bool annotation__has_source(struct annotation *notes)
> +{
> + struct annotation_line *al;
> + bool found_asm = false;
> +
> + /* Let's skip the first non-asm lines which present regardless of source. */
> + list_for_each_entry(al, ¬es->src->source, node) {
> + if (al->offset >= 0) {
> + found_asm = true;
> + break;
> + }
> + }
> +
> + if (found_asm) {
> + /* After assembly lines, any line without offset means source. */
> + list_for_each_entry_continue(al, ¬es->src->source, node) {
> + if (al->offset == -1)
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static bool annotate_browser__toggle_source(struct annotate_browser *browser,
> + struct evsel *evsel)
> {
> struct annotation *notes = browser__annotation(&browser->b);
> struct annotation_line *al;
> @@ -377,6 +418,39 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> browser->b.seek(&browser->b, offset, SEEK_CUR);
> al = list_entry(browser->b.top, struct annotation_line, node);
>
> + if (!annotate_opts.annotate_src)
> + annotate_opts.annotate_src = true;
> +
> + /*
> + * It's about to get source code annotation for the first time.
> + * Drop the existing annotation_lines and get the new one with source.
> + * And then move to the original line at the same asm index.
> + */
> + if (annotate_opts.hide_src_code && !notes->src->tried_source) {
> + struct map_symbol *ms = browser->b.priv;
> + int orig_idx_asm = al->idx_asm;
> +
> + /* annotate again with source code info */
> + annotate_opts.hide_src_code = false;
> + annotated_source__purge(notes->src);
> + symbol__annotate2(ms, evsel, &browser->arch);
> + annotate_opts.hide_src_code = true;
> +
> + /* should be after annotated_source__purge() */
> + notes->src->tried_source = true;
> +
> + if (!annotation__has_source(notes))
> + ui__warning("Annotation has no source code.");
> +
> + browser->b.entries = ¬es->src->source;
> + al = annotate_browser__find_new_asm_line(browser, orig_idx_asm);
> + if (unlikely(al == NULL)) {
> + al = list_first_entry(¬es->src->source,
> + struct annotation_line, node);
> + }
> + browser->b.seek(&browser->b, al->idx_asm, SEEK_SET);
> + }
> +
> if (annotate_opts.hide_src_code) {
> if (al->idx_asm < offset)
> offset = al->idx;
> @@ -833,7 +907,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> nd = browser->curr_hot;
> break;
> case 's':
> - if (annotate_browser__toggle_source(browser))
> + if (annotate_browser__toggle_source(browser, evsel))
> ui_helpline__puts(help);
> annotate__scnprintf_title(hists, title, sizeof(title));
> annotate_browser__show(&browser->b, title, help);
> @@ -1011,6 +1085,12 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
> return -1;
> }
> +
> + if (!annotate_opts.hide_src_code) {
> + notes->src->tried_source = true;
> + if (!annotation__has_source(notes))
> + ui__warning("Annotation has no source code.");
> + }
> }
>
> ui_helpline__push("Press ESC to exit");
> @@ -1025,7 +1105,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
>
> ret = annotate_browser__run(&browser, evsel, hbt);
>
> - if(not_annotated)
> + if (not_annotated && !notes->src->tried_source)
> annotated_source__purge(notes->src);
>
> return ret;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 264a212b47df850c..0dd475a744b6dfac 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1451,6 +1451,7 @@ void annotated_source__purge(struct annotated_source *as)
> list_del_init(&al->node);
> disasm_line__free(disasm_line(al));
> }
> + as->tried_source = false;
> }
>
> static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
> @@ -2280,6 +2281,7 @@ void annotation_options__init(void)
> opt->annotate_src = true;
> opt->offset_level = ANNOTATION__OFFSET_JUMP_TARGETS;
> opt->percent_type = PERCENT_PERIOD_LOCAL;
> + opt->hide_src_code = true;
> opt->hide_src_code_on_title = true;
> }
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index bbb89b32f398b3c9..8b5131d257b01e3e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -294,6 +294,7 @@ struct annotated_source {
> int nr_entries;
> int nr_asm_entries;
> int max_jump_sources;
> + bool tried_source;
> u64 start;
> struct {
> u8 addr;
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 8f0eb56c6fc66a96..ff475a239f4b0db4 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -2284,6 +2284,13 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> }
> }
>
> + /* FIXME: LLVM and CAPSTONE should support source code */
> + if (options->annotate_src && !options->hide_src_code) {
> + err = symbol__disassemble_objdump(symfs_filename, sym, args);
> + if (err == 0)
> + goto out_remove_tmp;
> + }
> +
> err = -1;
> for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
> enum perf_disassembler dis = options->disassemblers[i];
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] perf annotate: Fix source code annotate with objdump
2025-06-26 15:23 ` Ian Rogers
@ 2025-06-26 17:50 ` Namhyung Kim
2025-06-26 21:23 ` Ian Rogers
0 siblings, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2025-06-26 17:50 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
Hi Ian,
On Thu, Jun 26, 2025 at 08:23:30AM -0700, Ian Rogers wrote:
> On Wed, Jun 25, 2025 at 4:03 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Recently it uses llvm and capstone to speed up annotation or disassembly
> > of instructions. But they don't support source code view yet. Until it
> > fixed, we can force to use objdump for source code annotation.
> >
> > To prevent performance loss, it's disabled by default and turned it on
> > when user requests it in TUI by pressing 's' key.
> >
> > Link: https://lore.kernel.org/r/20250608004613.238291-1-namhyung@kernel.org
> > Reported-by: Ingo Molnar <mingo@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> Can we move this series forward:
> https://lore.kernel.org/lkml/20250417230740.86048-1-irogers@google.com/
> There was some build testing by Arnaldo but then things became
> stalled. I'm guessing I'll need to send a rebase.
I'll take a look. Is there any thing related to this patchset?
It'd be nice if we can land this first.
Thanks,
Namhyung
>
> > ---
> > v3)
> > * show warning when there's no source code
> >
> > tools/perf/ui/browsers/annotate.c | 86 +++++++++++++++++++++++++++++--
> > tools/perf/util/annotate.c | 2 +
> > tools/perf/util/annotate.h | 1 +
> > tools/perf/util/disasm.c | 7 +++
> > 4 files changed, 93 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index ab776b1ed2d5b4ba..183902dac042ecb0 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -345,6 +345,23 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> > browser->curr_hot = rb_last(&browser->entries);
> > }
> >
> > +static struct annotation_line *annotate_browser__find_new_asm_line(
> > + struct annotate_browser *browser,
> > + int idx_asm)
> > +{
> > + struct annotation_line *al;
> > + struct list_head *head = browser->b.entries;
> > +
> > + /* find an annotation line in the new list with the same idx_asm */
> > + list_for_each_entry(al, head, node) {
> > + if (al->idx_asm == idx_asm)
> > + return al;
> > + }
> > +
> > + /* There are no asm lines */
> > + return NULL;
> > +}
> > +
> > static struct annotation_line *annotate_browser__find_next_asm_line(
> > struct annotate_browser *browser,
> > struct annotation_line *al)
> > @@ -368,7 +385,31 @@ static struct annotation_line *annotate_browser__find_next_asm_line(
> > return NULL;
> > }
> >
> > -static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> > +static bool annotation__has_source(struct annotation *notes)
> > +{
> > + struct annotation_line *al;
> > + bool found_asm = false;
> > +
> > + /* Let's skip the first non-asm lines which present regardless of source. */
> > + list_for_each_entry(al, ¬es->src->source, node) {
> > + if (al->offset >= 0) {
> > + found_asm = true;
> > + break;
> > + }
> > + }
> > +
> > + if (found_asm) {
> > + /* After assembly lines, any line without offset means source. */
> > + list_for_each_entry_continue(al, ¬es->src->source, node) {
> > + if (al->offset == -1)
> > + return true;
> > + }
> > + }
> > + return false;
> > +}
> > +
> > +static bool annotate_browser__toggle_source(struct annotate_browser *browser,
> > + struct evsel *evsel)
> > {
> > struct annotation *notes = browser__annotation(&browser->b);
> > struct annotation_line *al;
> > @@ -377,6 +418,39 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> > browser->b.seek(&browser->b, offset, SEEK_CUR);
> > al = list_entry(browser->b.top, struct annotation_line, node);
> >
> > + if (!annotate_opts.annotate_src)
> > + annotate_opts.annotate_src = true;
> > +
> > + /*
> > + * It's about to get source code annotation for the first time.
> > + * Drop the existing annotation_lines and get the new one with source.
> > + * And then move to the original line at the same asm index.
> > + */
> > + if (annotate_opts.hide_src_code && !notes->src->tried_source) {
> > + struct map_symbol *ms = browser->b.priv;
> > + int orig_idx_asm = al->idx_asm;
> > +
> > + /* annotate again with source code info */
> > + annotate_opts.hide_src_code = false;
> > + annotated_source__purge(notes->src);
> > + symbol__annotate2(ms, evsel, &browser->arch);
> > + annotate_opts.hide_src_code = true;
> > +
> > + /* should be after annotated_source__purge() */
> > + notes->src->tried_source = true;
> > +
> > + if (!annotation__has_source(notes))
> > + ui__warning("Annotation has no source code.");
> > +
> > + browser->b.entries = ¬es->src->source;
> > + al = annotate_browser__find_new_asm_line(browser, orig_idx_asm);
> > + if (unlikely(al == NULL)) {
> > + al = list_first_entry(¬es->src->source,
> > + struct annotation_line, node);
> > + }
> > + browser->b.seek(&browser->b, al->idx_asm, SEEK_SET);
> > + }
> > +
> > if (annotate_opts.hide_src_code) {
> > if (al->idx_asm < offset)
> > offset = al->idx;
> > @@ -833,7 +907,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > nd = browser->curr_hot;
> > break;
> > case 's':
> > - if (annotate_browser__toggle_source(browser))
> > + if (annotate_browser__toggle_source(browser, evsel))
> > ui_helpline__puts(help);
> > annotate__scnprintf_title(hists, title, sizeof(title));
> > annotate_browser__show(&browser->b, title, help);
> > @@ -1011,6 +1085,12 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
> > return -1;
> > }
> > +
> > + if (!annotate_opts.hide_src_code) {
> > + notes->src->tried_source = true;
> > + if (!annotation__has_source(notes))
> > + ui__warning("Annotation has no source code.");
> > + }
> > }
> >
> > ui_helpline__push("Press ESC to exit");
> > @@ -1025,7 +1105,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> >
> > ret = annotate_browser__run(&browser, evsel, hbt);
> >
> > - if(not_annotated)
> > + if (not_annotated && !notes->src->tried_source)
> > annotated_source__purge(notes->src);
> >
> > return ret;
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 264a212b47df850c..0dd475a744b6dfac 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -1451,6 +1451,7 @@ void annotated_source__purge(struct annotated_source *as)
> > list_del_init(&al->node);
> > disasm_line__free(disasm_line(al));
> > }
> > + as->tried_source = false;
> > }
> >
> > static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
> > @@ -2280,6 +2281,7 @@ void annotation_options__init(void)
> > opt->annotate_src = true;
> > opt->offset_level = ANNOTATION__OFFSET_JUMP_TARGETS;
> > opt->percent_type = PERCENT_PERIOD_LOCAL;
> > + opt->hide_src_code = true;
> > opt->hide_src_code_on_title = true;
> > }
> >
> > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> > index bbb89b32f398b3c9..8b5131d257b01e3e 100644
> > --- a/tools/perf/util/annotate.h
> > +++ b/tools/perf/util/annotate.h
> > @@ -294,6 +294,7 @@ struct annotated_source {
> > int nr_entries;
> > int nr_asm_entries;
> > int max_jump_sources;
> > + bool tried_source;
> > u64 start;
> > struct {
> > u8 addr;
> > diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> > index 8f0eb56c6fc66a96..ff475a239f4b0db4 100644
> > --- a/tools/perf/util/disasm.c
> > +++ b/tools/perf/util/disasm.c
> > @@ -2284,6 +2284,13 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> > }
> > }
> >
> > + /* FIXME: LLVM and CAPSTONE should support source code */
> > + if (options->annotate_src && !options->hide_src_code) {
> > + err = symbol__disassemble_objdump(symfs_filename, sym, args);
> > + if (err == 0)
> > + goto out_remove_tmp;
> > + }
> > +
> > err = -1;
> > for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
> > enum perf_disassembler dis = options->disassemblers[i];
> > --
> > 2.50.0.727.gbf7dc18ff4-goog
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] perf annotate: Fix source code annotate with objdump
2025-06-26 17:50 ` Namhyung Kim
@ 2025-06-26 21:23 ` Ian Rogers
0 siblings, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2025-06-26 21:23 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 Thu, Jun 26, 2025 at 10:50 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Thu, Jun 26, 2025 at 08:23:30AM -0700, Ian Rogers wrote:
> > On Wed, Jun 25, 2025 at 4:03 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > Recently it uses llvm and capstone to speed up annotation or disassembly
> > > of instructions. But they don't support source code view yet. Until it
> > > fixed, we can force to use objdump for source code annotation.
> > >
> > > To prevent performance loss, it's disabled by default and turned it on
> > > when user requests it in TUI by pressing 's' key.
> > >
> > > Link: https://lore.kernel.org/r/20250608004613.238291-1-namhyung@kernel.org
> > > Reported-by: Ingo Molnar <mingo@kernel.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >
> > Can we move this series forward:
> > https://lore.kernel.org/lkml/20250417230740.86048-1-irogers@google.com/
> > There was some build testing by Arnaldo but then things became
> > stalled. I'm guessing I'll need to send a rebase.
>
> I'll take a look. Is there any thing related to this patchset?
> It'd be nice if we can land this first.
Ok, I'll rebase those changes after this lands.
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> Thanks,
> Namhyung
>
> >
> > > ---
> > > v3)
> > > * show warning when there's no source code
> > >
> > > tools/perf/ui/browsers/annotate.c | 86 +++++++++++++++++++++++++++++--
> > > tools/perf/util/annotate.c | 2 +
> > > tools/perf/util/annotate.h | 1 +
> > > tools/perf/util/disasm.c | 7 +++
> > > 4 files changed, 93 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > > index ab776b1ed2d5b4ba..183902dac042ecb0 100644
> > > --- a/tools/perf/ui/browsers/annotate.c
> > > +++ b/tools/perf/ui/browsers/annotate.c
> > > @@ -345,6 +345,23 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> > > browser->curr_hot = rb_last(&browser->entries);
> > > }
> > >
> > > +static struct annotation_line *annotate_browser__find_new_asm_line(
> > > + struct annotate_browser *browser,
> > > + int idx_asm)
> > > +{
> > > + struct annotation_line *al;
> > > + struct list_head *head = browser->b.entries;
> > > +
> > > + /* find an annotation line in the new list with the same idx_asm */
> > > + list_for_each_entry(al, head, node) {
> > > + if (al->idx_asm == idx_asm)
> > > + return al;
> > > + }
> > > +
> > > + /* There are no asm lines */
> > > + return NULL;
> > > +}
> > > +
> > > static struct annotation_line *annotate_browser__find_next_asm_line(
> > > struct annotate_browser *browser,
> > > struct annotation_line *al)
> > > @@ -368,7 +385,31 @@ static struct annotation_line *annotate_browser__find_next_asm_line(
> > > return NULL;
> > > }
> > >
> > > -static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> > > +static bool annotation__has_source(struct annotation *notes)
> > > +{
> > > + struct annotation_line *al;
> > > + bool found_asm = false;
> > > +
> > > + /* Let's skip the first non-asm lines which present regardless of source. */
> > > + list_for_each_entry(al, ¬es->src->source, node) {
> > > + if (al->offset >= 0) {
> > > + found_asm = true;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (found_asm) {
> > > + /* After assembly lines, any line without offset means source. */
> > > + list_for_each_entry_continue(al, ¬es->src->source, node) {
> > > + if (al->offset == -1)
> > > + return true;
> > > + }
> > > + }
> > > + return false;
> > > +}
> > > +
> > > +static bool annotate_browser__toggle_source(struct annotate_browser *browser,
> > > + struct evsel *evsel)
> > > {
> > > struct annotation *notes = browser__annotation(&browser->b);
> > > struct annotation_line *al;
> > > @@ -377,6 +418,39 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> > > browser->b.seek(&browser->b, offset, SEEK_CUR);
> > > al = list_entry(browser->b.top, struct annotation_line, node);
> > >
> > > + if (!annotate_opts.annotate_src)
> > > + annotate_opts.annotate_src = true;
> > > +
> > > + /*
> > > + * It's about to get source code annotation for the first time.
> > > + * Drop the existing annotation_lines and get the new one with source.
> > > + * And then move to the original line at the same asm index.
> > > + */
> > > + if (annotate_opts.hide_src_code && !notes->src->tried_source) {
> > > + struct map_symbol *ms = browser->b.priv;
> > > + int orig_idx_asm = al->idx_asm;
> > > +
> > > + /* annotate again with source code info */
> > > + annotate_opts.hide_src_code = false;
> > > + annotated_source__purge(notes->src);
> > > + symbol__annotate2(ms, evsel, &browser->arch);
> > > + annotate_opts.hide_src_code = true;
> > > +
> > > + /* should be after annotated_source__purge() */
> > > + notes->src->tried_source = true;
> > > +
> > > + if (!annotation__has_source(notes))
> > > + ui__warning("Annotation has no source code.");
> > > +
> > > + browser->b.entries = ¬es->src->source;
> > > + al = annotate_browser__find_new_asm_line(browser, orig_idx_asm);
> > > + if (unlikely(al == NULL)) {
> > > + al = list_first_entry(¬es->src->source,
> > > + struct annotation_line, node);
> > > + }
> > > + browser->b.seek(&browser->b, al->idx_asm, SEEK_SET);
> > > + }
> > > +
> > > if (annotate_opts.hide_src_code) {
> > > if (al->idx_asm < offset)
> > > offset = al->idx;
> > > @@ -833,7 +907,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > > nd = browser->curr_hot;
> > > break;
> > > case 's':
> > > - if (annotate_browser__toggle_source(browser))
> > > + if (annotate_browser__toggle_source(browser, evsel))
> > > ui_helpline__puts(help);
> > > annotate__scnprintf_title(hists, title, sizeof(title));
> > > annotate_browser__show(&browser->b, title, help);
> > > @@ -1011,6 +1085,12 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > > ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
> > > return -1;
> > > }
> > > +
> > > + if (!annotate_opts.hide_src_code) {
> > > + notes->src->tried_source = true;
> > > + if (!annotation__has_source(notes))
> > > + ui__warning("Annotation has no source code.");
> > > + }
> > > }
> > >
> > > ui_helpline__push("Press ESC to exit");
> > > @@ -1025,7 +1105,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
> > >
> > > ret = annotate_browser__run(&browser, evsel, hbt);
> > >
> > > - if(not_annotated)
> > > + if (not_annotated && !notes->src->tried_source)
> > > annotated_source__purge(notes->src);
> > >
> > > return ret;
> > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > > index 264a212b47df850c..0dd475a744b6dfac 100644
> > > --- a/tools/perf/util/annotate.c
> > > +++ b/tools/perf/util/annotate.c
> > > @@ -1451,6 +1451,7 @@ void annotated_source__purge(struct annotated_source *as)
> > > list_del_init(&al->node);
> > > disasm_line__free(disasm_line(al));
> > > }
> > > + as->tried_source = false;
> > > }
> > >
> > > static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
> > > @@ -2280,6 +2281,7 @@ void annotation_options__init(void)
> > > opt->annotate_src = true;
> > > opt->offset_level = ANNOTATION__OFFSET_JUMP_TARGETS;
> > > opt->percent_type = PERCENT_PERIOD_LOCAL;
> > > + opt->hide_src_code = true;
> > > opt->hide_src_code_on_title = true;
> > > }
> > >
> > > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> > > index bbb89b32f398b3c9..8b5131d257b01e3e 100644
> > > --- a/tools/perf/util/annotate.h
> > > +++ b/tools/perf/util/annotate.h
> > > @@ -294,6 +294,7 @@ struct annotated_source {
> > > int nr_entries;
> > > int nr_asm_entries;
> > > int max_jump_sources;
> > > + bool tried_source;
> > > u64 start;
> > > struct {
> > > u8 addr;
> > > diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> > > index 8f0eb56c6fc66a96..ff475a239f4b0db4 100644
> > > --- a/tools/perf/util/disasm.c
> > > +++ b/tools/perf/util/disasm.c
> > > @@ -2284,6 +2284,13 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> > > }
> > > }
> > >
> > > + /* FIXME: LLVM and CAPSTONE should support source code */
> > > + if (options->annotate_src && !options->hide_src_code) {
> > > + err = symbol__disassemble_objdump(symfs_filename, sym, args);
> > > + if (err == 0)
> > > + goto out_remove_tmp;
> > > + }
> > > +
> > > err = -1;
> > > for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
> > > enum perf_disassembler dis = options->disassemblers[i];
> > > --
> > > 2.50.0.727.gbf7dc18ff4-goog
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] perf annotate: Fix source code annotate with objdump
2025-06-25 23:03 [PATCH v3] perf annotate: Fix source code annotate with objdump Namhyung Kim
2025-06-26 15:23 ` Ian Rogers
@ 2025-06-27 18:53 ` Namhyung Kim
1 sibling, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2025-06-27 18:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang, Namhyung Kim
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users
On Wed, 25 Jun 2025 16:03:39 -0700, Namhyung Kim wrote:
> Recently it uses llvm and capstone to speed up annotation or disassembly
> of instructions. But they don't support source code view yet. Until it
> fixed, we can force to use objdump for source code annotation.
>
> To prevent performance loss, it's disabled by default and turned it on
> when user requests it in TUI by pressing 's' key.
>
> [...]
Applied to perf-tools-next, thanks!
Best regards,
Namhyung
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-27 18:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-25 23:03 [PATCH v3] perf annotate: Fix source code annotate with objdump Namhyung Kim
2025-06-26 15:23 ` Ian Rogers
2025-06-26 17:50 ` Namhyung Kim
2025-06-26 21:23 ` Ian Rogers
2025-06-27 18:53 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).