Misc improvements and bug fixes for perf

Misc improvements and bug fixes for perf

[Andi Kleen]

Mostly unrelated to each other, so can be picked'n'chosed.

- Fix perf stat --no-scale
- Support --reltime in perf script
- Fix crashes with stat -r
- Handle JITed code better in perf report
- Allow to limit rotated perf.data files
- Filter metrics in perf list
- Show all sort keys in perf report help
- Fix multiplexing formula

Also available in
git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/misc-fixes-4

[PATCH v1 01/10] perf, tools, list: Filter metrics too

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

When a filter is specified on the command line, filter the metrics too.

Before:

% perf list foo
List of pre-defined events (to be used in -e):

Metric Groups:

DSB:
  DSB_Coverage
       [Fraction of Uops delivered by the DSB (aka Decoded Icache; or Uop Cache)]
... more metrics ...

After:

% perf list foo

List of pre-defined events (to be used in -e):

Metric Groups:

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/builtin-list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index c9f98d00c0e9..a8394b4f1167 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -119,7 +119,7 @@ int cmd_list(int argc, const char **argv)
 						details_flag);
 			print_tracepoint_events(NULL, s, raw_dump);
 			print_sdt_events(NULL, s, raw_dump);
-			metricgroup__print(true, true, NULL, raw_dump, details_flag);
+			metricgroup__print(true, true, s, raw_dump, details_flag);
 			free(s);
 		}
 	}
-- 
2.20.1

[PATCH v1 02/10] perf, tools, stat: Avoid memory overrun with -r

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

When -r is used memory would get corrupted because the evsel->id array
would get overrun. evsel->ids is a running counter of the last id.
Normally this works fine, but with -r the same event is initialized
multiple times, but not this counter, so it would keep growing
beyond the array limit and corrupt random memory.

Always reinitialize ->ids, and also add an assert to catch
such overruns in the future.

This fixes a perf segfault when running it from toplev.

Before:

$ valgrind perf stat -r2 -e '{cycles,cycles,cycles,cycles}' true
==27012== Memcheck, a memory error detector
==27012== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27012== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==27012== Command: perf stat -r2 -e {cycles,cycles,cycles,cycles} true
==27012==
==27012== Invalid write of size 8
==27012==    at 0x33090F: perf_evlist__id_add_fd (in /usr/bin/perf)
==27012==    by 0x33C99B: perf_evsel__store_ids (in /usr/bin/perf)
==27012==    by 0x2B7E1D: ??? (in /usr/bin/perf)
==27012==    by 0x2B97DE: cmd_stat (in /usr/bin/perf)
==27012==    by 0x31BFC0: ??? (in /usr/bin/perf)
==27012==    by 0x29C7A9: main (in /usr/bin/perf)
==27012==  Address 0x13182be8 is 0 bytes after a block of size 8 alloc'd
==27012==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==27012==    by 0x33C921: perf_evsel__store_ids (in /usr/bin/perf)
==27012==    by 0x2B7E1D: ??? (in /usr/bin/perf)
==27012==    by 0x2B97DE: cmd_stat (in /usr/bin/perf)
==27012==    by 0x31BFC0: ??? (in /usr/bin/perf)
==27012==    by 0x29C7A9: main (in /usr/bin/perf)
==27012==
...

After:

$ valgrind ./perf stat -r2 -e '{cycles,cycles,cycles,cycles}' true
==27026== Memcheck, a memory error detector
==27026== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27026== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==27026== Command: ./perf stat -r2 -e {cycles,cycles,cycles,cycles} true
==27026==

 Performance counter stats for 'true' (2 runs):

...

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/evlist.c | 1 +
 tools/perf/util/evsel.c  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index ed20f4379956..4f02bccba204 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -529,6 +529,7 @@ void perf_evlist__id_add(struct perf_evlist *evlist, struct perf_evsel *evsel,
 			 int cpu, int thread, u64 id)
 {
 	perf_evlist__id_hash(evlist, evsel, cpu, thread, id);
+	assert(evsel->ids < evsel->sample_id->max_x * evsel->sample_id->max_y);
 	evsel->id[evsel->ids++] = id;
 }
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3bbf73e979c0..686318f69b1d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3001,5 +3001,7 @@ int perf_evsel__store_ids(struct perf_evsel *evsel, struct perf_evlist *evlist)
 	if (perf_evsel__alloc_id(evsel, cpus->nr, threads->nr))
 		return -ENOMEM;
 
+	evsel->ids = 0;
+
 	return store_evsel_ids(evsel, evlist);
 }
-- 
2.20.1

Re: [PATCH v1 02/10] perf, tools, stat: Avoid memory overrun with -r

[Andi Kleen]

On Mon, Mar 11, 2019 at 01:24:38PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> When -r is used memory would get corrupted because the evsel->id array
> would get overrun. evsel->ids is a running counter of the last id.
> Normally this works fine, but with -r the same event is initialized
> multiple times, but not this counter, so it would keep growing
> beyond the array limit and corrupt random memory.
> 
> Always reinitialize ->ids, and also add an assert to catch
> such overruns in the future.
> 
> This fixes a perf segfault when running it from toplev.

