All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Howard Chu <howardchu95@gmail.com>,
	Namhyung Kim <namhyung@kernel.org>,
	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: Wed, 9 Apr 2025 22:38:16 -0300	[thread overview]
Message-ID: <Z_chCGYb1_hMS1F9@x1> (raw)
In-Reply-To: <Z_bl8tabAwfqKuy-@gmail.com>

On Wed, Apr 09, 2025 at 11:26:10PM +0200, Ingo Molnar wrote:
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Using 't' to zoom into a thread and then LEFT to zoom out works, ditto
> > for 'd' + LEFT, 'k' (zoom into the kernel) + LEFT its just 'm' + thread
> > or ENTER + Zoom into Thread that isn't working...

> > Not even that, there is something subtle, investigating.

> > But as a suggestion, pressing 't' zooms into a thread, pressing it again
> > zooms out of that thread, rinse repeat.

> Indeed, this works it around. It's not *that* easy though: 't' goes 
> into thread view, but has to be pressed another 3(?) times to get back 
> to the previous non-threaded view again.

> > Ditto for 'd' for DSO and 'k' for the kernel DSO, 'S' for a CPU socket
> > (Kan Liang implemented this, but not for 'c' CPU, will try to add that).

> > Anyway, back to the bug.
 
> Thanks for having a look! :)

I guess I found it, another one-liner (well, two if you count removing a
comment line) with a long explanation, see below.

There is a tidy up patch before this, all is at:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf-annotate+build

So far:

⬢ [acme@toolbox perf-tools-next]$ git log --oneline v6.15-rc1..
4af32d73e850cb2c (HEAD -> perf-tools-next) perf ui browser hists: Set actions->thread before calling do_zoom_thread()
caab12ee523ccf7c perf ui browser hists: Simplify the routines that add entries to the popup menu
dd2ca9ca0d76a04b perf ui browser: Accept the left arrow key as a Zoom out if done on the first column
6b4e380deb02de46 perf ui browser annotate: Don't show the source code view status initially
befd9928ac3b0914 perf ui browser annotate: Show in the title the source code view toggle
6c557247d907487e perf ui browser map: Provide feedback on unhandled hotkeys
dcee76bd1ef37cd5 perf ui browser hists: Provide feedback on unhandled hotkeys
7cd3b61be22b6111 perf ui browser header: Provide feedback on unhandled hotkeys
4226b944d6cfec62 perf ui browser annotate: Provide feedback on unhandled hotkeys
96f43ea8e198b9cd perf ui browser annotate-data: Provide feedback on unhandled hotkeys
1101a930a674936a perf ui browser: Add a warn on unhandled hotkey helper
bdd6ce8eb8c2f2f7 perf ui browser: Add key_name() helper
65185f2cf7af5026 tools build: Don't show libbfd build status as it is opt-in
33da8804f7c00056 perf check: Add tip about building with libbfd using BUILD_NONDISTRO=1
da4de82814f54c6c perf build: Warn when libdebuginfod devel files are not available
d5f1c3eb842aaafe tools build: Don't show libunwind build status as it is opt-in
6cf8fa4dcce720f9 perf check: Allow showing a tip for opt-in features not built into perf
07f8bc136355d09a perf check: Move the FEATURE_STATUS() macro to its only user source file
a3fb01a0d062d9f4 perf check: Share the feature status printing routine with 'perf version'
c2bc1a34f2488b7f tools build: Don't set libunwind as available if test-all.c build succeeds
⬢ [acme@toolbox perf-tools-next]$

I'm starting to take a look at:

⬢ [acme@toolbox git]$ head -5 ./llvm-project/llvm/tools/llvm-objdump/SourcePrinter.h
//===-- SourcePrinter.h -  source interleaving utilities --------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
⬢ [acme@toolbox git]$

:-)

- Arnaldo

From 4af32d73e850cb2c0c1679a0d30a65d8d5f4222d Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Wed, 9 Apr 2025 21:58:19 -0300
Subject: [PATCH 1/1] perf ui browser hists: Set actions->thread before calling
 do_zoom_thread()

In 7cecb7fe8388d5c3 ("perf hists: Move sort__has_comm into struct
perf_hpp_list") it assumes that act->thread is set prior to calling
do_zoom_thread().

This doesn't happen when we use ESC or the Left arrow key to Zoom out of
a specific thread, making this operation not to work and we get stuck
into the thread zoom.

In 6422184b087ff435 ("perf hists browser: Simplify zooming code using
pstack_peek()") it says no need to set actions->thread, and at that
point that was true, but in 7cecb7fe8388d5c3 a actions->thread == NULL
check was added before the zoom out of thread could kick in.

We can zoom out using the alternative 't' thread zoom toggle hotkey to
finally set actions->thread before calling do_zoom_thread() and zoom
out, but lets also fix the ESC/Zoom out of thread case.

Fixes: 7cecb7fe8388d5c3 ("perf hists: Move sort__has_comm into struct perf_hpp_list")
Reported-by: Ingo Molnar <mingo@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lore.kernel.org/r/Z_TYux5fUg2pW-pF@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 0db2a3f06e23cc5a..cf022e92d06b9b28 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -3264,10 +3264,10 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
 				/*
 				 * No need to set actions->dso here since
 				 * it's just to remove the current filter.
-				 * Ditto for thread below.
 				 */
 				do_zoom_dso(browser, actions);
 			} else if (top == &browser->hists->thread_filter) {
+				actions->thread = thread;
 				do_zoom_thread(browser, actions);
 			} else if (top == &browser->hists->socket_filter) {
 				do_zoom_socket(browser, actions);
-- 
2.48.1


  reply	other threads:[~2025-04-10  1:38 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
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 [this message]
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_chCGYb1_hMS1F9@x1 \
    --to=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=namhyung@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.