linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] perf tools: Assorted random fixes and updates
@ 2022-09-23 17:31 Namhyung Kim
  2022-09-23 17:31 ` [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples() Namhyung Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Namhyung Kim @ 2022-09-23 17:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
	linux-perf-users, Stephane Eranian

Hello,

This is collection of random fixes and updates.
And I'm resending them as they seem to be lost in the list.

Thanks,
Namhyung


Namhyung Kim (4):
  perf record: Fix a segfault in record__read_lost_samples()
  perf inject: Clarify build-id options a little bit
  perf tools: Add 'addr' sort key
  perf annotate: Toggle full address <-> offset display

 tools/perf/Documentation/perf-inject.txt |  6 ++--
 tools/perf/Documentation/perf-report.txt |  3 +-
 tools/perf/builtin-record.c              |  6 ++++
 tools/perf/ui/browsers/annotate.c        |  6 +++-
 tools/perf/util/annotate.c               | 19 +++++++++++-
 tools/perf/util/annotate.h               |  4 ++-
 tools/perf/util/hist.c                   |  1 +
 tools/perf/util/hist.h                   |  1 +
 tools/perf/util/sort.c                   | 38 ++++++++++++++++++++++++
 tools/perf/util/sort.h                   |  1 +
 10 files changed, 79 insertions(+), 6 deletions(-)

-- 
2.37.3.998.g577e59143f-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples()
  2022-09-23 17:31 [PATCH 0/4] perf tools: Assorted random fixes and updates Namhyung Kim
@ 2022-09-23 17:31 ` Namhyung Kim
  2022-09-23 23:20   ` Ian Rogers
  2022-09-25  1:08   ` Leo Yan
  2022-09-23 17:31 ` [PATCH 2/4] perf inject: Clarify build-id options a little bit Namhyung Kim
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 10+ messages in thread
From: Namhyung Kim @ 2022-09-23 17:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
	linux-perf-users, Stephane Eranian

When it fails to open events record__open() returns without setting the
session->evlist.  Then it gets a segfault in the function trying to read
lost sample counts.  You can easily reproduce it as a normal user like:

  $ perf record -p 1 true
  ...
  perf: Segmentation fault
  ...

Skip the function if it has no evlist.  And add more protection for evsels
which are not properly initialized.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 741e763436ca..f4f1619199e5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1888,6 +1888,10 @@ static void record__read_lost_samples(struct record *rec)
 	struct perf_record_lost_samples *lost;
 	struct evsel *evsel;
 
+	/* there was an error during record__open */
+	if (session->evlist == NULL)
+		return;
+
 	lost = zalloc(PERF_SAMPLE_MAX_SIZE);
 	if (lost == NULL) {
 		pr_debug("Memory allocation failed\n");
@@ -1899,6 +1903,8 @@ static void record__read_lost_samples(struct record *rec)
 	evlist__for_each_entry(session->evlist, evsel) {
 		struct xyarray *xy = evsel->core.sample_id;
 
+		if (xy == NULL || evsel->core.fd == NULL)
+			continue;
 		if (xyarray__max_x(evsel->core.fd) != xyarray__max_x(xy) ||
 		    xyarray__max_y(evsel->core.fd) != xyarray__max_y(xy)) {
 			pr_debug("Unmatched FD vs. sample ID: skip reading LOST count\n");
-- 
2.37.3.998.g577e59143f-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] perf inject: Clarify build-id options a little bit
  2022-09-23 17:31 [PATCH 0/4] perf tools: Assorted random fixes and updates Namhyung Kim
  2022-09-23 17:31 ` [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples() Namhyung Kim
@ 2022-09-23 17:31 ` Namhyung Kim
  2022-09-23 17:31 ` [PATCH 3/4] perf tools: Add 'addr' sort key Namhyung Kim
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Namhyung Kim @ 2022-09-23 17:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
	linux-perf-users, Stephane Eranian

Update the documentation of --build-id and --buildid-all options to
clarify the difference between them.  The former requires full sample
processing to find which DSOs are actually used.  While the latter simply
injects every DSO's build-id from MMAP{,2} records, skipping SAMPLEs.

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-inject.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-inject.txt b/tools/perf/Documentation/perf-inject.txt
index 70e2ac3cc91a..c972032f4ca0 100644
--- a/tools/perf/Documentation/perf-inject.txt
+++ b/tools/perf/Documentation/perf-inject.txt
@@ -25,10 +25,12 @@ OPTIONS
 -------
 -b::
 --build-ids::
-        Inject build-ids into the output stream
+	Inject build-ids of DSOs hit by samples into the output stream.
+	This means it needs to process all SAMPLE records to find the DSOs.
 
 --buildid-all::
-	Inject build-ids of all DSOs into the output stream
+	Inject build-ids of all DSOs into the output stream regardless of hits
+	and skip SAMPLE processing.
 
 --known-build-ids=::
 	Override build-ids to inject using these comma-separated pairs of
-- 
2.37.3.998.g577e59143f-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] perf tools: Add 'addr' sort key
  2022-09-23 17:31 [PATCH 0/4] perf tools: Assorted random fixes and updates Namhyung Kim
  2022-09-23 17:31 ` [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples() Namhyung Kim
  2022-09-23 17:31 ` [PATCH 2/4] perf inject: Clarify build-id options a little bit Namhyung Kim