This one should be Cc: stable

-Andi

[PATCH v1 03/10] perf, tools, record: Allow to limit number of reported perf.data files

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

When doing long term recording and waiting for some event
to snapshot on, we often only care about the last minute or so.

--switch-output supports rotating the perf.data file when the
size exceeds a threshold. But the disk would still be filled
with unnecessary old files.

Add a new option to only keep a number of rotated files,
so that the disk space usage can be limited.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-record.txt |  4 ++++
 tools/perf/builtin-record.c              | 30 +++++++++++++++++++++++-
 tools/perf/util/data.c                   | 11 ++++-----
 tools/perf/util/data.h                   |  2 +-
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 8f0c2be34848..8fe4dffcadd0 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -495,6 +495,10 @@ overhead. You can still switch them on with:
 
   --switch-output --no-no-buildid  --no-no-buildid-cache
 
+--switch-max-files=N::
+
+When rotating perf.data with --switch-output, only keep N files.
+
 --dry-run::
 Parse options then exit. --dry-run can be used to detect errors in cmdline
 options.
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a468d882e74f..02d7c40b2d10 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -62,6 +62,9 @@ struct switch_output {
 	unsigned long	 time;
 	const char	*str;
 	bool		 set;
+	char		 **filenames;
+	int		 num_files;
+	int		 cur_file;
 };
 
 struct record {
@@ -892,6 +895,7 @@ record__switch_output(struct record *rec, bool at_exit)
 {
 	struct perf_data *data = &rec->data;
 	int fd, err;
+	char *new_filename;
 
 	/* Same Size:      "2015122520103046"*/
 	char timestamp[] = "InvalidTimestamp";
@@ -912,7 +916,7 @@ record__switch_output(struct record *rec, bool at_exit)
 
 	fd = perf_data__switch(data, timestamp,
 				    rec->session->header.data_offset,
-				    at_exit);
+				    at_exit, &new_filename);
 	if (fd >= 0 && !at_exit) {
 		rec->bytes_written = 0;
 		rec->session->header.data_size = 0;
@@ -922,6 +926,21 @@ record__switch_output(struct record *rec, bool at_exit)
 		fprintf(stderr, "[ perf record: Dump %s.%s ]\n",
 			data->path, timestamp);
 
+	if (rec->switch_output.num_files) {
+		int n = rec->switch_output.cur_file + 1;
+
+		if (n >= rec->switch_output.num_files)
+			n = 0;
+		rec->switch_output.cur_file = n;
+		if (rec->switch_output.filenames[n]) {
+			remove(rec->switch_output.filenames[n]);
+			free(rec->switch_output.filenames[n]);
+		}
+		rec->switch_output.filenames[n] = new_filename;
+	} else {
+		free(new_filename);
+	}
+
 	/* Output tracking events */
 	if (!at_exit) {
 		record__synthesize(rec, false);
@@ -1973,6 +1992,8 @@ static struct option __record_options[] = {
 			  &record.switch_output.set, "signal,size,time",
 			  "Switch output when receive SIGUSR2 or cross size,time threshold",
 			  "signal"),
+	OPT_INTEGER(0, "switch-max-files", &record.switch_output.num_files,
+		   "Limit number of switch output generated files"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "Parse options then exit"),
 #ifdef HAVE_AIO_SUPPORT
@@ -2059,6 +2080,13 @@ int cmd_record(int argc, const char **argv)
 		alarm(rec->switch_output.time);
 	}
 
+	if (rec->switch_output.num_files) {
+		rec->switch_output.filenames = calloc(sizeof(char *),
+						      rec->switch_output.num_files);
+		if (!rec->switch_output.filenames)
+			return -EINVAL;
+	}
+
 	/*
 	 * Allow aliases to facilitate the lookup of symbols for address
 	 * filters. Refer to auxtrace_parse_filters().
diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index c6b67efea11a..6a64f713710d 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -361,9 +361,9 @@ ssize_t perf_data__write(struct perf_data *data,
 
 int perf_data__switch(struct perf_data *data,
 			   const char *postfix,
-			   size_t pos, bool at_exit)
+			   size_t pos, bool at_exit,
+			   char **new_filepath)
 {
-	char *new_filepath;
 	int ret;
 
 	if (check_pipe(data))
@@ -371,15 +371,15 @@ int perf_data__switch(struct perf_data *data,
 	if (perf_data__is_read(data))
 		return -EINVAL;
 
-	if (asprintf(&new_filepath, "%s.%s", data->path, postfix) < 0)
+	if (asprintf(new_filepath, "%s.%s", data->path, postfix) < 0)
 		return -ENOMEM;
 
 	/*
 	 * Only fire a warning, don't return error, continue fill
 	 * original file.
 	 */
-	if (rename(data->path, new_filepath))
-		pr_warning("Failed to rename %s to %s\n", data->path, new_filepath);
+	if (rename(data->path, *new_filepath))
+		pr_warning("Failed to rename %s to %s\n", data->path, *new_filepath);
 
 	if (!at_exit) {
 		close(data->file.fd);
@@ -396,7 +396,6 @@ int perf_data__switch(struct perf_data *data,
 	}
 	ret = data->file.fd;
 out:
-	free(new_filepath);
 	return ret;
 }
 
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index 6aef8746469f..259868a39019 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -70,7 +70,7 @@ ssize_t perf_data_file__write(struct perf_data_file *file,
  */
 int perf_data__switch(struct perf_data *data,
 			   const char *postfix,
-			   size_t pos, bool at_exit);
+			   size_t pos, bool at_exit, char **new_filepath);
 
 int perf_data__create_dir(struct perf_data *data, int nr);
 int perf_data__open_dir(struct perf_data *data);
-- 
2.20.1

[PATCH v1 04/10] perf, tools, record: Clarify help for --switch-output

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

The help description for --switch-output looks like there
are multiple comma separated fields. But it's actually a choice
of different options. Make it clear and less confusing.

Before:

% perf report -h
...
        --switch-output[=<signal,size,time>]
                          Switch output when receive SIGUSR2 or cross size,time threshold

After:

% perf report -h
...

        --switch-output[=<signal or size[BKMG] or time[smhd]>]
                          Switch output when receiving SIGUSR2 (signal) or cross a size or time threshold

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/builtin-record.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 02d7c40b2d10..e7144a1c1c82 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1989,8 +1989,8 @@ static struct option __record_options[] = {
 	OPT_BOOLEAN(0, "timestamp-boundary", &record.timestamp_boundary,
 		    "Record timestamp boundary (time of first/last samples)"),
 	OPT_STRING_OPTARG_SET(0, "switch-output", &record.switch_output.str,
-			  &record.switch_output.set, "signal,size,time",
-			  "Switch output when receive SIGUSR2 or cross size,time threshold",
+			  &record.switch_output.set, "signal or size[BKMG] or time[smhd]",
+			  "Switch output when receiving SIGUSR2 (signal) or cross a size or time threshold",
 			  "signal"),
 	OPT_INTEGER(0, "switch-max-files", &record.switch_output.num_files,
 		   "Limit number of switch output generated files"),
-- 
2.20.1

Re: [PATCH v1 04/10] perf, tools, record: Clarify help for --switch-output

[Jiri Olsa]

On Mon, Mar 11, 2019 at 01:24:40PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The help description for --switch-output looks like there
> are multiple comma separated fields. But it's actually a choice
> of different options. Make it clear and less confusing.
> 
> Before:
> 
> % perf report -h

s/report/record/


> ...
>         --switch-output[=<signal,size,time>]
>                           Switch output when receive SIGUSR2 or cross size,time threshold
> 
> After:
> 
> % perf report -h

ditto

jirka

> ...
> 
>         --switch-output[=<signal or size[BKMG] or time[smhd]>]
>                           Switch output when receiving SIGUSR2 (signal) or cross a size or time threshold
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/builtin-record.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 02d7c40b2d10..e7144a1c1c82 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1989,8 +1989,8 @@ static struct option __record_options[] = {
>  	OPT_BOOLEAN(0, "timestamp-boundary", &record.timestamp_boundary,
>  		    "Record timestamp boundary (time of first/last samples)"),
>  	OPT_STRING_OPTARG_SET(0, "switch-output", &record.switch_output.str,
> -			  &record.switch_output.set, "signal,size,time",
> -			  "Switch output when receive SIGUSR2 or cross size,time threshold",
> +			  &record.switch_output.set, "signal or size[BKMG] or time[smhd]",
> +			  "Switch output when receiving SIGUSR2 (signal) or cross a size or time threshold",
>  			  "signal"),
>  	OPT_INTEGER(0, "switch-max-files", &record.switch_output.num_files,
>  		   "Limit number of switch output generated files"),
> -- 
> 2.20.1
> 

[PATCH v1 05/10] perf, report: Show all sort keys in help output

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Show all the supported sort keys in the command line help output,
so that it's not needed to refer to the manpage.

The output is not line wrapped, so it can be fairly long.

Before:

% perf report -h
...
     -s, --sort <key[,key2...]>
                          sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ... Please refer the man page for the complete list.

After:

% perf report -h
...
    -s, --sort <key[,key2...]>
                          sort by key(s): overhead overhead_sys overhead_us overhead_guest_sys overhead_guest_us overhead_children sample period pid comm dso symbol parent cpu ...

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/builtin-report.c |  5 ++---
 tools/perf/util/sort.c      | 41 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/sort.h      |  2 ++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 05c8dd41106c..03eb8f4a6c13 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1083,10 +1083,9 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN(0, "header-only", &report.header_only,
 		    "Show only data header."),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
-		   " Please refer the man page for the complete list."),
+		   sort_help("sort by key(s):")),
 	OPT_STRING('F', "fields", &field_order, "key[,keys...]",
-		   "output field(s): overhead, period, sample plus all of sort keys"),
+		   sort_help("output field(s): overhead, period, sample, ")),
 	OPT_BOOLEAN(0, "show-cpu-utilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_BOOLEAN_FLAG(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d2299e912e59..4e1531c6cdb9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -12,6 +12,7 @@
 #include "evsel.h"
 #include "evlist.h"
 #include "strlist.h"
+#include "strbuf.h"
 #include <traceevent/event-parse.h>
 #include "mem-events.h"
 #include "annotate.h"
@@ -3068,3 +3069,43 @@ void reset_output_field(void)
 	reset_dimensions();
 	perf_hpp__reset_output_field(&perf_hpp_list);
 }
+
+static void add_sort_string(struct strbuf *sb, struct sort_dimension *s, int n)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		strbuf_addch(sb, ' ');
+		strbuf_addstr(sb, s[i].name);
+	}
+}
+
+static void add_hpp_sort_string(struct strbuf *sb, struct hpp_dimension *s, int n)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		strbuf_addch(sb, ' ');
+		strbuf_addstr(sb, s[i].name);
+	}
+}
+
+const char *sort_help(const char *prefix)
+{
+	struct strbuf sb;
+	char *s;
+
+	strbuf_init(&sb, 300);
+	strbuf_addstr(&sb, prefix);
+	add_hpp_sort_string(&sb, hpp_sort_dimensions,
+			    ARRAY_SIZE(hpp_sort_dimensions));
+	add_sort_string(&sb, common_sort_dimensions,
+			    ARRAY_SIZE(common_sort_dimensions));
+	add_sort_string(&sb, bstack_sort_dimensions,
+			    ARRAY_SIZE(bstack_sort_dimensions));
+	add_sort_string(&sb, memory_sort_dimensions,
+			    ARRAY_SIZE(memory_sort_dimensions));
+	s = strbuf_detach(&sb, NULL);
+	strbuf_release(&sb);
+	return s;
+}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 2fbee0b1011c..93d1b25da341 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -286,6 +286,8 @@ void reset_output_field(void);
 void sort__setup_elide(FILE *fp);
 void perf_hpp__set_elide(int idx, bool elide);
 
