[RFC] perf report/top with callchain fixes improvements

[RFC] perf report/top with callchain fixes improvements

[Arnaldo Carvalho de Melo]

Hi,

	These patches try to address inconsistencies in the navigation
in the 'perf report' and 'perf top' when callchains are present.

 	Linus reported that when pressing ENTER with callchains enabled
it no longer shows the popup menu, instead it toggles the callchains,
which is surprising and rather annoying.

	So this patchset makes it consistent by presenting the popup
menu when ENTER is pressed and callchains are present, and then an
extra menu entry, associated with the new '+' toggle is added and
appears proeminently in the popup menu, together with suggestions about
using 'e' to toogle all the callchains for the top level entry,

	Other changes in this patchkit allows for using those suggested
hotkeys directly from the popup menu and add a new 'k' hotkey to zoom
straight into the main kernel map. Pressing it again works just like
using the popup menu via ENTER + "Zoom out of the kernel" or using ESC
to Zoom out from the last filtering operation.

	While working on this I noticed some other oddities like in
the default --children mode where the Annotation popup option appeared
even for entries where no samples were taken, i.e. something that
appeared just on some callchain, suppressing the Annotation popup entry
for those cases. If the user insists in using the 'a' hotkey on such an
entry, a popup warning is showed.

	Also a fix for kernel maps for ranges that seldom appear in
samples, like "[kernel.vmlinux].init.text", that didn't a pointer back
to the kernel maps data structure and thus was causing an assertion
falure were found and fix, please test, available at:

	Available at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/hists_browser

Best regards,

- Arnaldo

Arnaldo Carvalho de Melo (12):
  perf hists browser: Restore ESC as "Zoom out" of DSO/thread/etc
  perf report/top: Make ENTER consistently bring up menu
  perf report/top: Add menu entry for toggling callchain expansion
  perf report/top: Improve toggle callchain menu option
  perf hists browser: Generalize the do_zoom_dso() function
  perf report/top: Add 'k' hotkey to zoom directly into the kernel map
  perf hists browser: Allow passing an initial hotkey
  tools ui popup: Allow returning hotkeys
  perf report/top: Allow pressing hotkeys in the options popup menu
  perf report/top: Do not offer annotation for symbols without samples
  perf report/top: Make 'e' visible in the help and make it toggle showing callchains
  perf maps: Set maps pointer in the kmap area for kernel maps

 tools/perf/builtin-c2c.c            |   4 +-
 tools/perf/tests/maps.c             |   8 +-
 tools/perf/ui/browsers/hists.c      | 141 ++++++++++++++++++++++++----
 tools/perf/ui/browsers/hists.h      |   2 +-
 tools/perf/ui/browsers/res_sample.c |   2 +-
 tools/perf/ui/browsers/scripts.c    |   2 +-
 tools/perf/ui/tui/util.c            |  12 ++-
 tools/perf/ui/util.h                |   2 +-
 tools/perf/util/auxtrace.c          |   2 +-
 tools/perf/util/dso.c               |   4 +-
 tools/perf/util/dso.h               |   4 +-
 tools/perf/util/machine.c           |  10 +-
 tools/perf/util/map.c               |  15 ++-
 tools/perf/util/map.h               |   2 +-
 tools/perf/util/probe-event.c       |  10 +-
 tools/perf/util/sort.c              |   3 +-
 tools/perf/util/sort.h              |   2 +
 tools/perf/util/symbol-elf.c        |   2 +-
 tools/perf/util/symbol.c            |   6 +-
 19 files changed, 178 insertions(+), 55 deletions(-)

-- 
2.21.0

[PATCH 01/12] perf hists browser: Restore ESC as "Zoom out" of DSO/thread/etc

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

We need to set actions->ms.map since 599a2f38a989 ("perf hists browser:
Check sort keys before hot key actions"), as in that patch we bail out
if map is NULL.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Fixes: 599a2f38a989 ("perf hists browser: Check sort keys before hot key actions")
Link: https://lkml.kernel.org/n/tip-wp1ssoewy6zihwwexqpohv0j@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d4d3558fdef4..cfc6172ecab7 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -3062,6 +3062,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 
 				continue;
 			}
+			actions->ms.map = map;
 			top = pstack__peek(browser->pstack);
 			if (top == &browser->hists->dso_filter) {
 				/*
-- 
2.21.0

[PATCH 02/12] perf report/top: Make ENTER consistently bring up menu

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When callchains are present the ENTER key switches from bringing up the
menu that offers Annotation, Zoom by DSO, etc to expanding/collapsing
one callchain level, causing confusion, fix it by making it consistently
bring up the menu and use '+' to expand/collapse one callchain level.

Next patch will also add an entry to the menu to allow
expanding/collapsing, so that people used to ENTER expanding one
callchain level can quickly find it and use it instead.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-bjz35omktig8cwn6lbj1ifns@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index cfc6172ecab7..fefa505d4fa8 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -706,7 +706,7 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
 			browser->show_headers = !browser->show_headers;
 			hist_browser__update_rows(browser);
 			break;
-		case K_ENTER:
+		case '+':
 			if (hist_browser__toggle_fold(browser))
 				break;
 			/* fall thru */
@@ -2858,6 +2858,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 	"For symbolic views (--sort has sym):\n\n"			\
 	"ENTER         Zoom into DSO/Threads & Annotate current symbol\n" \
 	"ESC           Zoom out\n"					\
+	"+             Expand/Collapse one callchain level\n"		\
 	"a             Annotate current symbol\n"			\
 	"C             Collapse all callchains\n"			\
 	"d             Zoom into current DSO\n"				\
-- 
2.21.0

[PATCH 03/12] perf report/top: Add menu entry for toggling callchain expansion

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Since previously pressing ENTER toggled expansion/collapse of callchain
entries and now brings up the same menu used when callchains are not
present, add an entry so that users can quickly figure out the change in
behaviour.

Its worth mentioning that we also always had 'e'/'c' to expand/collapse
all entries in a hist entry and 'E'/'C' for all hist entries.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-f9o03jo29fypvd8ly3j49d36@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fefa505d4fa8..1b5a5990dddb 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2527,6 +2527,26 @@ add_dso_opt(struct hist_browser *browser, struct popup_action *act,
 	return 1;
 }
 
+static int do_toggle_callchain(struct hist_browser *browser, struct popup_action *act __maybe_unused)
+{
+	hist_browser__toggle_fold(browser);
+	return 0;
+}
+
+static int add_callchain_toggle_opt(struct hist_browser *browser, struct popup_action *act, char **optstr)
+{
+	struct hist_entry *he = browser->he_selection;
+
+        if (!he->has_children)
+                return 0;
+
+	if (asprintf(optstr, "Expand/Collapse callchain") < 0)
+		return 0;
+
+	act->fn = do_toggle_callchain;
+	return 1;
+}
+
 static int
 do_browse_map(struct hist_browser *browser __maybe_unused,
 	      struct popup_action *act)
@@ -3137,6 +3157,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 					     &options[nr_options], thread);
 		nr_options += add_dso_opt(browser, &actions[nr_options],
 					  &options[nr_options], map);
+		nr_options += add_callchain_toggle_opt(browser, &actions[nr_options], &options[nr_options]);
 		nr_options += add_map_opt(browser, &actions[nr_options],
 					  &options[nr_options],
 					  browser->selection ?
-- 
2.21.0

[PATCH 04/12] perf report/top: Improve toggle callchain menu option

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Taking into account the current status of the callchain, i.e. if folded,
show "Expand", otherwise "Collapse", also show the name of the entry
that will be affected and mention the hotkeys for expanding/collapsing
all callchains below the main entry, the one that appears with/without
callchains.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-03arm6poo8463k5tfcfp7gkk@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 54 ++++++++++++++++++++++++++++++++--
 tools/perf/util/sort.c         |  3 +-
 tools/perf/util/sort.h         |  2 ++
 3 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 1b5a5990dddb..a4413d983216 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -391,6 +391,52 @@ static void hist_entry__init_have_children(struct hist_entry *he)
 	he->init_have_children = true;
 }
 