@ 2022-09-23 17:31 ` Namhyung Kim
  2022-09-23 23:21   ` Ian Rogers
  2022-09-23 17:31 ` [PATCH 4/4] perf annotate: Toggle full address <-> offset display Namhyung Kim
  2022-09-26 13:09 ` [PATCH 0/4] perf tools: Assorted random fixes and updates Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 10+ messages in thread
From: Namhyung Kim @ 2022-09-23 17:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
	linux-perf-users, Stephane Eranian

Sometimes users want to see actual (virtual) address of sampled instructions.
Add a new 'addr' sort key to display the raw addresses.

  $ perf record -o- true | perf report -i- -s addr
  # To display the perf.data header info, please use --header/--header-only options.
  #
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  #
  # Total Lost Samples: 0
  #
  # Samples: 12  of event 'cycles:u'
  # Event count (approx.): 252512
  #
  # Overhead  Address
  # ........  ..................
  #
      42.96%  0x7f96f08443d7
      29.55%  0x7f96f0859b50
      14.76%  0x7f96f0852e02
       8.30%  0x7f96f0855028
       4.43%  0xffffffff8de01087

Note that it just compares and displays the sample ip.  Each process can
have a different memory layout and the ip will be different even if they run
the same binary.  So this sort key is mostly meaningful for per-process
profile data.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt |  3 +-
 tools/perf/util/hist.c                   |  1 +
 tools/perf/util/hist.h                   |  1 +
 tools/perf/util/sort.c                   | 38 ++++++++++++++++++++++++
 tools/perf/util/sort.h                   |  1 +
 5 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 24efc0583c93..4533db2ee56b 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -73,7 +73,7 @@ OPTIONS
 	Sort histogram entries by given key(s) - multiple keys can be specified
 	in CSV format.  Following sort keys are available:
 	pid, comm, dso, symbol, parent, cpu, socket, srcline, weight,
-	local_weight, cgroup_id.
+	local_weight, cgroup_id, addr.
 
 	Each key has following meaning:
 
@@ -114,6 +114,7 @@ OPTIONS
 	- local_ins_lat: Local instruction latency version
 	- p_stage_cyc: On powerpc, this presents the number of cycles spent in a
 	  pipeline stage. And currently supported only on powerpc.
+	- addr: (Full) virtual address of the sampled instruction
 
 	By default, comm, dso and symbol keys are used.
 	(i.e. --sort comm,dso,symbol)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 06f5dbf213ad..17a05e943b44 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -215,6 +215,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
 	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
 	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
+	hists__new_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
 
 	if (symbol_conf.nanosecs)
 		hists__new_col_len(hists, HISTC_TIME, 16);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index c7a7a3fa0b87..ebd8a8f783ee 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -79,6 +79,7 @@ enum hist_column {
 	HISTC_GLOBAL_P_STAGE_CYC,
 	HISTC_ADDR_FROM,
 	HISTC_ADDR_TO,
+	HISTC_ADDR,
 	HISTC_NR_COLS, /* Last entry */
 };
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 6d5588e80935..2e7330867e2e 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1948,6 +1948,43 @@ struct sort_entry sort_dso_size = {
 	.se_width_idx	= HISTC_DSO_SIZE,
 };
 