+const char *sort_help(const char *prefix);
+
 int report_parse_ignore_callees_opt(const struct option *opt, const char *arg, int unset);
 
 bool is_strict_order(const char *order);
-- 
2.20.1

Re: [PATCH v1 05/10] perf, report: Show all sort keys in help output

[Jiri Olsa]

On Mon, Mar 11, 2019 at 01:24:41PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Show all the supported sort keys in the command line help output,
> so that it's not needed to refer to the manpage.
> 
> The output is not line wrapped, so it can be fairly long.
> 
> Before:
> 
> % perf report -h
> ...
>      -s, --sort <key[,key2...]>
>                           sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ... Please refer the man page for the complete list.
> 
> After:
> 
> % perf report -h
> ...
>     -s, --sort <key[,key2...]>
>                           sort by key(s): overhead overhead_sys overhead_us overhead_guest_sys overhead_guest_us overhead_children sample period pid comm dso symbol parent cpu ...

that line goes forever now, please provide some formating
into multiple lines

jirka

Re: [PATCH v1 05/10] perf, report: Show all sort keys in help output

[Andi Kleen]

> that line goes forever now, please provide some formating
> into multiple lines

Here's an updated patch with line wrapping.

---

perf report: Show all sort keys in help output

Show all the supported sort keys in the command line help output,
so that it's not needed to refer to the manpage.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---