+static bool hist_browser__selection_has_children(struct hist_browser *browser)
+{
+	struct hist_entry *he = browser->he_selection;
+	struct map_symbol *ms = browser->selection;
+
+	if (!he || !ms)
+		return false;
+
+	if (ms == &he->ms)
+	       return he->has_children;
+
+	return container_of(ms, struct callchain_list, ms)->has_children;
+}
+
+static bool hist_browser__selection_unfolded(struct hist_browser *browser)
+{
+	struct hist_entry *he = browser->he_selection;
+	struct map_symbol *ms = browser->selection;
+
+	if (!he || !ms)
+		return false;
+
+	if (ms == &he->ms)
+	       return he->unfolded;
+
+	return container_of(ms, struct callchain_list, ms)->unfolded;
+}
+
+static char *hist_browser__selection_sym_name(struct hist_browser *browser, char *bf, size_t size)
+{
+	struct hist_entry *he = browser->he_selection;
+	struct map_symbol *ms = browser->selection;
+	struct callchain_list *callchain_entry;
+
+	if (!he || !ms)
+		return NULL;
+
+	if (ms == &he->ms) {
+	       hist_entry__sym_snprintf(he, bf, size, 0);
+	       return bf + 4; // skip the level, e.g. '[k] '
+	}
+
+	callchain_entry = container_of(ms, struct callchain_list, ms);
+	return callchain_list__sym_name(callchain_entry, bf, size, browser->show_dso);
+}
+
 static bool hist_browser__toggle_fold(struct hist_browser *browser)
 {
 	struct hist_entry *he = browser->he_selection;
@@ -2535,12 +2581,14 @@ static int do_toggle_callchain(struct hist_browser *browser, struct popup_action
 
 static int add_callchain_toggle_opt(struct hist_browser *browser, struct popup_action *act, char **optstr)
 {
-	struct hist_entry *he = browser->he_selection;
+	char sym_name[512];
 
-        if (!he->has_children)
+        if (!hist_browser__selection_has_children(browser))
                 return 0;
 
-	if (asprintf(optstr, "Expand/Collapse callchain") < 0)
+	if (asprintf(optstr, "%s [%s] callchain (one level, same as '+' hotkey, use 'e'/'c' for the whole main level entry)",
+		     hist_browser__selection_unfolded(browser) ? "Collapse" : "Expand",
+		     hist_browser__selection_sym_name(browser, sym_name, sizeof(sym_name))) < 0)
 		return 0;
 
 	act->fn = do_toggle_callchain;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 9fcba2872130..ab0cfd790ad0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -324,8 +324,7 @@ static int _hist_entry__sym_snprintf(struct map_symbol *ms,
 	return ret;
 }
 
-static int hist_entry__sym_snprintf(struct hist_entry *he, char *bf,
-				    size_t size, unsigned int width)
+int hist_entry__sym_snprintf(struct hist_entry *he, char *bf, size_t size, unsigned int width)
 {
 	return _hist_entry__sym_snprintf(&he->ms, he->ip,
 					 he->level, bf, size, width);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 5aff9542d9b7..6c862d62d052 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -164,6 +164,8 @@ static __pure inline bool hist_entry__has_callchains(struct hist_entry *he)
 	return he->callchain_size != 0;
 }
 
+int hist_entry__sym_snprintf(struct hist_entry *he, char *bf, size_t size, unsigned int width);
+
 static inline bool hist_entry__has_pairs(struct hist_entry *he)
 {
 	return !list_empty(&he->pairs.node);
-- 
2.21.0

[PATCH 05/12] perf hists browser: Generalize the do_zoom_dso() function

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

We'll use it to provide a top level hotkey to zoom into the kernel dso
directly.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-ae9cjel6v05wjnz9r6z77b6x@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a4413d983216..8aba1aeea0eb 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2530,11 +2530,8 @@ add_thread_opt(struct hist_browser *browser, struct popup_action *act,
 	return 1;
 }
 
-static int
-do_zoom_dso(struct hist_browser *browser, struct popup_action *act)
+static int hists_browser__zoom_map(struct hist_browser *browser, struct map *map)
 {
-	struct map *map = act->ms.map;
-
 	if (!hists__has(browser->hists, dso) || map == NULL)
 		return 0;
 
@@ -2556,6 +2553,12 @@ do_zoom_dso(struct hist_browser *browser, struct popup_action *act)
 	return 0;
 }
 
+static int
+do_zoom_dso(struct hist_browser *browser, struct popup_action *act)
+{
+	return hists_browser__zoom_map(browser, act->ms.map);
+}
+
 static int
 add_dso_opt(struct hist_browser *browser, struct popup_action *act,
 	    char **optstr, struct map *map)
-- 
2.21.0

[PATCH 06/12] perf report/top: Add 'k' hotkey to zoom directly into the kernel map

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

As a convenience, equivalent to pressing Enter in a line with a kernel
symbol and then selecting "Zoom" into the kernel DSO.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-vbnlnrpyfvz9deqoobtc3dz7@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 8aba1aeea0eb..6dfdd8d5a743 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -18,7 +18,9 @@
 #include "../../util/evlist.h"
 #include "../../util/header.h"
 #include "../../util/hist.h"
+#include "../../util/machine.h"
 #include "../../util/map.h"
+#include "../../util/maps.h"
 #include "../../util/symbol.h"
 #include "../../util/map_symbol.h"
 #include "../../util/branch.h"
@@ -2566,7 +2568,7 @@ add_dso_opt(struct hist_browser *browser, struct popup_action *act,
 	if (!hists__has(browser->hists, dso) || map == NULL)
 		return 0;
 
-	if (asprintf(optstr, "Zoom %s %s DSO",
+	if (asprintf(optstr, "Zoom %s %s DSO (use the 'k' hotkey to zoom directly into the kernel)",
 		     browser->hists->dso_filter ? "out of" : "into",
 		     __map__is_kernel(map) ? "the Kernel" : map->dso->short_name) < 0)
 		return 0;
@@ -2936,6 +2938,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 	"E             Expand all callchains\n"				\
 	"F             Toggle percentage of filtered entries\n"		\
 	"H             Display column headers\n"			\
+	"k             Zoom into the kernel map\n"			\
 	"L             Change percent limit\n"				\
 	"m             Display context menu\n"				\
 	"S             Zoom into current Processor Socket\n"		\
@@ -3033,6 +3036,10 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 			actions->ms.map = map;
 			do_zoom_dso(browser, actions);
 			continue;
+		case 'k':
+			if (browser->selection != NULL)
+				hists_browser__zoom_map(browser, browser->selection->maps->machine->vmlinux_map);
+			continue;
 		case 'V':
 			verbose = (verbose + 1) % 4;
 			browser->show_dso = verbose > 0;
-- 
2.21.0

Re: [PATCH 06/12] perf report/top: Add 'k' hotkey to zoom directly into the kernel map

[Namhyung Kim]

Hi Arnaldo,

On Tue, Dec 17, 2019 at 11:49 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> As a convenience, equivalent to pressing Enter in a line with a kernel
> symbol and then selecting "Zoom" into the kernel DSO.

We already have 'd' key for 'zoom into current dso'.
Do you really want 'k' for kernel specially?

>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Link: https://lkml.kernel.org/n/tip-vbnlnrpyfvz9deqoobtc3dz7@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/ui/browsers/hists.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 8aba1aeea0eb..6dfdd8d5a743 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -18,7 +18,9 @@
>  #include "../../util/evlist.h"
>  #include "../../util/header.h"
>  #include "../../util/hist.h"
> +#include "../../util/machine.h"
>  #include "../../util/map.h"
> +#include "../../util/maps.h"
>  #include "../../util/symbol.h"
>  #include "../../util/map_symbol.h"
>  #include "../../util/branch.h"
> @@ -2566,7 +2568,7 @@ add_dso_opt(struct hist_browser *browser, struct popup_action *act,
>         if (!hists__has(browser->hists, dso) || map == NULL)
>                 return 0;
>
> -       if (asprintf(optstr, "Zoom %s %s DSO",
> +       if (asprintf(optstr, "Zoom %s %s DSO (use the 'k' hotkey to zoom directly into the kernel)",

Wouldn't it be better suggesting 'd' key instead?

Thanks
Namhyung


>                      browser->hists->dso_filter ? "out of" : "into",
>                      __map__is_kernel(map) ? "the Kernel" : map->dso->short_name) < 0)
>                 return 0;
> @@ -2936,6 +2938,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>         "E             Expand all callchains\n"                         \
>         "F             Toggle percentage of filtered entries\n"         \
>         "H             Display column headers\n"                        \
> +       "k             Zoom into the kernel map\n"                      \
>         "L             Change percent limit\n"                          \
>         "m             Display context menu\n"                          \
>         "S             Zoom into current Processor Socket\n"            \
> @@ -3033,6 +3036,10 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>                         actions->ms.map = map;
>                         do_zoom_dso(browser, actions);
>                         continue;
> +               case 'k':
> +                       if (browser->selection != NULL)
> +                               hists_browser__zoom_map(browser, browser->selection->maps->machine->vmlinux_map);
> +                       continue;
>                 case 'V':
>                         verbose = (verbose + 1) % 4;
>                         browser->show_dso = verbose > 0;
> --
> 2.21.0
>

Re: [PATCH 06/12] perf report/top: Add 'k' hotkey to zoom directly into the kernel map

[Arnaldo Carvalho de Melo]

Em Fri, Dec 20, 2019 at 03:48:23PM +0900, Namhyung Kim escreveu:
> On Tue, Dec 17, 2019 at 11:49 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > As a convenience, equivalent to pressing Enter in a line with a kernel
> > symbol and then selecting "Zoom" into the kernel DSO.
 
> We already have 'd' key for 'zoom into current dso'.

Right, current DSO, 'k' is equivalent to:

1. Navigate to a kernel map entry
2. Press 'd'

And also to:

1. Navigate to a kernel map entry
2. Press ENTER
3. Navigate to "Zoom into Kernel DSO"
4. Press ENTER

One key versus 2 or four.

> Do you really want 'k' for kernel specially?

I thought kernel hackers would like the convenience, doing:

  perf top + k

To get the main kernel samples looks faster than:

  perf top -e cycles:k

And those are not even equivalent, as cycles:k will show everything in
ring 0, while 'perf top + k' will show just what is in the kernel _and_
in the main kernel map.

- Arnaldo

Re: [PATCH 06/12] perf report/top: Add 'k' hotkey to zoom directly into the kernel map

[Namhyung Kim]

On Fri, Dec 20, 2019 at 9:17 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Fri, Dec 20, 2019 at 03:48:23PM +0900, Namhyung Kim escreveu:
> > On Tue, Dec 17, 2019 at 11:49 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > As a convenience, equivalent to pressing Enter in a line with a kernel
> > > symbol and then selecting "Zoom" into the kernel DSO.
>
> > We already have 'd' key for 'zoom into current dso'.
>
> Right, current DSO, 'k' is equivalent to:
>
> 1. Navigate to a kernel map entry
> 2. Press 'd'
>
> And also to:
>
> 1. Navigate to a kernel map entry
> 2. Press ENTER
> 3. Navigate to "Zoom into Kernel DSO"
> 4. Press ENTER
>
> One key versus 2 or four.
>
> > Do you really want 'k' for kernel specially?
>
> I thought kernel hackers would like the convenience, doing:
>
>   perf top + k
>
> To get the main kernel samples looks faster than:
>
>   perf top -e cycles:k
>
> And those are not even equivalent, as cycles:k will show everything in
> ring 0, while 'perf top + k' will show just what is in the kernel _and_
> in the main kernel map.

I'm fine with adding 'k' key itself, but I thought a bit strange when
it suggests kernel even with user DSOs.

Thanks
Namhyung

[PATCH 07/12] perf hists browser: Allow passing an initial hotkey

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Sometimes we're in an outer code, like the main hists browser popup menu
and the user follows a suggestion about using some hotkey, and that
hotkey is really handled by hists_browser__run(), so allow for calling
it with that hotkey, making it handle it instead of waiting for the user
to press one.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-i0kwpuicoy4p1tyrw5k054zq@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c       |  4 ++--
 tools/perf/ui/browsers/hists.c | 13 +++++++------
 tools/perf/ui/browsers/hists.h |  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index e69f44941aad..346351260c0b 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2384,7 +2384,7 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)
 	c2c_browser__update_nr_entries(browser);
 
 	while (1) {
-		key = hist_browser__run(browser, "? - help", true);
+		key = hist_browser__run(browser, "? - help", true, 0);
 
 		switch (key) {
 		case 's':
@@ -2453,7 +2453,7 @@ static int perf_c2c__hists_browse(struct hists *hists)
 	c2c_browser__update_nr_entries(browser);
 
 	while (1) {
-		key = hist_browser__run(browser, "? - help", true);
+		key = hist_browser__run(browser, "? - help", true, 0);
 
 		switch (key) {
 		case 'q':
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 6dfdd8d5a743..3b7af5a8d101 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -673,9 +673,8 @@ static int hist_browser__title(struct hist_browser *browser, char *bf, size_t si
 }
 
 int hist_browser__run(struct hist_browser *browser, const char *help,
-		      bool warn_lost_event)
+		      bool warn_lost_event, int key)
 {
-	int key;
 	char title[160];
 	struct hist_browser_timer *hbt = browser->hbt;
 	int delay_secs = hbt ? hbt->refresh : 0;
@@ -688,9 +687,12 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
 	if (ui_browser__show(&browser->b, title, "%s", help) < 0)
 		return -1;
 
+	if (key)
+		goto do_hotkey;
+
 	while (1) {
 		key = ui_browser__run(&browser->b, delay_secs);
-
+do_hotkey:
 		switch (key) {
 		case K_TIMER: {
 			u64 nr_entries;
@@ -2994,8 +2996,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 
 		nr_options = 0;
 
-		key = hist_browser__run(browser, helpline,
-					warn_lost_event);
+		key = hist_browser__run(browser, helpline, warn_lost_event, 0);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -3573,7 +3574,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
 	memset(&action, 0, sizeof(action));
 
 	while (1) {
-		key = hist_browser__run(browser, "? - help", true);
+		key = hist_browser__run(browser, "? - help", true, 0);
 
 		switch (key) {
 		case 'q':
diff --git a/tools/perf/ui/browsers/hists.h b/tools/perf/ui/browsers/hists.h
index 078f2f2c7abd..1e938d9ffa5e 100644
--- a/tools/perf/ui/browsers/hists.h
+++ b/tools/perf/ui/browsers/hists.h
@@ -34,7 +34,7 @@ struct hist_browser {
 struct hist_browser *hist_browser__new(struct hists *hists);
 void hist_browser__delete(struct hist_browser *browser);
 int hist_browser__run(struct hist_browser *browser, const char *help,
-		      bool warn_lost_event);
+		      bool warn_lost_event, int key);
 void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists);
 #endif /* _PERF_UI_BROWSER_HISTS_H_ */
-- 
2.21.0

Re: [PATCH 07/12] perf hists browser: Allow passing an initial hotkey

[Jiri Olsa]

On Tue, Dec 17, 2019 at 11:48:23AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 6dfdd8d5a743..3b7af5a8d101 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -673,9 +673,8 @@ static int hist_browser__title(struct hist_browser *browser, char *bf, size_t si
>  }
>  
>  int hist_browser__run(struct hist_browser *browser, const char *help,
> -		      bool warn_lost_event)
> +		      bool warn_lost_event, int key)
>  {
> -	int key;
>  	char title[160];
>  	struct hist_browser_timer *hbt = browser->hbt;
>  	int delay_secs = hbt ? hbt->refresh : 0;
> @@ -688,9 +687,12 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
>  	if (ui_browser__show(&browser->b, title, "%s", help) < 0)
>  		return -1;
>  
> +	if (key)
> +		goto do_hotkey;
> +
>  	while (1) {
>  		key = ui_browser__run(&browser->b, delay_secs);
> -
> +do_hotkey:

or we could switch the 'swtich' and ui_browser__run, and get rid of the goto, like:

	while (1) {
  		switch (key) {
		...
		}

		key = ui_browser__run(&browser->b, delay_secs);
	}

jirka

>  		switch (key) {
>  		case K_TIMER: {
>  			u64 nr_entries;
> @@ -2994,8 +2996,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>  
>  		nr_options = 0;
>  
> -		key = hist_browser__run(browser, helpline,
> -					warn_lost_event);
> +		key = hist_browser__run(browser, helpline, warn_lost_event, 0);
>  
>  		if (browser->he_selection != NULL) {
>  			thread = hist_browser__selected_thread(browser);
> @@ -3573,7 +3574,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>  	memset(&action, 0, sizeof(action));
>  
>  	while (1) {
> -		key = hist_browser__run(browser, "? - help", true);
> +		key = hist_browser__run(browser, "? - help", true, 0);
>  
>  		switch (key) {
>  		case 'q':
> diff --git a/tools/perf/ui/browsers/hists.h b/tools/perf/ui/browsers/hists.h
> index 078f2f2c7abd..1e938d9ffa5e 100644
> --- a/tools/perf/ui/browsers/hists.h
> +++ b/tools/perf/ui/browsers/hists.h
> @@ -34,7 +34,7 @@ struct hist_browser {
>  struct hist_browser *hist_browser__new(struct hists *hists);
>  void hist_browser__delete(struct hist_browser *browser);
>  int hist_browser__run(struct hist_browser *browser, const char *help,
> -		      bool warn_lost_event);
> +		      bool warn_lost_event, int key);
>  void hist_browser__init(struct hist_browser *browser,
>  			struct hists *hists);
>  #endif /* _PERF_UI_BROWSER_HISTS_H_ */
> -- 
> 2.21.0
> 

Re: [PATCH 07/12] perf hists browser: Allow passing an initial hotkey

[Arnaldo Carvalho de Melo]

Em Wed, Dec 18, 2019 at 09:08:18AM +0100, Jiri Olsa escreveu:
> On Tue, Dec 17, 2019 at 11:48:23AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index 6dfdd8d5a743..3b7af5a8d101 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -673,9 +673,8 @@ static int hist_browser__title(struct hist_browser *browser, char *bf, size_t si
> >  }
> >  
> >  int hist_browser__run(struct hist_browser *browser, const char *help,
> > -		      bool warn_lost_event)
> > +		      bool warn_lost_event, int key)
> >  {
> > -	int key;
> >  	char title[160];
> >  	struct hist_browser_timer *hbt = browser->hbt;
> >  	int delay_secs = hbt ? hbt->refresh : 0;
> > @@ -688,9 +687,12 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
> >  	if (ui_browser__show(&browser->b, title, "%s", help) < 0)
> >  		return -1;
> >  
> > +	if (key)
> > +		goto do_hotkey;
> > +
> >  	while (1) {
> >  		key = ui_browser__run(&browser->b, delay_secs);
> > -
> > +do_hotkey:
> 
> or we could switch the 'swtich' and ui_browser__run, and get rid of the goto, like:
> 
> 	while (1) {
>   		switch (key) {
> 		...
> 		}
> 
> 		key = ui_browser__run(&browser->b, delay_secs);
> 	}

I think those are equivalent and having the test like I did is more
clear, i.e. "has this key been provided" instead of going to the switch
just to hit the default case for the zero in key and call
ui_browser__run().

- Arnaldo
 
> jirka
> 
> >  		switch (key) {
> >  		case K_TIMER: {
> >  			u64 nr_entries;
> > @@ -2994,8 +2996,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
> >  
> >  		nr_options = 0;
> >  
> > -		key = hist_browser__run(browser, helpline,
> > -					warn_lost_event);
> > +		key = hist_browser__run(browser, helpline, warn_lost_event, 0);
> >  
> >  		if (browser->he_selection != NULL) {
> >  			thread = hist_browser__selected_thread(browser);
> > @@ -3573,7 +3574,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
> >  	memset(&action, 0, sizeof(action));
> >  
> >  	while (1) {
> > -		key = hist_browser__run(browser, "? - help", true);
> > +		key = hist_browser__run(browser, "? - help", true, 0);
> >  
> >  		switch (key) {
> >  		case 'q':
> > diff --git a/tools/perf/ui/browsers/hists.h b/tools/perf/ui/browsers/hists.h
> > index 078f2f2c7abd..1e938d9ffa5e 100644
> > --- a/tools/perf/ui/browsers/hists.h
> > +++ b/tools/perf/ui/browsers/hists.h
> > @@ -34,7 +34,7 @@ struct hist_browser {
> >  struct hist_browser *hist_browser__new(struct hists *hists);
> >  void hist_browser__delete(struct hist_browser *browser);
> >  int hist_browser__run(struct hist_browser *browser, const char *help,
> > -		      bool warn_lost_event);
> > +		      bool warn_lost_event, int key);
> >  void hist_browser__init(struct hist_browser *browser,
> >  			struct hists *hists);
> >  #endif /* _PERF_UI_BROWSER_HISTS_H_ */
> > -- 
> > 2.21.0
> > 

-- 

- Arnaldo

Re: [PATCH 07/12] perf hists browser: Allow passing an initial hotkey

[Jiri Olsa]

On Wed, Dec 18, 2019 at 11:08:31AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 18, 2019 at 09:08:18AM +0100, Jiri Olsa escreveu:
> > On Tue, Dec 17, 2019 at 11:48:23AM -0300, Arnaldo Carvalho de Melo wrote:
> > 
> > SNIP
> > 
> > > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > > index 6dfdd8d5a743..3b7af5a8d101 100644
> > > --- a/tools/perf/ui/browsers/hists.c
> > > +++ b/tools/perf/ui/browsers/hists.c
> > > @@ -673,9 +673,8 @@ static int hist_browser__title(struct hist_browser *browser, char *bf, size_t si
> > >  }
> > >  
> > >  int hist_browser__run(struct hist_browser *browser, const char *help,
> > > -		      bool warn_lost_event)
> > > +		      bool warn_lost_event, int key)
> > >  {
> > > -	int key;
> > >  	char title[160];
> > >  	struct hist_browser_timer *hbt = browser->hbt;
> > >  	int delay_secs = hbt ? hbt->refresh : 0;
> > > @@ -688,9 +687,12 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
> > >  	if (ui_browser__show(&browser->b, title, "%s", help) < 0)
> > >  		return -1;
> > >  
> > > +	if (key)
> > > +		goto do_hotkey;
> > > +
> > >  	while (1) {
> > >  		key = ui_browser__run(&browser->b, delay_secs);
> > > -
> > > +do_hotkey:
> > 
> > or we could switch the 'swtich' and ui_browser__run, and get rid of the goto, like:
> > 
> > 	while (1) {
> >   		switch (key) {
> > 		...
> > 		}
> > 
> > 		key = ui_browser__run(&browser->b, delay_secs);
> > 	}
> 
> I think those are equivalent and having the test like I did is more
> clear, i.e. "has this key been provided" instead of going to the switch
> just to hit the default case for the zero in key and call
> ui_browser__run().

sure, I just don't like goto other than for error handling,
looks too hacky to me ;-) but of course it's your call

jirka

Re: [PATCH 07/12] perf hists browser: Allow passing an initial hotkey

[Arnaldo Carvalho de Melo]

Em Wed, Dec 18, 2019 at 03:23:21PM +0100, Jiri Olsa escreveu:
> On Wed, Dec 18, 2019 at 11:08:31AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Dec 18, 2019 at 09:08:18AM +0100, Jiri Olsa escreveu:
> > > On Tue, Dec 17, 2019 at 11:48:23AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > +	if (key)
> > > > +		goto do_hotkey;
> > > > +
> > > >  	while (1) {
> > > >  		key = ui_browser__run(&browser->b, delay_secs);
> > > > -
> > > > +do_hotkey:

> > > or we could switch the 'swtich' and ui_browser__run, and get rid of the goto, like:

> > > 	while (1) {
> > >   		switch (key) {
> > > 		...
> > > 		}
> > > 
> > > 		key = ui_browser__run(&browser->b, delay_secs);
> > > 	}

> > I think those are equivalent and having the test like I did is more
> > clear, i.e. "has this key been provided" instead of going to the switch
> > just to hit the default case for the zero in key and call
> > ui_browser__run().

> sure, I just don't like goto other than for error handling,
> looks too hacky to me ;-) but of course it's your call

How about the one below?

- Arnaldo

commit 9290297c911ad6c315c5f46f35afa3ce517a4c9f
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Thu Dec 12 15:31:40 2019 -0300

    perf hists browser: Allow passing an initial hotkey
    
    Sometimes we're in an outer code, like the main hists browser popup menu
    and the user follows a suggestion about using some hotkey, and that
    hotkey is really handled by hists_browser__run(), so allow for calling
    it with that hotkey, making it handle it instead of waiting for the user
    to press one.
    
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Andi Kleen <ak@linux.intel.com>
    Cc: Jin Yao <yao.jin@linux.intel.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Kan Liang <kan.liang@intel.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Link: https://lkml.kernel.org/n/tip-xv2l7i6o4urn37nv1h40ryfs@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index e69f44941aad..346351260c0b 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2384,7 +2384,7 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)
 	c2c_browser__update_nr_entries(browser);
 
 	while (1) {
-		key = hist_browser__run(browser, "? - help", true);
+		key = hist_browser__run(browser, "? - help", true, 0);
 
 		switch (key) {
 		case 's':
@@ -2453,7 +2453,7 @@ static int perf_c2c__hists_browse(struct hists *hists)
 	c2c_browser__update_nr_entries(browser);
 
 	while (1) {
-		key = hist_browser__run(browser, "? - help", true);
+		key = hist_browser__run(browser, "? - help", true, 0);
 
 		switch (key) {
 		case 'q':
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 6dfdd8d5a743..ac118aef5ed1 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -672,10 +672,81 @@ static int hist_browser__title(struct hist_browser *browser, char *bf, size_t si
 	return browser->title ? browser->title(browser, bf, size) : 0;
 }
 
+static int hist_browser__handle_hotkey(struct hist_browser *browser, bool warn_lost_event, char *title, int key)
+{
+	switch (key) {
+	case K_TIMER: {
+		struct hist_browser_timer *hbt = browser->hbt;
+		u64 nr_entries;
+
+		WARN_ON_ONCE(!hbt);
+
+		if (hbt)
+			hbt->timer(hbt->arg);
+
+		if (hist_browser__has_filter(browser) || symbol_conf.report_hierarchy)
+			hist_browser__update_nr_entries(browser);
+
+		nr_entries = hist_browser__nr_entries(browser);
+		ui_browser__update_nr_entries(&browser->b, nr_entries);
+
+		if (warn_lost_event &&
+		    (browser->hists->stats.nr_lost_warned !=
+		    browser->hists->stats.nr_events[PERF_RECORD_LOST])) {
+			browser->hists->stats.nr_lost_warned =
+				browser->hists->stats.nr_events[PERF_RECORD_LOST];
+			ui_browser__warn_lost_events(&browser->b);
+		}
+
+		hist_browser__title(browser, title, sizeof(title));
+		ui_browser__show_title(&browser->b, title);
+		break;
+	}
+	case 'D': { /* Debug */
+		struct hist_entry *h = rb_entry(browser->b.top, struct hist_entry, rb_node);
+		static int seq;
+
+		ui_helpline__pop();
+		ui_helpline__fpush("%d: nr_ent=(%d,%d), etl: %d, rows=%d, idx=%d, fve: idx=%d, row_off=%d, nrows=%d",
+				   seq++, browser->b.nr_entries, browser->hists->nr_entries,
+				   browser->b.extra_title_lines, browser->b.rows,
+				   browser->b.index, browser->b.top_idx, h->row_offset, h->nr_rows);
+	}
+		break;
+	case 'C':
+		/* Collapse the whole world. */
+		hist_browser__set_folding(browser, false);
+		break;
+	case 'c':
+		/* Collapse the selected entry. */
+		hist_browser__set_folding_selected(browser, false);
+		break;
+	case 'E':
+		/* Expand the whole world. */
+		hist_browser__set_folding(browser, true);
+		break;
+	case 'e':
+		/* Expand the selected entry. */
+		hist_browser__set_folding_selected(browser, true);
+		break;
+	case 'H':
+		browser->show_headers = !browser->show_headers;
+		hist_browser__update_rows(browser);
+		break;
+	case '+':
+		if (hist_browser__toggle_fold(browser))
+			break;
+		/* fall thru */
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
 int hist_browser__run(struct hist_browser *browser, const char *help,
-		      bool warn_lost_event)
+		      bool warn_lost_event, int key)
 {
-	int key;
 	char title[160];
 	struct hist_browser_timer *hbt = browser->hbt;
 	int delay_secs = hbt ? hbt->refresh : 0;
@@ -688,79 +759,14 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
 	if (ui_browser__show(&browser->b, title, "%s", help) < 0)
 		return -1;
 
+	if (key && hist_browser__handle_hotkey(browser, warn_lost_event, title, key))
+		goto out;
+
 	while (1) {
 		key = ui_browser__run(&browser->b, delay_secs);
 
-		switch (key) {
-		case K_TIMER: {
-			u64 nr_entries;
-
-			WARN_ON_ONCE(!hbt);
-
-			if (hbt)
-				hbt->timer(hbt->arg);
-
-			if (hist_browser__has_filter(browser) ||
-			    symbol_conf.report_hierarchy)
-				hist_browser__update_nr_entries(browser);
-
-			nr_entries = hist_browser__nr_entries(browser);
-			ui_browser__update_nr_entries(&browser->b, nr_entries);
-
-			if (warn_lost_event &&
-			    (browser->hists->stats.nr_lost_warned !=
-			    browser->hists->stats.nr_events[PERF_RECORD_LOST])) {
-				browser->hists->stats.nr_lost_warned =
-					browser->hists->stats.nr_events[PERF_RECORD_LOST];
-				ui_browser__warn_lost_events(&browser->b);
-			}
-
-			hist_browser__title(browser, title, sizeof(title));
-			ui_browser__show_title(&browser->b, title);
-			continue;
-		}
-		case 'D': { /* Debug */
-			static int seq;
-			struct hist_entry *h = rb_entry(browser->b.top,
-							struct hist_entry, rb_node);
-			ui_helpline__pop();
-			ui_helpline__fpush("%d: nr_ent=(%d,%d), etl: %d, rows=%d, idx=%d, fve: idx=%d, row_off=%d, nrows=%d",
-					   seq++, browser->b.nr_entries,
-					   browser->hists->nr_entries,
-					   browser->b.extra_title_lines,
-					   browser->b.rows,
-					   browser->b.index,
-					   browser->b.top_idx,
-					   h->row_offset, h->nr_rows);
-		}
-			break;
-		case 'C':
-			/* Collapse the whole world. */
-			hist_browser__set_folding(browser, false);
-			break;
-		case 'c':
-			/* Collapse the selected entry. */
-			hist_browser__set_folding_selected(browser, false);
-			break;
-		case 'E':
-			/* Expand the whole world. */
-			hist_browser__set_folding(browser, true);
+		if (hist_browser__handle_hotkey(browser, warn_lost_event, title, key))
 			break;
-		case 'e':
-			/* Expand the selected entry. */
-			hist_browser__set_folding_selected(browser, true);
-			break;
-		case 'H':
-			browser->show_headers = !browser->show_headers;
-			hist_browser__update_rows(browser);
-			break;
-		case '+':
-			if (hist_browser__toggle_fold(browser))
-				break;
-			/* fall thru */
-		default:
-			goto out;
-		}
 	}
 out:
 	ui_browser__hide(&browser->b);
@@ -2994,8 +3000,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 
 		nr_options = 0;
 
-		key = hist_browser__run(browser, helpline,
-					warn_lost_event);
+		key = hist_browser__run(browser, helpline, warn_lost_event, 0);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -3573,7 +3578,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
 	memset(&action, 0, sizeof(action));
 
 	while (1) {
-		key = hist_browser__run(browser, "? - help", true);
+		key = hist_browser__run(browser, "? - help", true, 0);
 
 		switch (key) {
 		case 'q':
diff --git a/tools/perf/ui/browsers/hists.h b/tools/perf/ui/browsers/hists.h
index 078f2f2c7abd..1e938d9ffa5e 100644
--- a/tools/perf/ui/browsers/hists.h
+++ b/tools/perf/ui/browsers/hists.h
@@ -34,7 +34,7 @@ struct hist_browser {
 struct hist_browser *hist_browser__new(struct hists *hists);
 void hist_browser__delete(struct hist_browser *browser);
 int hist_browser__run(struct hist_browser *browser, const char *help,
-		      bool warn_lost_event);
+		      bool warn_lost_event, int key);
 void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists);
 #endif /* _PERF_UI_BROWSER_HISTS_H_ */

Re: [PATCH 07/12] perf hists browser: Allow passing an initial hotkey

[Jiri Olsa]

On Thu, Dec 19, 2019 at 02:26:42PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 18, 2019 at 03:23:21PM +0100, Jiri Olsa escreveu:
> > On Wed, Dec 18, 2019 at 11:08:31AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Wed, Dec 18, 2019 at 09:08:18AM +0100, Jiri Olsa escreveu:
> > > > On Tue, Dec 17, 2019 at 11:48:23AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > +	if (key)
> > > > > +		goto do_hotkey;
> > > > > +
> > > > >  	while (1) {
> > > > >  		key = ui_browser__run(&browser->b, delay_secs);
> > > > > -
> > > > > +do_hotkey:
> 
> > > > or we could switch the 'swtich' and ui_browser__run, and get rid of the goto, like:
> 
> > > > 	while (1) {
> > > >   		switch (key) {
> > > > 		...
> > > > 		}
> > > > 
> > > > 		key = ui_browser__run(&browser->b, delay_secs);
> > > > 	}
> 
> > > I think those are equivalent and having the test like I did is more
> > > clear, i.e. "has this key been provided" instead of going to the switch
> > > just to hit the default case for the zero in key and call
> > > ui_browser__run().
> 
> > sure, I just don't like goto other than for error handling,
> > looks too hacky to me ;-) but of course it's your call
> 
> How about the one below?

looks good, thanks

jirka

[PATCH 08/12] tools ui popup: Allow returning hotkeys

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

With this patch if an optional pointer is passed to ui__popup_menu()
then when any key that is not being handled (ENTER, ESC, etc) is typed,
it'll record that key in the pointer and return, allowing for hotkey
processing on the caller.

If NULL is passed, no change in logic, unhandled keys continue to be
ignored.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-6ojn19mqzgmrdm8kdoigic0m@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c      |  4 ++--
 tools/perf/ui/browsers/res_sample.c |  2 +-
 tools/perf/ui/browsers/scripts.c    |  2 +-
 tools/perf/ui/tui/util.c            | 12 ++++++++----
 tools/perf/ui/util.h                |  2 +-
 5 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 3b7af5a8d101..da7de49b3553 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2389,7 +2389,7 @@ static int switch_data_file(void)
 	closedir(pwd_dir);
 
 	if (nr_options) {
-		choice = ui__popup_menu(nr_options, options);
+		choice = ui__popup_menu(nr_options, options, NULL);
 		if (choice < nr_options && choice >= 0) {
 			tmp = strdup(abs_path[choice]);
 			if (tmp) {
@@ -3275,7 +3275,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 		do {
 			struct popup_action *act;
 
-			choice = ui__popup_menu(nr_options, options);
+			choice = ui__popup_menu(nr_options, options, NULL);
 			if (choice == -1 || choice >= nr_options)
 				break;
 
diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
index 76d356a18790..7cb2d6678039 100644
--- a/tools/perf/ui/browsers/res_sample.c
+++ b/tools/perf/ui/browsers/res_sample.c
@@ -56,7 +56,7 @@ int res_sample_browse(struct res_sample *res_samples, int num_res,
 			return -1;
 		}
 	}
-	choice = ui__popup_menu(num_res, names);
+	choice = ui__popup_menu(num_res, names, NULL);
 	for (i = 0; i < num_res; i++)
 		zfree(&names[i]);
 	free(names);
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index fc733a6354d4..47d2c7a8cbe1 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -126,7 +126,7 @@ static int list_scripts(char *script_name, bool *custom,
 			SCRIPT_FULLPATH_LEN);
 	if (num < 0)
 		num = 0;
-	choice = ui__popup_menu(num + max_std, (char * const *)names);
+	choice = ui__popup_menu(num + max_std, (char * const *)names, NULL);
 	if (choice < 0) {
 		ret = -1;
 		goto out;
diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
index b98dd0e31dc1..0f562e2cb1e8 100644
--- a/tools/perf/ui/tui/util.c
+++ b/tools/perf/ui/tui/util.c
@@ -23,7 +23,7 @@ static void ui_browser__argv_write(struct ui_browser *browser,
 	ui_browser__write_nstring(browser, *arg, browser->width);
 }
 
-static int popup_menu__run(struct ui_browser *menu)
+static int popup_menu__run(struct ui_browser *menu, int *keyp)
 {
 	int key;
 
@@ -45,6 +45,11 @@ static int popup_menu__run(struct ui_browser *menu)
 			key = -1;
 			break;
 		default:
+			if (keyp) {
+				*keyp = key;
+				key = menu->nr_entries;
+				break;
+			}
 			continue;
 		}
 
@@ -55,7 +60,7 @@ static int popup_menu__run(struct ui_browser *menu)
 	return key;
 }
 
-int ui__popup_menu(int argc, char * const argv[])
+int ui__popup_menu(int argc, char * const argv[], int *keyp)
 {
 	struct ui_browser menu = {
 		.entries    = (void *)argv,
@@ -64,8 +69,7 @@ int ui__popup_menu(int argc, char * const argv[])
 		.write	    = ui_browser__argv_write,
 		.nr_entries = argc,
 	};
-
-	return popup_menu__run(&menu);
+	return popup_menu__run(&menu, keyp);
 }
 
 int ui_browser__input_window(const char *title, const char *text, char *input,
diff --git a/tools/perf/ui/util.h b/tools/perf/ui/util.h
index 40891942f465..e30cea807564 100644
--- a/tools/perf/ui/util.h
+++ b/tools/perf/ui/util.h
@@ -5,7 +5,7 @@
 #include <stdarg.h>
 
 int ui__getch(int delay_secs);
-int ui__popup_menu(int argc, char * const argv[]);
+int ui__popup_menu(int argc, char * const argv[], int *keyp);
 int ui__help_window(const char *text);
 int ui__dialog_yesno(const char *msg);
 void __ui__info_window(const char *title, const char *text, const char *exit_msg);
-- 
2.21.0

[PATCH 09/12] perf report/top: Allow pressing hotkeys in the options popup menu

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When the users presses ENTER in the main 'perf report/top' screen a
popup menu is presented, in it some hotkeys are suggested as
alternatives to using the menu, or for additional features.

At that point the user may try those hotkeys, so allow for that by
recording the key used and exiting, the caller then can check for that
possibility and process the hotkey.

I.e. try pressing ENTER, and then 'k' to exit and zoom into the kernel
map, using ESC then zooms out, etc.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-ujfq3fw44kf6qrtfajl5dcsp@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index da7de49b3553..7f653b9f5cd8 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2991,12 +2991,13 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 	while (1) {
 		struct thread *thread = NULL;
 		struct map *map = NULL;
-		int choice = 0;
+		int choice;
 		int socked_id = -1;
 
-		nr_options = 0;
-
-		key = hist_browser__run(browser, helpline, warn_lost_event, 0);
+		key = 0; // reset key
+do_hotkey:		 // key came straight from options ui__popup_menu()
+		choice = nr_options = 0;
+		key = hist_browser__run(browser, helpline, warn_lost_event, key);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -3275,10 +3276,13 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 		do {
 			struct popup_action *act;
 
-			choice = ui__popup_menu(nr_options, options, NULL);
-			if (choice == -1 || choice >= nr_options)
+			choice = ui__popup_menu(nr_options, options, &key);
+			if (choice == -1)
 				break;
 
+			if (choice == nr_options)
+				goto do_hotkey;
+
 			act = &actions[choice];
 			key = act->fn(browser, act);
 		} while (key == 1);
-- 
2.21.0

[PATCH 10/12] perf report/top: Do not offer annotation for symbols without samples

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

This can happen in the --children mode, i.e. the default mode when
callchains are present, where one of the main entries may be a callchain
entry with no samples.

So far we were not providing any information about why an annotation
couldn't be provided even offering the Annotation option in the popup
menu.

Work is needed to allow for no-samples "annotation', i.e. to show the
disassembly anyway and allow for navigation, etc.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-0hhzj2de15o88cguy7h66zre@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 7f653b9f5cd8..3f10e1a070c5 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2461,7 +2461,8 @@ add_annotate_opt(struct hist_browser *browser __maybe_unused,
 		 struct popup_action *act, char **optstr,
 		 struct map_symbol *ms)
 {
-	if (ms->sym == NULL || ms->map->dso->annotate_warned)
+	if (ms->sym == NULL || ms->map->dso->annotate_warned ||
+	    symbol__annotation(ms->sym)->src == NULL)
 		return 0;
 
 	if (asprintf(optstr, "Annotate %s", ms->sym->name) < 0)
@@ -3027,6 +3028,14 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 			    browser->selection->map->dso->annotate_warned)
 				continue;
 
+			if (symbol__annotation(browser->selection->sym)->src == NULL) {
+				ui_browser__warning(&browser->b, delay_secs * 2,
+						    "No samples for the \"%s\" symbol.\n\n"
+						    "Probably appeared just in a callchain",
+						    browser->selection->sym->name);
+				continue;
+			}
+
 			actions->ms.map = browser->selection->map;
 			actions->ms.sym = browser->selection->sym;
 			do_annotate(browser, actions);
-- 
2.21.0

[PATCH 11/12] perf report/top: Make 'e' visible in the help and make it toggle showing callchains

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The 'e' and 'c' hotkeys were present for a long time, but not documented
in the help window, change 'e' to be a toggle so that it gets consistent
with other toggles like '+' and document it in the help window.

Keep 'c' as is for people used to it but don't document, as it is easier
to just use 'e' to show/hide all the callchains for a top level
histogram entry.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-84s2zp381n4vquc1byexania@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 3f10e1a070c5..fa3a16c49398 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -407,6 +407,11 @@ static bool hist_browser__selection_has_children(struct hist_browser *browser)
 	return container_of(ms, struct callchain_list, ms)->has_children;
 }
 
+static bool hist_browser__he_selection_unfolded(struct hist_browser *browser)
+{
+	return browser->he_selection ? browser->he_selection->unfolded : false;
+}
+
 static bool hist_browser__selection_unfolded(struct hist_browser *browser)
 {
 	struct hist_entry *he = browser->he_selection;
@@ -750,7 +755,7 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
 			break;
 		case 'e':
 			/* Expand the selected entry. */
-			hist_browser__set_folding_selected(browser, true);
+			hist_browser__set_folding_selected(browser, !hist_browser__he_selection_unfolded(browser));
 			break;
 		case 'H':
 			browser->show_headers = !browser->show_headers;
@@ -2938,6 +2943,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 	"a             Annotate current symbol\n"			\
 	"C             Collapse all callchains\n"			\
 	"d             Zoom into current DSO\n"				\
+	"e             Expand/Collapse main entry callchains\n"	\
 	"E             Expand all callchains\n"				\
 	"F             Toggle percentage of filtered entries\n"		\
 	"H             Display column headers\n"			\
-- 
2.21.0

[PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When kernel maps are created with map__new2() we allocate an extra area
to store a pointer to the 'struct maps' for the kernel maps, and this
ends up being used in places like __map__is_kernel() to figure out if a
map is the main kernel one.

We were setting this up only in __machine__create_kernel_maps() and
machine__create_extra_kernel_map(), but not when splitting kallsyms,
kcore address ranges in new kernel maps such as
"[kernel.vmlinux].init.text", leading to assertion failures in
__map__is_kernel().

So make map__new2() receive the kernel 'struct maps' pointer so that all
kernel maps point back to it in its 'struct kmap' area.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-z4qrek7y7s7sjnczremjdn1z@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/maps.c       |  8 ++++----
 tools/perf/util/auxtrace.c    |  2 +-
 tools/perf/util/dso.c         |  4 ++--
 tools/perf/util/dso.h         |  4 +++-
 tools/perf/util/machine.c     | 10 ++++------
 tools/perf/util/map.c         | 15 ++++++++++++++-
 tools/perf/util/map.h         |  2 +-
 tools/perf/util/probe-event.c | 10 ++++++----
 tools/perf/util/symbol-elf.c  |  2 +-
 tools/perf/util/symbol.c      |  6 ++++--
 10 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
index edcbc70ff9d6..7e32b4df8ff1 100644
--- a/tools/perf/tests/maps.c
+++ b/tools/perf/tests/maps.c
@@ -69,7 +69,7 @@ int test__maps__merge_in(struct test *t __maybe_unused, int subtest __maybe_unus
 	for (i = 0; i < ARRAY_SIZE(bpf_progs); i++) {
 		struct map *map;
 
-		map = dso__new_map(bpf_progs[i].name);
+		map = dso__new_map(bpf_progs[i].name, &maps);
 		TEST_ASSERT_VAL("failed to create map", map);
 
 		map->start = bpf_progs[i].start;
@@ -78,13 +78,13 @@ int test__maps__merge_in(struct test *t __maybe_unused, int subtest __maybe_unus
 		map__put(map);
 	}
 
-	map_kcore1 = dso__new_map("kcore1");
+	map_kcore1 = dso__new_map("kcore1", &maps);
 	TEST_ASSERT_VAL("failed to create map", map_kcore1);
 
-	map_kcore2 = dso__new_map("kcore2");
+	map_kcore2 = dso__new_map("kcore2", &maps);
 	TEST_ASSERT_VAL("failed to create map", map_kcore2);
 
-	map_kcore3 = dso__new_map("kcore3");
+	map_kcore3 = dso__new_map("kcore3", &maps);
 	TEST_ASSERT_VAL("failed to create map", map_kcore3);
 
 	/* kcore1 map overlaps over all bpf maps */
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index eb087e7df6f4..c5be6eeff069 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -2250,7 +2250,7 @@ static struct dso *load_dso(const char *name)
 	struct map *map;
 	struct dso *dso;
 
-	map = dso__new_map(name);
+	map = dso__new_map(name, NULL);
 	if (!map)
 		return NULL;
 
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 91f21239608b..dd5c2e71d8d7 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1118,13 +1118,13 @@ ssize_t dso__data_write_cache_addr(struct dso *dso, struct map *map,
 	return dso__data_write_cache_offs(dso, machine, offset, data, size);
 }
 
-struct map *dso__new_map(const char *name)
+struct map *dso__new_map(const char *name, struct maps *kmaps)
 {
 	struct map *map = NULL;
 	struct dso *dso = dso__new(name);
 
 	if (dso)
-		map = map__new2(0, dso);
+		map = map__new2(0, dso, kmaps);
 
 	return map;
 }
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 2db64b79617a..3b4f690c67e0 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -353,7 +353,9 @@ ssize_t dso__data_write_cache_addr(struct dso *dso, struct map *map,
 				   struct machine *machine, u64 addr,
 				   const u8 *data, ssize_t size);
 
-struct map *dso__new_map(const char *name);
+struct maps;
+
+struct map *dso__new_map(const char *name, struct maps *kmaps);
 struct dso *machine__findnew_kernel(struct machine *machine, const char *name,
 				    const char *short_name, int dso_type);
 
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c8c5410315e8..7ee14963edc9 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -726,7 +726,7 @@ static int machine__process_ksymbol_register(struct machine *machine,
 	struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
 
 	if (!map) {
-		map = dso__new_map(event->ksymbol.name);
+		map = dso__new_map(event->ksymbol.name, &machine->kmaps);
 		if (!map)
 			return -ENOMEM;
 
@@ -784,7 +784,7 @@ static struct map *machine__addnew_module_map(struct machine *machine, u64 start
 	if (dso == NULL)
 		goto out;
 
-	map = map__new2(start, dso);
+	map = map__new2(start, dso, &machine->kmaps);
 	if (map == NULL)
 		goto out;
 
@@ -963,7 +963,7 @@ int machine__create_extra_kernel_map(struct machine *machine,
 	struct kmap *kmap;
 	struct map *map;
 
-	map = map__new2(xm->start, kernel);
+	map = map__new2(xm->start, kernel, &machine->kmaps);
 	if (!map)
 		return -1;
 
@@ -972,7 +972,6 @@ int machine__create_extra_kernel_map(struct machine *machine,
 
 	kmap = map__kmap(map);
 
-	kmap->kmaps = &machine->kmaps;
 	strlcpy(kmap->name, xm->name, KMAP_NAME_LEN);
 
 	maps__insert(&machine->kmaps, map);
@@ -1088,7 +1087,7 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
 	/* In case of renewal the kernel map, destroy previous one */
 	machine__destroy_kernel_maps(machine);
 
-	machine->vmlinux_map = map__new2(0, kernel);
+	machine->vmlinux_map = map__new2(0, kernel, &machine->kmaps);
 	if (machine->vmlinux_map == NULL)
 		return -1;
 
@@ -1098,7 +1097,6 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
 	if (!kmap)
 		return -1;
 
-	kmap->kmaps = &machine->kmaps;
 	maps__insert(&machine->kmaps, map);
 
 	return 0;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index fdd5bddb3075..a2cdfe62df94 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -223,7 +223,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
  * they are loaded) and for vmlinux, where only after we load all the
  * symbols we'll know where it starts and ends.
  */
-struct map *map__new2(u64 start, struct dso *dso)
+struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps)
 {
 	struct map *map = calloc(1, (sizeof(*map) +
 				     (dso->kernel ? sizeof(struct kmap) : 0)));
@@ -232,6 +232,19 @@ struct map *map__new2(u64 start, struct dso *dso)
 		 * ->end will be filled after we load all the symbols
 		 */
 		map__init(map, start, 0, 0, dso);
+		if (dso->kernel) {
+			/*
+			 * __map__is_kernel() Needs this for in-kernel map ranges
+			 * such as:
+			 * (gdb) print map->dso->name
+			 * $8 = 0x1232d6c "[kernel.vmlinux].init.text"
+			 * (gdb) print map->dso->kernel
+			 * $9 = DSO_TYPE_KERNEL
+			 * (gdb)
+			 */
+			struct kmap *kmap = map__kmap(map);
+			kmap->kmaps = kmaps;
+		}
 	}
 
 	return map;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 067036e8970c..b4531876cd66 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -108,7 +108,7 @@ struct dso_id;
 struct map *map__new(struct machine *machine, u64 start, u64 len,
 		     u64 pgoff, struct dso_id *id, u32 prot, u32 flags,
 		     char *filename, struct thread *thread);
-struct map *map__new2(u64 start, struct dso *dso);
+struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps);
 void map__delete(struct map *map);
 struct map *map__clone(struct map *map);
 
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index eea132f512b0..b123a800d260 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -146,7 +146,7 @@ static struct map *kernel_get_module_map(const char *module)
 
 	/* A file path -- this is an offline module */
 	if (module && strchr(module, '/'))
-		return dso__new_map(module);
+		return dso__new_map(module, maps);
 
 	if (!module) {
 		pos = machine__kernel_map(host_machine);
@@ -170,7 +170,7 @@ struct map *get_target_map(const char *target, struct nsinfo *nsi, bool user)
 	if (user) {
 		struct map *map;
 
-		map = dso__new_map(target);
+		map = dso__new_map(target, NULL);
 		if (map && map->dso)
 			map->dso->nsinfo = nsinfo__get(nsi);
 		return map;
@@ -651,12 +651,13 @@ static int
 post_process_offline_probe_trace_events(struct probe_trace_event *tevs,
 					int ntevs, const char *pathname)
 {
+	struct maps *maps = machine__kernel_maps(host_machine);
 	struct map *map;
 	unsigned long stext = 0;
 	int i, ret = 0;
 
 	/* Prepare a map for offline binary */
-	map = dso__new_map(pathname);
+	map = dso__new_map(pathname, maps);
 	if (!map || get_text_start_address(pathname, &stext, NULL) < 0) {
 		pr_warning("Failed to get ELF symbols for %s\n", pathname);
 		return -EINVAL;
@@ -2111,7 +2112,8 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
 	int ret = -ENOENT;
 
 	if (!is_kprobe) {
-		map = dso__new_map(tp->module);
+		struct maps *maps = machine__kernel_maps(host_machine);
+		map = dso__new_map(tp->module, maps);
 		if (!map)
 			goto out;
 		sym = map__find_symbol(map, addr);
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6658fbf196e6..cc896ba85a99 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -915,7 +915,7 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 		curr_dso->kernel = dso->kernel;
 		curr_dso->long_name = dso->long_name;
 		curr_dso->long_name_len = dso->long_name_len;
-		curr_map = map__new2(start, curr_dso);
+		curr_map = map__new2(start, curr_dso, kmaps);
 		dso__put(curr_dso);
 		if (curr_map == NULL)
 			return -1;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 3b379b1296f1..d166f0fb258c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -856,7 +856,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
 
 			ndso->kernel = dso->kernel;
 
-			curr_map = map__new2(pos->start, ndso);
+			curr_map = map__new2(pos->start, ndso, kmaps);
 			if (curr_map == NULL) {
 				dso__put(ndso);
 				return -1;
@@ -1144,6 +1144,7 @@ static int validate_kcore_addresses(const char *kallsyms_filename,
 
 struct kcore_mapfn_data {
 	struct dso *dso;
+	struct maps *kmaps;
 	struct list_head maps;
 };
 
@@ -1152,7 +1153,7 @@ static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
 	struct kcore_mapfn_data *md = data;
 	struct map *map;
 
-	map = map__new2(start, md->dso);
+	map = map__new2(start, md->dso, md->kmaps);
 	if (map == NULL)
 		return -ENOMEM;
 
@@ -1271,6 +1272,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 		return -EINVAL;
 
 	md.dso = dso;
+	md.kmaps = kmaps;
 	INIT_LIST_HEAD(&md.maps);
 
 	fd = open(kcore_filename, O_RDONLY);
-- 
2.21.0

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Jiri Olsa]

On Tue, Dec 17, 2019 at 11:48:28AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> +	machine->vmlinux_map = map__new2(0, kernel, &machine->kmaps);
>  	if (machine->vmlinux_map == NULL)
>  		return -1;
>  
> @@ -1098,7 +1097,6 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
>  	if (!kmap)
>  		return -1;
>  
> -	kmap->kmaps = &machine->kmaps;
>  	maps__insert(&machine->kmaps, map);
>  
>  	return 0;
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index fdd5bddb3075..a2cdfe62df94 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -223,7 +223,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
>   * they are loaded) and for vmlinux, where only after we load all the
>   * symbols we'll know where it starts and ends.
>   */
> -struct map *map__new2(u64 start, struct dso *dso)
> +struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps)
>  {
>  	struct map *map = calloc(1, (sizeof(*map) +
>  				     (dso->kernel ? sizeof(struct kmap) : 0)));
> @@ -232,6 +232,19 @@ struct map *map__new2(u64 start, struct dso *dso)
>  		 * ->end will be filled after we load all the symbols
>  		 */
>  		map__init(map, start, 0, 0, dso);

we are passing NULL for kmaps in some cases,
should we check it in here and warn?

		if (!WARN_ON_ONCE(!kmaps, "too bad..") && dso->kernel)
			

jirka

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Arnaldo Carvalho de Melo]

Em Wed, Dec 18, 2019 at 10:05:04AM +0100, Jiri Olsa escreveu:
> On Tue, Dec 17, 2019 at 11:48:28AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > +	machine->vmlinux_map = map__new2(0, kernel, &machine->kmaps);
> >  	if (machine->vmlinux_map == NULL)
> >  		return -1;
> >  
> > @@ -1098,7 +1097,6 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
> >  	if (!kmap)
> >  		return -1;
> >  
> > -	kmap->kmaps = &machine->kmaps;
> >  	maps__insert(&machine->kmaps, map);
> >  
> >  	return 0;
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index fdd5bddb3075..a2cdfe62df94 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -223,7 +223,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> >   * they are loaded) and for vmlinux, where only after we load all the
> >   * symbols we'll know where it starts and ends.
> >   */
> > -struct map *map__new2(u64 start, struct dso *dso)
> > +struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps)
> >  {
> >  	struct map *map = calloc(1, (sizeof(*map) +
> >  				     (dso->kernel ? sizeof(struct kmap) : 0)));
> > @@ -232,6 +232,19 @@ struct map *map__new2(u64 start, struct dso *dso)
> >  		 * ->end will be filled after we load all the symbols
> >  		 */
> >  		map__init(map, start, 0, 0, dso);
> 
> we are passing NULL for kmaps in some cases,
> should we check it in here and warn?
> 
> 		if (!WARN_ON_ONCE(!kmaps, "too bad..") && dso->kernel)

I'll add that, yeah

- Arnaldo

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Arnaldo Carvalho de Melo]

Em Wed, Dec 18, 2019 at 10:05:04AM +0100, Jiri Olsa escreveu:
> On Tue, Dec 17, 2019 at 11:48:28AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > +	machine->vmlinux_map = map__new2(0, kernel, &machine->kmaps);
> >  	if (machine->vmlinux_map == NULL)
> >  		return -1;
> >  
> > @@ -1098,7 +1097,6 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
> >  	if (!kmap)
> >  		return -1;
> >  
> > -	kmap->kmaps = &machine->kmaps;
> >  	maps__insert(&machine->kmaps, map);
> >  
> >  	return 0;
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index fdd5bddb3075..a2cdfe62df94 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -223,7 +223,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> >   * they are loaded) and for vmlinux, where only after we load all the
> >   * symbols we'll know where it starts and ends.
> >   */
> > -struct map *map__new2(u64 start, struct dso *dso)
> > +struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps)
> >  {
> >  	struct map *map = calloc(1, (sizeof(*map) +
> >  				     (dso->kernel ? sizeof(struct kmap) : 0)));
> > @@ -232,6 +232,19 @@ struct map *map__new2(u64 start, struct dso *dso)
> >  		 * ->end will be filled after we load all the symbols
> >  		 */
> >  		map__init(map, start, 0, 0, dso);
> 
> we are passing NULL for kmaps in some cases,
> should we check it in here and warn?
> 
> 		if (!WARN_ON_ONCE(!kmaps, "too bad..") && dso->kernel)

After looking at this some more, I retract this patch and instead the
two-liner at the end of this message seems enough.

I forgot the dso->kernel is just set for the main kernel map, and what
dso__process_kernel_symbol is doing is to split that map into chunks for
the ELF sections for the main kernel map, as an extra tidbit of
information, i.e. that symbol is in a specific main kernel section, so
its dso->kernel continues being set and thus it expects the kmap area to
be in place.

There is space for simplification in this old code, but for now the
safest is just the patch below, agreed?

- Arnaldo

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6658fbf196e6..1965aefccb02 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -920,6 +920,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 		if (curr_map == NULL)
 			return -1;
 
+		if (curr_dso->kernel)
+			map__kmap(curr_map)->kmaps = kmaps;
+
 		if (adjust_kernel_syms) {
 			curr_map->start  = shdr->sh_addr + ref_reloc(kmap);
 			curr_map->end	 = curr_map->start + shdr->sh_size;

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Arnaldo Carvalho de Melo]

Em Wed, Dec 18, 2019 at 03:22:54PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Dec 18, 2019 at 10:05:04AM +0100, Jiri Olsa escreveu:
> After looking at this some more, I retract this patch and instead the
> two-liner at the end of this message seems enough.

So here is the full cset, I have not made changes to the other patches,

- Arnaldo

commit 178427a192bcbe3ad93dd0637c3dceaa3ccef31e
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Dec 18 15:23:14 2019 -0300

    perf map: Set kmap->kmaps backpointer for main kernel map chunks
    
    When a map is create to represent the main kernel area (vmlinux) with
    map__new2() we allocate an extra area to store a pointer to the 'struct
    maps' for the kernel maps, so that we can access that struct when
    loading ELF files or kallsyms, as we will need to split it in multiple
    maps, one per kernel module or ELF section (such as ".init.text").
    
    So when map->dso->kernel is non-zero, it is expected that
    map__kmap(map)->kmaps to be set to the tree of kernel maps (modules,
    chunks of the main kernel, bpf progs put in place via
    PERF_RECORD_KSYMBOL, the main kernel).
    
    This was not the case when we were splitting the main kernel into chunks
    for its ELF sections, which ended up making 'perf report --children'
    processing a perf.data file with callchains to trip on
    __map__is_kernel(), when we press ENTER to see the popup menu for main
    histogram entries that starts at a symbol in the ".init.text" ELF
    section, e.g.:
    
    -    8.83%     0.00%  swapper     [kernel.vmlinux].init.text  [k] start_kernel
         start_kernel
         cpu_startup_entry
         do_idle
         cpuidle_enter
         cpuidle_enter_state
         intel_idle
    
    Fix it.
    
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Link: https://lkml.kernel.org/n/tip-vcokue57w4goyzweg2rr5i80@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6658fbf196e6..1965aefccb02 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -920,6 +920,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 		if (curr_map == NULL)
 			return -1;
 
+		if (curr_dso->kernel)
+			map__kmap(curr_map)->kmaps = kmaps;
+
 		if (adjust_kernel_syms) {
 			curr_map->start  = shdr->sh_addr + ref_reloc(kmap);
 			curr_map->end	 = curr_map->start + shdr->sh_size;

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Jiri Olsa]

On Wed, Dec 18, 2019 at 04:01:20PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 18, 2019 at 03:22:54PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Dec 18, 2019 at 10:05:04AM +0100, Jiri Olsa escreveu:
> > After looking at this some more, I retract this patch and instead the
> > two-liner at the end of this message seems enough.
> 
> So here is the full cset, I have not made changes to the other patches,
> 
> - Arnaldo
> 
> commit 178427a192bcbe3ad93dd0637c3dceaa3ccef31e
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Dec 18 15:23:14 2019 -0300
> 
>     perf map: Set kmap->kmaps backpointer for main kernel map chunks
>     
>     When a map is create to represent the main kernel area (vmlinux) with
>     map__new2() we allocate an extra area to store a pointer to the 'struct
>     maps' for the kernel maps, so that we can access that struct when
>     loading ELF files or kallsyms, as we will need to split it in multiple
>     maps, one per kernel module or ELF section (such as ".init.text").
>     
>     So when map->dso->kernel is non-zero, it is expected that
>     map__kmap(map)->kmaps to be set to the tree of kernel maps (modules,
>     chunks of the main kernel, bpf progs put in place via
>     PERF_RECORD_KSYMBOL, the main kernel).
>     
>     This was not the case when we were splitting the main kernel into chunks
>     for its ELF sections, which ended up making 'perf report --children'
>     processing a perf.data file with callchains to trip on
>     __map__is_kernel(), when we press ENTER to see the popup menu for main
>     histogram entries that starts at a symbol in the ".init.text" ELF
>     section, e.g.:
>     
>     -    8.83%     0.00%  swapper     [kernel.vmlinux].init.text  [k] start_kernel
>          start_kernel
>          cpu_startup_entry
>          do_idle
>          cpuidle_enter
>          cpuidle_enter_state
>          intel_idle
>     
>     Fix it.
>     
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Link: https://lkml.kernel.org/n/tip-vcokue57w4goyzweg2rr5i80@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 6658fbf196e6..1965aefccb02 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -920,6 +920,9 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
>  		if (curr_map == NULL)
>  			return -1;
>  
> +		if (curr_dso->kernel)
> +			map__kmap(curr_map)->kmaps = kmaps;
> +
>  		if (adjust_kernel_syms) {
>  			curr_map->start  = shdr->sh_addr + ref_reloc(kmap);
>  			curr_map->end	 = curr_map->start + shdr->sh_size;
> 

looks good ;-)

jirka

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Jiri Olsa]

On Tue, Dec 17, 2019 at 11:48:28AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

>  
> @@ -1098,7 +1097,6 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
>  	if (!kmap)
>  		return -1;
>  
> -	kmap->kmaps = &machine->kmaps;
>  	maps__insert(&machine->kmaps, map);
>  
>  	return 0;
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index fdd5bddb3075..a2cdfe62df94 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -223,7 +223,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
>   * they are loaded) and for vmlinux, where only after we load all the
>   * symbols we'll know where it starts and ends.
>   */
> -struct map *map__new2(u64 start, struct dso *dso)
> +struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps)

hum, how about map__new? kernel maps could go throught that as well, right?

jirka

Re: [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps

[Arnaldo Carvalho de Melo]

Em Wed, Dec 18, 2019 at 10:07:07AM +0100, Jiri Olsa escreveu:
> On Tue, Dec 17, 2019 at 11:48:28AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> >  
> > @@ -1098,7 +1097,6 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
> >  	if (!kmap)
> >  		return -1;
> >  
> > -	kmap->kmaps = &machine->kmaps;
> >  	maps__insert(&machine->kmaps, map);
> >  
> >  	return 0;
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index fdd5bddb3075..a2cdfe62df94 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -223,7 +223,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> >   * they are loaded) and for vmlinux, where only after we load all the
> >   * symbols we'll know where it starts and ends.
> >   */
> > -struct map *map__new2(u64 start, struct dso *dso)
> > +struct map *map__new2(u64 start, struct dso *dso, struct maps *kmaps)

> hum, how about map__new? kernel maps could go throught that as well, right?

Nope, I even thought about renaming map__new2() to map__new_kmap() as
only it is used to create kernel/modules maps, as is stated above in the
comment just before map__new2().

map__new() is only called from machine__process_mmap_event() and machine__process_mmap2_event()
and only after checking if it is not a kernel "mmap":

        if (sample->cpumode == PERF_RECORD_MISC_GUEST_KERNEL ||
            sample->cpumode == PERF_RECORD_MISC_KERNEL) {
                ret = machine__process_kernel_mmap_event(machine, event);
		if (ret < 0)
                        goto out_problem;
                return 0;
	...
	map = map__new(...);

- Arnaldo