linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Howard Chu <howardchu95@gmail.com>,
	Ian Rogers <irogers@google.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-perf-users@vger.kernel.org,
	Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [perf top] annotation doesn't work, libunwind doesn't seem to be working either
Date: Thu, 10 Apr 2025 13:48:28 -0700	[thread overview]
Message-ID: <Z_gunJ9C3H25Uo9Q@google.com> (raw)
In-Reply-To: <Z_XpADpc9WtUdbmX@x1>

On Wed, Apr 09, 2025 at 12:26:56AM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Apr 07, 2025 at 11:16:04PM -0700, Namhyung Kim wrote:
> > On Mon, Apr 07, 2025 at 09:54:47PM -0300, Arnaldo Carvalho de Melo wrote:
> > > A quick test here with perf top without setting the above ends up with
> > > the behaviour that Ingo reported, no source code is shown and 's'
> > > doesn't work. Then if I set it to use objdump:
> 
> > > root@number:~# perf config annotate.disassemblers=objdump
> > > root@number:~# perf config annotate.disassemblers
> > > annotate.disassemblers=objdump
> > > root@number:~# cat ~/.perfconfig 
> > > # this file is auto-generated.
> > > [annotate]
> > > 	disassemblers = objdump
> > > root@number:~#
> 
> > > It takes longer to show the annotated output but there is source code
> > > and 's' works.
> 
> > Thanks for chasing it down, we miss source line support in the disasm
> > using capstone and libllvm.  We can add that later but for now we may
> > refresh the result and force to objdump when source line is requested.
> 
> That is a good idea, I thought about it as well and will check how to do
> it on top of what I have at:
> 
> https://web.git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf-annotate%2bbuild

How about this?


From ce37ae9008ec7c6ab66ee1b6632a3c51bc0802f3 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 10 Apr 2025 13:42:16 -0700
Subject: [PATCH] perf annotate: Fix source code annotate with objdump

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.

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate.c | 43 +++++++++++++++++++++++++++++--
 tools/perf/util/annotate.c        |  2 ++
 tools/perf/util/annotate.h        |  1 +
 tools/perf/util/disasm.c          |  7 +++++
 4 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index ab776b1ed2d5b4ba..fe31069990d77e65 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,8 @@ static struct annotation_line *annotate_browser__find_next_asm_line(
 	return NULL;
 }
 
-static bool annotate_browser__toggle_source(struct annotate_browser *browser)
+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 +395,27 @@ 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;
+
+	if (annotate_opts.hide_src_code && !notes->src->tried_source) {
+		struct map_symbol *ms = browser->b.priv;
+		bool orig_entries = notes->src->nr_entries;
+		int orig_idx_asm = al->idx_asm;
+
+		notes->src->tried_source = true;
+
+		/* 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;
+
+		browser->b.entries = &notes->src->source;
+		al = annotate_browser__find_new_asm_line(browser, orig_idx_asm);
+		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 +872,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);
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.49.0.604.gff1f9ca942-goog



  reply	other threads:[~2025-04-10 20:48 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-07  8:08 [PATCH 1/3] perf sort: Keep output fields in the same level Namhyung Kim
2025-03-07  8:08 ` [PATCH 2/3] perf report: Allow hierarchy mode for --children Namhyung Kim
2025-03-07  8:08 ` [PATCH 3/3] perf report: Disable children column for data type profiling Namhyung Kim
2025-03-20  0:36 ` [PATCH 1/3] perf sort: Keep output fields in the same level Namhyung Kim
2025-03-20  9:32   ` Ingo Molnar
2025-03-20 16:16     ` Namhyung Kim
2025-03-24  7:28       ` Ingo Molnar
2025-03-25  0:26         ` Namhyung Kim
2025-04-04  9:41         ` [perf top] annotation doesn't work, libunwind doesn't seem to be working either Ingo Molnar
2025-04-04 17:28           ` Namhyung Kim
2025-04-04 18:13             ` Arnaldo Carvalho de Melo
2025-04-04 18:25               ` Arnaldo Carvalho de Melo
2025-04-04 18:40                 ` Arnaldo Carvalho de Melo
2025-04-05  9:06             ` Ingo Molnar
2025-04-05  9:09               ` Ingo Molnar
2025-04-07  6:02           ` Howard Chu
2025-04-07 16:58             ` Ingo Molnar
2025-04-07 17:04               ` Ingo Molnar
2025-04-08  0:54               ` Arnaldo Carvalho de Melo
2025-04-08  6:16                 ` Namhyung Kim
2025-04-09  3:26                   ` Arnaldo Carvalho de Melo
2025-04-10 20:48                     ` Namhyung Kim [this message]
2025-04-10 20:54                       ` Ingo Molnar
2025-04-24 12:37                       ` Arnaldo Carvalho de Melo
2025-04-08  8:05                 ` Ingo Molnar
2025-04-09  2:23                   ` Arnaldo Carvalho de Melo
2025-04-09 12:19                     ` Arnaldo Carvalho de Melo
2025-04-09 15:57                       ` Arnaldo Carvalho de Melo
2025-04-09 19:17                         ` Arnaldo Carvalho de Melo
2025-04-09 19:22                           ` Arnaldo Carvalho de Melo
2025-04-09 21:26                             ` Ingo Molnar
2025-04-10  1:38                               ` Arnaldo Carvalho de Melo
2025-04-10  6:24                                 ` Ingo Molnar
2025-04-10 14:03                                   ` Fixes for perf build system and TUI browsers was " Arnaldo Carvalho de Melo
2025-03-25  0:46     ` [PATCH 1/3] perf sort: Keep output fields in the same level Namhyung Kim
2025-03-30  5:54     ` Namhyung Kim
2025-03-21 18:30 ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z_gunJ9C3H25Uo9Q@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=dvyukov@google.com \
    --cc=howardchu95@gmail.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).