v2: Now line wrap output.
---
 tools/perf/builtin-report.c |  5 ++--
 tools/perf/util/sort.c      | 52 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/sort.h      |  2 ++
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1532ebde6c4b..6317c38762dd 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1044,10 +1044,9 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN(0, "header-only", &report.header_only,
 		    "Show only data header."),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..."
-		   " Please refer the man page for the complete list."),
+		   sort_help("sort by key(s):")),
 	OPT_STRING('F', "fields", &field_order, "key[,keys...]",
-		   "output field(s): overhead, period, sample plus all of sort keys"),
+		   sort_help("output field(s): overhead, period, sample,")),
 	OPT_BOOLEAN(0, "show-cpu-utilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_BOOLEAN_FLAG(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d2299e912e59..ea71d3732a50 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -12,6 +12,7 @@
 #include "evsel.h"
 #include "evlist.h"
 #include "strlist.h"
+#include "strbuf.h"
 #include <traceevent/event-parse.h>
 #include "mem-events.h"
 #include "annotate.h"
@@ -3068,3 +3069,54 @@ void reset_output_field(void)
 	reset_dimensions();
 	perf_hpp__reset_output_field(&perf_hpp_list);
 }
+
+#define INDENT (3*8 + 1)
+
+static void add_key(struct strbuf *sb, const char *str, int *llen)
+{
+	if (*llen >= 75) {
+		strbuf_addstr(sb, "\n\t\t\t ");
+		*llen = INDENT;
+	}
+	strbuf_addf(sb, " %s", str);
+	*llen += strlen(str) + 1;
+}
+
+static void add_sort_string(struct strbuf *sb, struct sort_dimension *s, int n,
+			    int *llen)
+{
+	int i;
+
+	for (i = 0; i < n; i++)
+		add_key(sb, s[i].name, llen);
+}
+
+static void add_hpp_sort_string(struct strbuf *sb, struct hpp_dimension *s, int n,
+				int *llen)
+{
+	int i;
+
+	for (i = 0; i < n; i++)
+		add_key(sb, s[i].name, llen);
+}
+
+const char *sort_help(const char *prefix)
+{
+	struct strbuf sb;
+	char *s;
+	int len = strlen(prefix) + INDENT;
+
+	strbuf_init(&sb, 300);
+	strbuf_addstr(&sb, prefix);
+	add_hpp_sort_string(&sb, hpp_sort_dimensions,
+			    ARRAY_SIZE(hpp_sort_dimensions), &len);
+	add_sort_string(&sb, common_sort_dimensions,
+			    ARRAY_SIZE(common_sort_dimensions), &len);
+	add_sort_string(&sb, bstack_sort_dimensions,
+			    ARRAY_SIZE(bstack_sort_dimensions), &len);
+	add_sort_string(&sb, memory_sort_dimensions,
+			    ARRAY_SIZE(memory_sort_dimensions), &len);
+	s = strbuf_detach(&sb, NULL);
+	strbuf_release(&sb);
+	return s;
+}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 2fbee0b1011c..93d1b25da341 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -286,6 +286,8 @@ void reset_output_field(void);
 void sort__setup_elide(FILE *fp);
 void perf_hpp__set_elide(int idx, bool elide);
 