+/* --sort dso_size */
+
+static int64_t
+sort__addr_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	u64 left_ip = left->ip;
+	u64 right_ip = right->ip;
+	struct map *left_map = left->ms.map;
+	struct map *right_map = right->ms.map;
+
+	if (left_map)
+		left_ip = left_map->unmap_ip(left_map, left_ip);
+	if (right_map)
+		right_ip = right_map->unmap_ip(right_map, right_ip);
+
+	return _sort__addr_cmp(left_ip, right_ip);
+}
+
+static int hist_entry__addr_snprintf(struct hist_entry *he, char *bf,
+				     size_t size, unsigned int width)
+{
+	u64 ip = he->ip;
+	struct map *map = he->ms.map;
+
+	if (map)
+		ip = map->unmap_ip(map, ip);
+
+	return repsep_snprintf(bf, size, "%-#*llx", width, ip);
+}
+
+struct sort_entry sort_addr = {
+	.se_header	= "Address",
+	.se_cmp		= sort__addr_cmp,
+	.se_snprintf	= hist_entry__addr_snprintf,
+	.se_width_idx	= HISTC_ADDR,
+};
+
 
 struct sort_dimension {
 	const char		*name;
@@ -1997,6 +2034,7 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
 	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
 	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
+	DIM(SORT_ADDR, "addr", sort_addr),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index af14eb46c2b6..04ff8b61a2a7 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -236,6 +236,7 @@ enum sort_type {
 	SORT_GLOBAL_INS_LAT,
 	SORT_LOCAL_PIPELINE_STAGE_CYC,
 	SORT_GLOBAL_PIPELINE_STAGE_CYC,
+	SORT_ADDR,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
2.37.3.998.g577e59143f-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] perf annotate: Toggle full address <-> offset display
  2022-09-23 17:31 [PATCH 0/4] perf tools: Assorted random fixes and updates Namhyung Kim
                   ` (2 preceding siblings ...)
  2022-09-23 17:31 ` [PATCH 3/4] perf tools: Add 'addr' sort key Namhyung Kim
@ 2022-09-23 17:31 ` Namhyung Kim
  2022-09-23 23:22   ` Ian Rogers
  2022-09-26 13:09 ` [PATCH 0/4] perf tools: Assorted random fixes and updates Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 10+ messages in thread
From: Namhyung Kim @ 2022-09-23 17:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
	linux-perf-users, Stephane Eranian

Handle 'f' key to toggle the display offset and full address.  Obviously
it only works when users set to see disassembler output ('o' key).  It'd
be useful when users want to see the full virtual address in the TUI
annotate browser.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate.c |  6 +++++-
 tools/perf/util/annotate.c        | 19 ++++++++++++++++++-
 tools/perf/util/annotate.h        |  4 +++-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 9bc1076374ff..725662e21b23 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -805,7 +805,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"r             Run available scripts\n"
 		"p             Toggle percent type [local/global]\n"
 		"b             Toggle percent base [period/hits]\n"
-		"?             Search string backwards\n");
+		"?             Search string backwards\n"
+		"f             Toggle showing offsets to full address\n");
 			continue;
 		case 'r':
 			script_browse(NULL, NULL);
@@ -912,6 +913,9 @@ static int annotate_browser__run(struct annotate_browser *browser,
 			hists__scnprintf_title(hists, title, sizeof(title));
 			annotate_browser__show(&browser->b, title, help);
 			continue;
+		case 'f':
+			annotation__toggle_full_addr(notes, ms);
+			continue;
 		case K_LEFT:
 		case K_ESC:
 		case 'q':
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 5bc63c9e0324..db475e44f42f 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2239,7 +2239,10 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
 	}
 
 	args.ms = *ms;
-	notes->start = map__rip_2objdump(ms->map, sym->start);
+	if (notes->options && notes->options->full_addr)
+		notes->start = map__objdump_2mem(ms->map, ms->sym->start);
+	else
+		notes->start = map__rip_2objdump(ms->map, ms->sym->start);
 
 	return symbol__disassemble(sym, &args);
 }
@@ -2762,6 +2765,8 @@ void annotation__update_column_widths(struct annotation *notes)
 {
 	if (notes->options->use_offset)
 		notes->widths.target = notes->widths.min_addr;
+	else if (notes->options->full_addr)
+		notes->widths.target = BITS_PER_LONG / 4;
 	else
 		notes->widths.target = notes->widths.max_addr;
 
@@ -2771,6 +2776,18 @@ void annotation__update_column_widths(struct annotation *notes)
 		notes->widths.addr += notes->widths.jumps + 1;
 }
 
+void annotation__toggle_full_addr(struct annotation *notes, struct map_symbol *ms)
+{
+	notes->options->full_addr = !notes->options->full_addr;
+
+	if (notes->options->full_addr)
+		notes->start = map__objdump_2mem(ms->map, ms->sym->start);
+	else
+		notes->start = map__rip_2objdump(ms->map, ms->sym->start);
+
+	annotation__update_column_widths(notes);
+}
+
 static void annotation__calc_lines(struct annotation *notes, struct map *map,
 				   struct rb_root *root,
 				   struct annotation_options *opts)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 3cbd883e4d7a..8934072c39e6 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -88,7 +88,8 @@ struct annotation_options {
 	     show_nr_jumps,
 	     show_minmax_cycle,
 	     show_asm_raw,
-	     annotate_src;
+	     annotate_src,
+	     full_addr;
 	u8   offset_level;
 	int  min_pcnt;
 	int  max_lines;
@@ -325,6 +326,7 @@ void annotation__compute_ipc(struct annotation *notes, size_t size);
 void annotation__mark_jump_targets(struct annotation *notes, struct symbol *sym);
 void annotation__update_column_widths(struct annotation *notes);
 void annotation__init_column_widths(struct annotation *notes, struct symbol *sym);
+void annotation__toggle_full_addr(struct annotation *notes, struct map_symbol *ms);
 
 static inline struct sym_hist *annotated_source__histogram(struct annotated_source *src, int idx)
 {
-- 
2.37.3.998.g577e59143f-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples()
  2022-09-23 17:31 ` [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples() Namhyung Kim
@ 2022-09-23 23:20   ` Ian Rogers
  2022-09-25  1:08   ` Leo Yan
  1 sibling, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2022-09-23 23:20 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	LKML, Adrian Hunter, linux-perf-users, Stephane Eranian

On Fri, Sep 23, 2022 at 10:32 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> When it fails to open events record__open() returns without setting the
> session->evlist.  Then it gets a segfault in the function trying to read
> lost sample counts.  You can easily reproduce it as a normal user like:
>
>   $ perf record -p 1 true
>   ...
>   perf: Segmentation fault
>   ...
>
> Skip the function if it has no evlist.  And add more protection for evsels
> which are not properly initialized.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-record.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 741e763436ca..f4f1619199e5 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1888,6 +1888,10 @@ static void record__read_lost_samples(struct record *rec)
>         struct perf_record_lost_samples *lost;
>         struct evsel *evsel;
>
> +       /* there was an error during record__open */
> +       if (session->evlist == NULL)
> +               return;
> +
>         lost = zalloc(PERF_SAMPLE_MAX_SIZE);
>         if (lost == NULL) {
>                 pr_debug("Memory allocation failed\n");
> @@ -1899,6 +1903,8 @@ static void record__read_lost_samples(struct record *rec)
>         evlist__for_each_entry(session->evlist, evsel) {
>                 struct xyarray *xy = evsel->core.sample_id;
>
> +               if (xy == NULL || evsel->core.fd == NULL)
> +                       continue;
>                 if (xyarray__max_x(evsel->core.fd) != xyarray__max_x(xy) ||
>                     xyarray__max_y(evsel->core.fd) != xyarray__max_y(xy)) {
>                         pr_debug("Unmatched FD vs. sample ID: skip reading LOST count\n");
> --
> 2.37.3.998.g577e59143f-goog
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] perf tools: Add 'addr' sort key
  2022-09-23 17:31 ` [PATCH 3/4] perf tools: Add 'addr' sort key Namhyung Kim
@ 2022-09-23 23:21   ` Ian Rogers
  0 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2022-09-23 23:21 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	LKML, Adrian Hunter, linux-perf-users, Stephane Eranian

On Fri, Sep 23, 2022 at 10:32 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Sometimes users want to see actual (virtual) address of sampled instructions.
> Add a new 'addr' sort key to display the raw addresses.
>
>   $ perf record -o- true | perf report -i- -s addr
>   # To display the perf.data header info, please use --header/--header-only options.
>   #
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.000 MB - ]
>   #
>   # Total Lost Samples: 0
>   #
>   # Samples: 12  of event 'cycles:u'
>   # Event count (approx.): 252512
>   #
>   # Overhead  Address
>   # ........  ..................
>   #
>       42.96%  0x7f96f08443d7
>       29.55%  0x7f96f0859b50
>       14.76%  0x7f96f0852e02
>        8.30%  0x7f96f0855028
>        4.43%  0xffffffff8de01087
>
> Note that it just compares and displays the sample ip.  Each process can
> have a different memory layout and the ip will be different even if they run
> the same binary.  So this sort key is mostly meaningful for per-process
> profile data.
>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/Documentation/perf-report.txt |  3 +-
>  tools/perf/util/hist.c                   |  1 +
>  tools/perf/util/hist.h                   |  1 +
>  tools/perf/util/sort.c                   | 38 ++++++++++++++++++++++++
>  tools/perf/util/sort.h                   |  1 +
>  5 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index 24efc0583c93..4533db2ee56b 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -73,7 +73,7 @@ OPTIONS
>         Sort histogram entries by given key(s) - multiple keys can be specified
>         in CSV format.  Following sort keys are available:
>         pid, comm, dso, symbol, parent, cpu, socket, srcline, weight,
> -       local_weight, cgroup_id.
> +       local_weight, cgroup_id, addr.
>
>         Each key has following meaning:
>
> @@ -114,6 +114,7 @@ OPTIONS
>         - local_ins_lat: Local instruction latency version
>         - p_stage_cyc: On powerpc, this presents the number of cycles spent in a
>           pipeline stage. And currently supported only on powerpc.
> +       - addr: (Full) virtual address of the sampled instruction
>
>         By default, comm, dso and symbol keys are used.
>         (i.e. --sort comm,dso,symbol)
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 06f5dbf213ad..17a05e943b44 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -215,6 +215,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>         hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
>         hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
>         hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
> +       hists__new_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
>
>         if (symbol_conf.nanosecs)
>                 hists__new_col_len(hists, HISTC_TIME, 16);
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index c7a7a3fa0b87..ebd8a8f783ee 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -79,6 +79,7 @@ enum hist_column {
>         HISTC_GLOBAL_P_STAGE_CYC,
>         HISTC_ADDR_FROM,
>         HISTC_ADDR_TO,
> +       HISTC_ADDR,
>         HISTC_NR_COLS, /* Last entry */
>  };
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 6d5588e80935..2e7330867e2e 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1948,6 +1948,43 @@ struct sort_entry sort_dso_size = {
>         .se_width_idx   = HISTC_DSO_SIZE,
>  };
>
> +/* --sort dso_size */
> +
> +static int64_t
> +sort__addr_cmp(struct hist_entry *left, struct hist_entry *right)
> +{
> +       u64 left_ip = left->ip;
> +       u64 right_ip = right->ip;
> +       struct map *left_map = left->ms.map;
> +       struct map *right_map = right->ms.map;
> +
> +       if (left_map)
> +               left_ip = left_map->unmap_ip(left_map, left_ip);
> +       if (right_map)
> +               right_ip = right_map->unmap_ip(right_map, right_ip);
> +
> +       return _sort__addr_cmp(left_ip, right_ip);
> +}
> +
> +static int hist_entry__addr_snprintf(struct hist_entry *he, char *bf,
> +                                    size_t size, unsigned int width)
> +{
> +       u64 ip = he->ip;
> +       struct map *map = he->ms.map;
> +
> +       if (map)
> +               ip = map->unmap_ip(map, ip);
> +
> +       return repsep_snprintf(bf, size, "%-#*llx", width, ip);
> +}
> +
> +struct sort_entry sort_addr = {
> +       .se_header      = "Address",
> +       .se_cmp         = sort__addr_cmp,
> +       .se_snprintf    = hist_entry__addr_snprintf,
> +       .se_width_idx   = HISTC_ADDR,
> +};
> +
>
>  struct sort_dimension {
>         const char              *name;
> @@ -1997,6 +2034,7 @@ static struct sort_dimension common_sort_dimensions[] = {
>         DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
>         DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
>         DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
> +       DIM(SORT_ADDR, "addr", sort_addr),
>  };
>
>  #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index af14eb46c2b6..04ff8b61a2a7 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -236,6 +236,7 @@ enum sort_type {
>         SORT_GLOBAL_INS_LAT,
>         SORT_LOCAL_PIPELINE_STAGE_CYC,
>         SORT_GLOBAL_PIPELINE_STAGE_CYC,
> +       SORT_ADDR,
>
>         /* branch stack specific sort keys */
>         __SORT_BRANCH_STACK,
> --
> 2.37.3.998.g577e59143f-goog
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] perf annotate: Toggle full address <-> offset display
  2022-09-23 17:31 ` [PATCH 4/4] perf annotate: Toggle full address <-> offset display Namhyung Kim
@ 2022-09-23 23:22   ` Ian Rogers
  0 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2022-09-23 23:22 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	LKML, Adrian Hunter, linux-perf-users, Stephane Eranian

On Fri, Sep 23, 2022 at 10:32 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Handle 'f' key to toggle the display offset and full address.  Obviously
> it only works when users set to see disassembler output ('o' key).  It'd
> be useful when users want to see the full virtual address in the TUI
> annotate browser.
>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/ui/browsers/annotate.c |  6 +++++-
>  tools/perf/util/annotate.c        | 19 ++++++++++++++++++-
>  tools/perf/util/annotate.h        |  4 +++-
>  3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 9bc1076374ff..725662e21b23 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -805,7 +805,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
>                 "r             Run available scripts\n"
>                 "p             Toggle percent type [local/global]\n"
>                 "b             Toggle percent base [period/hits]\n"
> -               "?             Search string backwards\n");
> +               "?             Search string backwards\n"
> +               "f             Toggle showing offsets to full address\n");
>                         continue;
>                 case 'r':
>                         script_browse(NULL, NULL);
> @@ -912,6 +913,9 @@ static int annotate_browser__run(struct annotate_browser *browser,
>                         hists__scnprintf_title(hists, title, sizeof(title));
>                         annotate_browser__show(&browser->b, title, help);
>                         continue;
> +               case 'f':
> +                       annotation__toggle_full_addr(notes, ms);
> +                       continue;
>                 case K_LEFT:
>                 case K_ESC:
>                 case 'q':
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 5bc63c9e0324..db475e44f42f 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2239,7 +2239,10 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
>         }
>
>         args.ms = *ms;
> -       notes->start = map__rip_2objdump(ms->map, sym->start);
> +       if (notes->options && notes->options->full_addr)
> +               notes->start = map__objdump_2mem(ms->map, ms->sym->start);
> +       else
> +               notes->start = map__rip_2objdump(ms->map, ms->sym->start);
>
>         return symbol__disassemble(sym, &args);
>  }
> @@ -2762,6 +2765,8 @@ void annotation__update_column_widths(struct annotation *notes)
>  {
>         if (notes->options->use_offset)
>                 notes->widths.target = notes->widths.min_addr;
> +       else if (notes->options->full_addr)
> +               notes->widths.target = BITS_PER_LONG / 4;
>         else
>                 notes->widths.target = notes->widths.max_addr;
>
> @@ -2771,6 +2776,18 @@ void annotation__update_column_widths(struct annotation *notes)
>                 notes->widths.addr += notes->widths.jumps + 1;
>  }
>
> +void annotation__toggle_full_addr(struct annotation *notes, struct map_symbol *ms)
> +{
> +       notes->options->full_addr = !notes->options->full_addr;
> +
> +       if (notes->options->full_addr)
> +               notes->start = map__objdump_2mem(ms->map, ms->sym->start);
> +       else
> +               notes->start = map__rip_2objdump(ms->map, ms->sym->start);
> +
> +       annotation__update_column_widths(notes);
> +}
> +
>  static void annotation__calc_lines(struct annotation *notes, struct map *map,
>                                    struct rb_root *root,
>                                    struct annotation_options *opts)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 3cbd883e4d7a..8934072c39e6 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -88,7 +88,8 @@ struct annotation_options {
>              show_nr_jumps,
>              show_minmax_cycle,
>              show_asm_raw,
> -            annotate_src;
> +            annotate_src,
> +            full_addr;
>         u8   offset_level;
>         int  min_pcnt;
>         int  max_lines;
> @@ -325,6 +326,7 @@ void annotation__compute_ipc(struct annotation *notes, size_t size);
>  void annotation__mark_jump_targets(struct annotation *notes, struct symbol *sym);
>  void annotation__update_column_widths(struct annotation *notes);
>  void annotation__init_column_widths(struct annotation *notes, struct symbol *sym);
> +void annotation__toggle_full_addr(struct annotation *notes, struct map_symbol *ms);
>
>  static inline struct sym_hist *annotated_source__histogram(struct annotated_source *src, int idx)
>  {
> --
> 2.37.3.998.g577e59143f-goog
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples()
  2022-09-23 17:31 ` [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples() Namhyung Kim
  2022-09-23 23:20   ` Ian Rogers
@ 2022-09-25  1:08   ` Leo Yan
  1 sibling, 0 replies; 10+ messages in thread
From: Leo Yan @ 2022-09-25  1:08 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	LKML, Ian Rogers, Adrian Hunter, linux-perf-users,
	Stephane Eranian

On Fri, Sep 23, 2022 at 10:31:39AM -0700, Namhyung Kim wrote:
> When it fails to open events record__open() returns without setting the
> session->evlist.  Then it gets a segfault in the function trying to read
> lost sample counts.  You can easily reproduce it as a normal user like:
> 
>   $ perf record -p 1 true
>   ...
>   perf: Segmentation fault
>   ...
> 
> Skip the function if it has no evlist.  And add more protection for evsels
> which are not properly initialized.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Tested-by: Leo Yan <leo.yan@linaro.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] perf tools: Assorted random fixes and updates
  2022-09-23 17:31 [PATCH 0/4] perf tools: Assorted random fixes and updates Namhyung Kim
                   ` (3 preceding siblings ...)
  2022-09-23 17:31 ` [PATCH 4/4] perf annotate: Toggle full address <-> offset display Namhyung Kim
@ 2022-09-26 13:09 ` Arnaldo Carvalho de Melo
  4 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-26 13:09 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers,
	Adrian Hunter, linux-perf-users, Stephane Eranian

Em Fri, Sep 23, 2022 at 10:31:38AM -0700, Namhyung Kim escreveu:
> Hello,
> 
> This is collection of random fixes and updates.
> And I'm resending them as they seem to be lost in the list.

Thanks, applied.

- Arnaldo

 
> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (4):
>   perf record: Fix a segfault in record__read_lost_samples()
>   perf inject: Clarify build-id options a little bit
>   perf tools: Add 'addr' sort key
>   perf annotate: Toggle full address <-> offset display
> 
>  tools/perf/Documentation/perf-inject.txt |  6 ++--
>  tools/perf/Documentation/perf-report.txt |  3 +-
>  tools/perf/builtin-record.c              |  6 ++++
>  tools/perf/ui/browsers/annotate.c        |  6 +++-
>  tools/perf/util/annotate.c               | 19 +++++++++++-
>  tools/perf/util/annotate.h               |  4 ++-
>  tools/perf/util/hist.c                   |  1 +
>  tools/perf/util/hist.h                   |  1 +
>  tools/perf/util/sort.c                   | 38 ++++++++++++++++++++++++
>  tools/perf/util/sort.h                   |  1 +
>  10 files changed, 79 insertions(+), 6 deletions(-)
> 
> -- 
> 2.37.3.998.g577e59143f-goog

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-09-26 14:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 17:31 [PATCH 0/4] perf tools: Assorted random fixes and updates Namhyung Kim
2022-09-23 17:31 ` [PATCH 1/4] perf record: Fix a segfault in record__read_lost_samples() Namhyung Kim
2022-09-23 23:20   ` Ian Rogers
2022-09-25  1:08   ` Leo Yan
2022-09-23 17:31 ` [PATCH 2/4] perf inject: Clarify build-id options a little bit Namhyung Kim
2022-09-23 17:31 ` [PATCH 3/4] perf tools: Add 'addr' sort key Namhyung Kim
2022-09-23 23:21   ` Ian Rogers
2022-09-23 17:31 ` [PATCH 4/4] perf annotate: Toggle full address <-> offset display Namhyung Kim
2022-09-23 23:22   ` Ian Rogers
2022-09-26 13:09 ` [PATCH 0/4] perf tools: Assorted random fixes and updates Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).