+const char *sort_help(const char *prefix);
+
 int report_parse_ignore_callees_opt(const struct option *opt, const char *arg, int unset);
 
 bool is_strict_order(const char *order);
-- 
2.20.1

[PATCH v1 06/10] perf, tools, report: Print better message for JITed code

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

The message about missing /tmp/perf-* for JITed code is quite confusing
to users. Add a better error message, but only print it once.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/map.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index fbeb0c6efaa6..d476b76abc6a 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -316,6 +316,15 @@ int map__load(struct map *map)
 
 	nr = dso__load(map->dso, map);
 	if (nr < 0) {
+		if (!strncmp(map->dso->name, "/tmp/perf-", 10)) {
+			static bool warned;
+			if (!warned) {
+				pr_err("Cannot find executable, JITed code present? May need agent.\n");
+				warned = true;
+			}
+			return -1;
+		}
+
 		if (map->dso->has_build_id) {
 			char sbuild_id[SBUILD_ID_SIZE];
 
-- 
2.20.1

Re: [PATCH v1 06/10] perf, tools, report: Print better message for JITed code

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 01:24:42PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The message about missing /tmp/perf-* for JITed code is quite confusing
> to users. Add a better error message, but only print it once.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/map.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index fbeb0c6efaa6..d476b76abc6a 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -316,6 +316,15 @@ int map__load(struct map *map)
>  
>  	nr = dso__load(map->dso, map);
>  	if (nr < 0) {
> +		if (!strncmp(map->dso->name, "/tmp/perf-", 10)) {
> +			static bool warned;
> +			if (!warned) {
> +				pr_err("Cannot find executable, JITed code present? May need agent.\n");
> +				warned = true;
> +			}

Please use WARN_ON_ONCE(), we have it in tools/include/asm/bug.h, just
like the kernel.

> +			return -1;
> +		}
> +
>  		if (map->dso->has_build_id) {
>  			char sbuild_id[SBUILD_ID_SIZE];
>  
> -- 
> 2.20.1

-- 

- Arnaldo

Re: [PATCH v1 06/10] perf, tools, report: Print better message for JITed code

[Andi Kleen]

On Mon, Mar 11, 2019 at 05:33:02PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 11, 2019 at 01:24:42PM -0700, Andi Kleen escreveu:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > The message about missing /tmp/perf-* for JITed code is quite confusing
> > to users. Add a better error message, but only print it once.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > ---
> >  tools/perf/util/map.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index fbeb0c6efaa6..d476b76abc6a 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -316,6 +316,15 @@ int map__load(struct map *map)
> >  
> >  	nr = dso__load(map->dso, map);
> >  	if (nr < 0) {
> > +		if (!strncmp(map->dso->name, "/tmp/perf-", 10)) {
> > +			static bool warned;
> > +			if (!warned) {
> > +				pr_err("Cannot find executable, JITed code present? May need agent.\n");
> > +				warned = true;
> > +			}
> 
> Please use WARN_ON_ONCE(), we have it in tools/include/asm/bug.h, just
> like the kernel.

But that prints "assertation failed". This is not a perf bug, but can happen
in normal operation.

-Andi

Re: [PATCH v1 06/10] perf, tools, report: Print better message for JITed code

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 01:48:56PM -0700, Andi Kleen escreveu:
> On Mon, Mar 11, 2019 at 05:33:02PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Mar 11, 2019 at 01:24:42PM -0700, Andi Kleen escreveu:
> > > +		if (!strncmp(map->dso->name, "/tmp/perf-", 10)) {
> > > +			static bool warned;
> > > +			if (!warned) {
> > > +				pr_err("Cannot find executable, JITed code present? May need agent.\n");
> > > +				warned = true;
> > > +			}
 
> > Please use WARN_ON_ONCE(), we have it in tools/include/asm/bug.h, just
> > like the kernel.

> But that prints "assertation failed". This is not a perf bug, but can happen
> in normal operation.

Sorry, we also have WARN_ONCE() and that one doesn't have any extra
message printed, only what you pass to it, right?

- Arnaldo

[PATCH v1 07/10] perf, tools, report: Indicate JITed code better in report

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Print [TID] tid %d instead of the crypted /tmp/perf-%d.map default.

% cat >loop.java
public class loop {
        public static void main(String[] args)
        {
                for (;;);
        }
}
^D
% javac loop.java
% perf record java loop
^C

Before:

% perf report --stdio
...
    56.09%  java     perf-34724.map      [.] 0x00007fd5bd021896
    19.12%  java     perf-34724.map      [.] 0x00007fd5bd021887
     9.79%  java     perf-34724.map      [.] 0x00007fd5bd021783
     8.97%  java     perf-34724.map      [.] 0x00007fd5bd02175b

After:

% perf report --stdio
...
    56.09%  java     [JIT] tid 34724     [.] 0x00007fd5bd021896
    19.12%  java     [JIT] tid 34724     [.] 0x00007fd5bd021887
     9.79%  java     [JIT] tid 34724     [.] 0x00007fd5bd021783
     8.97%  java     [JIT] tid 34724     [.] 0x00007fd5bd02175b

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/dso.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index ba58ba603b69..ab8a455d2283 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1141,28 +1141,34 @@ void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated)
 
 static void dso__set_basename(struct dso *dso)
 {
-       /*
-        * basename() may modify path buffer, so we must pass
-        * a copy.
-        */
-       char *base, *lname = strdup(dso->long_name);
+	char *base, *lname;
+	int tid;
 
-       if (!lname)
-               return;
-
-       /*
-        * basename() may return a pointer to internal
-        * storage which is reused in subsequent calls
-        * so copy the result.
-        */
-       base = strdup(basename(lname));
+	if (sscanf(dso->long_name, "/tmp/perf-%d.map", &tid) == 1) {
+		if (asprintf(&base, "[JIT] tid %d", tid) < 0)
+			return;
+	} else {
+	      /*
+	       * basename() may modify path buffer, so we must pass
+               * a copy.
+               */
+		lname = strdup(dso->long_name);
+		if (!lname)
+			return;
 
-       free(lname);
+		/*
+		 * basename() may return a pointer to internal
+		 * storage which is reused in subsequent calls
+		 * so copy the result.
+		 */
+		base = strdup(basename(lname));
 
-       if (!base)
-               return;
+		free(lname);
 
-       dso__set_short_name(dso, base, true);
+		if (!base)
+			return;
+	}
+	dso__set_short_name(dso, base, true);
 }
 
 int dso__name_len(const struct dso *dso)
-- 
2.20.1

[PATCH v1 08/10] perf, tools, script: Support relative time

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

When comparing time stamps in perf script traces it can be annoying
to work with the full perf time stamps. Add a --reltime option
that displays time stamps relative to the trace start to make
it easier to read the traces.

Note: not currently supported for --time. Report an error in this
case.

Before:

% perf script
         swapper     0 [000] 245402.891216:          1 cycles:ppp:  ffffffffa0068814 native_write_msr+0x4 ([kernel.kallsyms])
         swapper     0 [000] 245402.891223:          1 cycles:ppp:  ffffffffa0068814 native_write_msr+0x4 ([kernel.kallsyms])
         swapper     0 [000] 245402.891227:          5 cycles:ppp:  ffffffffa0068814 native_write_msr+0x4 ([kernel.kallsyms])
         swapper     0 [000] 245402.891231:         41 cycles:ppp:  ffffffffa0068816 native_write_msr+0x6 ([kernel.kallsyms])
         swapper     0 [000] 245402.891235:        355 cycles:ppp:  ffffffffa000dd51 intel_bts_enable_local+0x21 ([kernel.kallsyms])
         swapper     0 [000] 245402.891239:       3084 cycles:ppp:  ffffffffa0a0150a end_repeat_nmi+0x48 ([kernel.kallsyms])

After:

% perf script --reltime

         swapper     0 [000]     0.000000:          1 cycles:ppp:  ffffffffa0068814 native_write_msr+0x4 ([kernel.kallsyms])
         swapper     0 [000]     0.000006:          1 cycles:ppp:  ffffffffa0068814 native_write_msr+0x4 ([kernel.kallsyms])
         swapper     0 [000]     0.000010:          5 cycles:ppp:  ffffffffa0068814 native_write_msr+0x4 ([kernel.kallsyms])
         swapper     0 [000]     0.000014:         41 cycles:ppp:  ffffffffa0068816 native_write_msr+0x6 ([kernel.kallsyms])
         swapper     0 [000]     0.000018:        355 cycles:ppp:  ffffffffa000dd51 intel_bts_enable_local+0x21 ([kernel.kallsyms])
         swapper     0 [000]     0.000022:       3084 cycles:ppp:  ffffffffa0a0150a end_repeat_nmi+0x48 ([kernel.kallsyms])

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-script.txt |  3 +++
 tools/perf/builtin-script.c              | 18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 2e19fd7ffe35..9b0d04dd2a61 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -380,6 +380,9 @@ include::itrace.txt[]
 	Set the maximum number of program blocks to print with brstackasm for
 	each sample.
 
+--reltime::
+	Print time stamps relative to trace start.
+
 --per-event-dump::
 	Create per event files with a "perf.data.EVENT.dump" name instead of
         printing to stdout, useful, for instance, for generating flamegraphs.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 111787e83784..9664db79dc5a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -53,6 +53,8 @@
 
 static char const		*script_name;
 static char const		*generate_script_lang;
+static bool			reltime;
+static u64			initial_time;
 static bool			debug_mode;
 static u64			last_timestamp;
 static u64			nr_unordered;
@@ -686,7 +688,13 @@ static int perf_sample__fprintf_start(struct perf_sample *sample,
 	}
 
 	if (PRINT_FIELD(TIME)) {
-		nsecs = sample->time;
+		u64 time = sample->time;
+		if (reltime) {
+			if (!initial_time)
+				initial_time = sample->time;
+			time = sample->time - initial_time;
+		}
+		nsecs = time;
 		secs = nsecs / NSEC_PER_SEC;
 		nsecs -= secs * NSEC_PER_SEC;
 
@@ -694,7 +702,7 @@ static int perf_sample__fprintf_start(struct perf_sample *sample,
 			printed += fprintf(fp, "%5lu.%09llu: ", secs, nsecs);
 		else {
 			char sample_time[32];
-			timestamp__scnprintf_usec(sample->time, sample_time, sizeof(sample_time));
+			timestamp__scnprintf_usec(time, sample_time, sizeof(sample_time));
 			printed += fprintf(fp, "%12s: ", sample_time);
 		}
 	}
@@ -3386,6 +3394,7 @@ int cmd_script(int argc, const char **argv)
 		     "Set the maximum stack depth when parsing the callchain, "
 		     "anything beyond the specified depth will be ignored. "
 		     "Default: kernel.perf_event_max_stack or " __stringify(PERF_MAX_STACK_DEPTH)),
+	OPT_BOOLEAN(0, "reltime", &reltime, "Show time stamps relative to start"),
 	OPT_BOOLEAN('I', "show-info", &show_full_info,
 		    "display extended information from perf.data file"),
 	OPT_BOOLEAN('\0', "show-kernel-path", &symbol_conf.show_kernel_path,
@@ -3460,6 +3469,11 @@ int cmd_script(int argc, const char **argv)
 		}
 	}
 
+	if (script.time_str && reltime) {
+		fprintf(stderr, "Don't combine --reltime with --time\n");
+		return -1;
+	}
+
 	if (itrace_synth_opts.callchain &&
 	    itrace_synth_opts.callchain_sz > scripting_max_stack)
 		scripting_max_stack = itrace_synth_opts.callchain_sz;
-- 
2.20.1

[PATCH v1 09/10] perf, tools, stat: Fix --no-scale

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

The -c option to enable multiplex scaling has been useless for quite some
time because scaling is default. It's only useful as --no-scale to
disable scaling. But the non scaling code path has bitrotted
and doesn't print anything because perf output code
relies on value run/ena information.

Also even when we don't want to scale a value it's still useful
to show its multiplex percentage.

This patch:
- Fixes help and documentation to show --no-scale instead of -c
- Removes -c, only keeps the long option because -c doesn't
support negatives.
- Enables running/enabled even with --no-scale
- And fixes some other problems in the no-scale output.

Before:

$ perf stat --no-scale -e cycles true

 Performance counter stats for 'true':

     <not counted>      cycles

       0.000984154 seconds time elapsed

After:

$ ./perf stat --no-scale -e cycles true

 Performance counter stats for 'true':

           706,070      cycles

       0.001219821 seconds time elapsed

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-stat.txt |  5 ++---
 tools/perf/builtin-stat.c              |  3 ++-
 tools/perf/util/evsel.c                |  3 +--
 tools/perf/util/stat.c                 | 12 ++++--------
 4 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 4bc2085e5197..39c05f89104e 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -72,9 +72,8 @@ report::
 --all-cpus::
         system-wide collection from all CPUs (default if no target is specified)
 
--c::
---scale::
-	scale/normalize counter values
+--no-scale::
+	Don't scale/normalize counter values
 
 -d::
 --detailed::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7b8f09b0b8bf..49ee3c2033ec 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -718,7 +718,8 @@ static struct option stat_options[] = {
 		    "system-wide collection from all CPUs"),
 	OPT_BOOLEAN('g', "group", &group,
 		    "put the counters into a counter group"),
-	OPT_BOOLEAN('c', "scale", &stat_config.scale, "scale/normalize counters"),
+	OPT_BOOLEAN(0, "scale", &stat_config.scale,
+		    "Use --no-scale to disable counter scaling for multiplexing"),
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show counter open errors, etc)"),
 	OPT_INTEGER('r', "repeat", &stat_config.run_count,
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 686318f69b1d..60450087f198 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1344,8 +1344,7 @@ void perf_counts_values__scale(struct perf_counts_values *count,
 			scaled = 1;
 			count->val = (u64)((double) count->val * count->ena / count->run + 0.5);
 		}
-	} else
-		count->ena = count->run = 0;
+	}
 
 	if (pscaled)
 		*pscaled = scaled;
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index 4d40515307b8..2856cc9d5a31 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -291,10 +291,8 @@ process_counter_values(struct perf_stat_config *config, struct perf_evsel *evsel
 		break;
 	case AGGR_GLOBAL:
 		aggr->val += count->val;
-		if (config->scale) {
-			aggr->ena += count->ena;
-			aggr->run += count->run;
-		}
+		aggr->ena += count->ena;
+		aggr->run += count->run;
 	case AGGR_UNSET:
 	default:
 		break;
@@ -442,10 +440,8 @@ int create_perf_stat_counter(struct perf_evsel *evsel,
 	struct perf_event_attr *attr = &evsel->attr;
 	struct perf_evsel *leader = evsel->leader;
 
-	if (config->scale) {
-		attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
-				    PERF_FORMAT_TOTAL_TIME_RUNNING;
-	}
+	attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
+			    PERF_FORMAT_TOTAL_TIME_RUNNING;
 
 	/*
 	 * The event is part of non trivial group, let's enable
-- 
2.20.1

[PATCH v1 10/10] perf, tools, stat: Improve scaling

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

The multiplexing scaling in perf stat mysteriously adds 0.5 to the
value. This dates back to the original perf tool. Other scaling
code doesn't use that strange convention. Remove the extra 0.5.

Before:

$ perf stat -e 'cycles,cycles,cycles,cycles,cycles,cycles' grep -rq foo

 Performance counter stats for 'grep -rq foo':

         6,403,580      cycles                                                        (81.62%)
         6,404,341      cycles                                                        (81.64%)
         6,402,983      cycles                                                        (81.62%)
         6,399,941      cycles                                                        (81.63%)
         6,399,451      cycles                                                        (81.62%)
         6,436,105      cycles                                                        (91.87%)

       0.005843799 seconds time elapsed

       0.002905000 seconds user
       0.002902000 seconds sys

After:

$ ./perf stat -e 'cycles,cycles,cycles,cycles,cycles,cycles' grep -rq foo

 Performance counter stats for 'grep -rq foo':

         6,422,704      cycles                                                        (81.68%)
         6,401,842      cycles                                                        (81.68%)
         6,398,432      cycles                                                        (81.68%)
         6,397,098      cycles                                                        (81.68%)
         6,396,074      cycles                                                        (81.67%)
         6,434,980      cycles                                                        (91.62%)

       0.005884437 seconds time elapsed

       0.003580000 seconds user
       0.002356000 seconds sys

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 60450087f198..e425f342fe81 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1342,7 +1342,7 @@ void perf_counts_values__scale(struct perf_counts_values *count,
 			count->val = 0;
 		} else if (count->run < count->ena) {
 			scaled = 1;
-			count->val = (u64)((double) count->val * count->ena / count->run + 0.5);
+			count->val = (u64)((double) count->val * count->ena / count->run);
 		}
 	}
 
-- 
2.20.1

Re: Misc improvements and bug fixes for perf

[Jiri Olsa]

On Mon, Mar 11, 2019 at 01:24:36PM -0700, Andi Kleen wrote:
> Mostly unrelated to each other, so can be picked'n'chosed.
> 
> - Fix perf stat --no-scale
> - Support --reltime in perf script
> - Fix crashes with stat -r
> - Handle JITed code better in perf report
> - Allow to limit rotated perf.data files
> - Filter metrics in perf list
> - Show all sort keys in perf report help
> - Fix multiplexing formula
> 
> Also available in
> git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/misc-fixes-4
> 

apart from the 2 comments it all looks good to me

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka