Support sample context in perf report

Support sample context in perf report

[Andi Kleen]

[Changes: 
v6:
Rebase.
Drop already merged patches.
Address review comments: free samples and refactor perf script checking 
]

We currently have two ways to look at sample data in perf:
either use perf report to aggregate everything, or use
perf script to look at all individual samples.

Both ways are useful. Of course aggregation is useful
to quickly find the most expensive part of the code.

But sometimes a single sample is not good enough to
determine the problem and we need to look at context, either
through branch contexts, or other previous samples (e.g. for
correlating different micro architecture events or computing
metrics)

This can be done through perf script today, but it can
be rather cumbersome to find the right samples to look
at.

Another problem with perf report is that it aggregates
the whole measurement period. But many real workloads
have phases where they behave quite differently, and it is
often not useful to combine them into a single histogram.

While this can be worked around with the --time option
to report, it can be quite cumbersome.

This patch kit attempts to address some of these
problems in perf report by making it time aware.

- It adds a new time sort key that allows perf report
to separate samples from different regions. The time
regions can be defined with a new --time-quantum option.

- Then it extends the perf script support in the
tui record browser to allow browsing samples for 
different time regions from within a perf report
session.

- Extends the report browser script display
to automatically select sensible defaults
based on what was recorded. For example it will
automatically show branch contexts with -b.

- Support browsing the context of individual samples.
perf report can save a limited number of random samples
per histogram entry with the new --samples option.
Then the browser allows directly jumping to any
of the saved samples and browsing the context on the current
thread or CPU.

There could be probably be done more to make
perf report even better for such use cases (e.g. a real
time line display), but this basic support is good
enough for many useful usages.

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

[PATCH v6 01/11] perf tools script: Filter COMM/FORK/.. events by CPU

[Andi Kleen]

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

The --cpu option only filtered samples. Filter other perf events,
such as COMM, FORK, SWITCH by the CPU too.

Reported-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
v2: Only filter printf output
v3: Move checking to function
---
 tools/perf/builtin-script.c | 71 ++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 24 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 111787e83784..b695b20ffc8a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1933,6 +1933,13 @@ static int cleanup_scripting(void)
 	return scripting_ops ? scripting_ops->stop_script() : 0;
 }
 
+static bool filter_cpu(struct perf_sample *sample)
+{
+	if (cpu_list)
+		return !test_bit(sample->cpu, cpu_bitmap);
+	return false;
+}
+
 static int process_sample_event(struct perf_tool *tool,
 				union perf_event *event,
 				struct perf_sample *sample,
@@ -1967,7 +1974,7 @@ static int process_sample_event(struct perf_tool *tool,
 	if (al.filtered)
 		goto out_put;
 
-	if (cpu_list && !test_bit(sample->cpu, cpu_bitmap))
+	if (filter_cpu(sample))
 		goto out_put;
 
 	if (scripting_ops)
@@ -2052,9 +2059,11 @@ static int process_comm_event(struct perf_tool *tool,
 		sample->tid = event->comm.tid;
 		sample->pid = event->comm.pid;
 	}
-	perf_sample__fprintf_start(sample, thread, evsel,
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
 				   PERF_RECORD_COMM, stdout);
-	perf_event__fprintf(event, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	ret = 0;
 out:
 	thread__put(thread);
@@ -2088,9 +2097,11 @@ static int process_namespaces_event(struct perf_tool *tool,
 		sample->tid = event->namespaces.tid;
 		sample->pid = event->namespaces.pid;
 	}
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_NAMESPACES, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_NAMESPACES, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	ret = 0;
 out:
 	thread__put(thread);
@@ -2122,9 +2133,11 @@ static int process_fork_event(struct perf_tool *tool,
 		sample->tid = event->fork.tid;
 		sample->pid = event->fork.pid;
 	}
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_FORK, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_FORK, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	thread__put(thread);
 
 	return 0;
@@ -2152,9 +2165,11 @@ static int process_exit_event(struct perf_tool *tool,
 		sample->tid = event->fork.tid;
 		sample->pid = event->fork.pid;
 	}
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_EXIT, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_EXIT, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 
 	if (perf_event__process_exit(tool, event, sample, machine) < 0)
 		err = -1;
@@ -2188,9 +2203,11 @@ static int process_mmap_event(struct perf_tool *tool,
 		sample->tid = event->mmap.tid;
 		sample->pid = event->mmap.pid;
 	}
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_MMAP, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_MMAP, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	thread__put(thread);
 	return 0;
 }
@@ -2220,9 +2237,11 @@ static int process_mmap2_event(struct perf_tool *tool,
 		sample->tid = event->mmap2.tid;
 		sample->pid = event->mmap2.pid;
 	}
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_MMAP2, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_MMAP2, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	thread__put(thread);
 	return 0;
 }
@@ -2247,9 +2266,11 @@ static int process_switch_event(struct perf_tool *tool,
 		return -1;
 	}
 
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_SWITCH, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_SWITCH, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	thread__put(thread);
 	return 0;
 }
@@ -2270,9 +2291,11 @@ process_lost_event(struct perf_tool *tool,
 	if (thread == NULL)
 		return -1;
 
-	perf_sample__fprintf_start(sample, thread, evsel,
-				   PERF_RECORD_LOST, stdout);
-	perf_event__fprintf(event, stdout);
+	if (!filter_cpu(sample)) {
+		perf_sample__fprintf_start(sample, thread, evsel,
+					   PERF_RECORD_LOST, stdout);
+		perf_event__fprintf(event, stdout);
+	}
 	thread__put(thread);
 	return 0;
 }
-- 
2.20.1

Re: [PATCH v6 01/11] perf tools script: Filter COMM/FORK/.. events by CPU

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:44:52AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The --cpu option only filtered samples. Filter other perf events,
> such as COMM, FORK, SWITCH by the CPU too.
> 
> Reported-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Thanks, applied.
 
> ---
> v2: Only filter printf output
> v3: Move checking to function
> ---
>  tools/perf/builtin-script.c | 71 ++++++++++++++++++++++++-------------
>  1 file changed, 47 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 111787e83784..b695b20ffc8a 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1933,6 +1933,13 @@ static int cleanup_scripting(void)
>  	return scripting_ops ? scripting_ops->stop_script() : 0;
>  }
>  
> +static bool filter_cpu(struct perf_sample *sample)
> +{
> +	if (cpu_list)
> +		return !test_bit(sample->cpu, cpu_bitmap);
> +	return false;
> +}
> +
>  static int process_sample_event(struct perf_tool *tool,
>  				union perf_event *event,
>  				struct perf_sample *sample,
> @@ -1967,7 +1974,7 @@ static int process_sample_event(struct perf_tool *tool,
>  	if (al.filtered)
>  		goto out_put;
>  
> -	if (cpu_list && !test_bit(sample->cpu, cpu_bitmap))
> +	if (filter_cpu(sample))
>  		goto out_put;
>  
>  	if (scripting_ops)
> @@ -2052,9 +2059,11 @@ static int process_comm_event(struct perf_tool *tool,
>  		sample->tid = event->comm.tid;
>  		sample->pid = event->comm.pid;
>  	}
> -	perf_sample__fprintf_start(sample, thread, evsel,
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
>  				   PERF_RECORD_COMM, stdout);
> -	perf_event__fprintf(event, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	ret = 0;
>  out:
>  	thread__put(thread);
> @@ -2088,9 +2097,11 @@ static int process_namespaces_event(struct perf_tool *tool,
>  		sample->tid = event->namespaces.tid;
>  		sample->pid = event->namespaces.pid;
>  	}
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_NAMESPACES, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_NAMESPACES, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	ret = 0;
>  out:
>  	thread__put(thread);
> @@ -2122,9 +2133,11 @@ static int process_fork_event(struct perf_tool *tool,
>  		sample->tid = event->fork.tid;
>  		sample->pid = event->fork.pid;
>  	}
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_FORK, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_FORK, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	thread__put(thread);
>  
>  	return 0;
> @@ -2152,9 +2165,11 @@ static int process_exit_event(struct perf_tool *tool,
>  		sample->tid = event->fork.tid;
>  		sample->pid = event->fork.pid;
>  	}
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_EXIT, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_EXIT, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  
>  	if (perf_event__process_exit(tool, event, sample, machine) < 0)
>  		err = -1;
> @@ -2188,9 +2203,11 @@ static int process_mmap_event(struct perf_tool *tool,
>  		sample->tid = event->mmap.tid;
>  		sample->pid = event->mmap.pid;
>  	}
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_MMAP, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_MMAP, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	thread__put(thread);
>  	return 0;
>  }
> @@ -2220,9 +2237,11 @@ static int process_mmap2_event(struct perf_tool *tool,
>  		sample->tid = event->mmap2.tid;
>  		sample->pid = event->mmap2.pid;
>  	}
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_MMAP2, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_MMAP2, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	thread__put(thread);
>  	return 0;
>  }
> @@ -2247,9 +2266,11 @@ static int process_switch_event(struct perf_tool *tool,
>  		return -1;
>  	}
>  
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_SWITCH, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_SWITCH, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	thread__put(thread);
>  	return 0;
>  }
> @@ -2270,9 +2291,11 @@ process_lost_event(struct perf_tool *tool,
>  	if (thread == NULL)
>  		return -1;
>  
> -	perf_sample__fprintf_start(sample, thread, evsel,
> -				   PERF_RECORD_LOST, stdout);
> -	perf_event__fprintf(event, stdout);
> +	if (!filter_cpu(sample)) {
> +		perf_sample__fprintf_start(sample, thread, evsel,
> +					   PERF_RECORD_LOST, stdout);
> +		perf_event__fprintf(event, stdout);
> +	}
>  	thread__put(thread);
>  	return 0;
>  }
> -- 
> 2.20.1

-- 

- Arnaldo

[PATCH v6 02/11] perf tools report: Parse time quantum

[Andi Kleen]

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

Many workloads change over time. perf report currently aggregates
the whole time range reported in perf.data.

This patch adds an option for a time quantum to quantisize the
perf.data over time.

This just adds the option, will be used in follow on patches
for a time sort key.

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

---
v2:
Move time_quantum to symbol_conf. check for zero time quantum
v3:
Document s unit
v4:
Use _NSEC defines
---
 tools/perf/util/symbol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 6b73a0eeb6a1..967066d4d899 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -30,6 +30,7 @@
 #include <elf.h>
 #include <limits.h>
 #include <symbol/kallsyms.h>
+#include <linux/time64.h>
 #include <sys/utsname.h>
 
 static int dso__load_kernel_sym(struct dso *dso, struct map *map);
-- 
2.20.1

Re: [PATCH v6 02/11] perf tools report: Parse time quantum

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:44:53AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Many workloads change over time. perf report currently aggregates
> the whole time range reported in perf.data.
> 
> This patch adds an option for a time quantum to quantisize the
> perf.data over time.
> 
> This just adds the option, will be used in follow on patches
> for a time sort key.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
> v2:
> Move time_quantum to symbol_conf. check for zero time quantum
> v3:
> Document s unit
> v4:
> Use _NSEC defines

Humm? This just adds a header, but nevermind, I have the full patch in
my tree already :-)

- Arnaldo

> ---
>  tools/perf/util/symbol.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 6b73a0eeb6a1..967066d4d899 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -30,6 +30,7 @@
>  #include <elf.h>
>  #include <limits.h>
>  #include <symbol/kallsyms.h>
> +#include <linux/time64.h>
>  #include <sys/utsname.h>
>  
>  static int dso__load_kernel_sym(struct dso *dso, struct map *map);
> -- 
> 2.20.1

-- 

- Arnaldo

[PATCH v6 03/11] perf tools report: Support time sort key

[Andi Kleen]

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

Add a time sort key to perf report to display samples for
different time quantums separately. This allows easier
analysis of workloads that change over time, and also
will allow looking at the context of samples.

% perf record ...
% perf report --sort time,overhead,symbol --time-quantum 1ms --stdio
...
     0.67%  277061.87300  [.] _dl_start
     0.50%  277061.87300  [.] f1
     0.50%  277061.87300  [.] f2
     0.33%  277061.87300  [.] main
     0.29%  277061.87300  [.] _dl_lookup_symbol_x
     0.29%  277061.87300  [.] dl_main
     0.29%  277061.87300  [.] do_lookup_x
     0.17%  277061.87300  [.] _dl_debug_initialize
     0.17%  277061.87300  [.] _dl_init_paths
     0.08%  277061.87300  [.] check_match
     0.04%  277061.87300  [.] _dl_count_modids
     1.33%  277061.87400  [.] f1
     1.33%  277061.87400  [.] f2
     1.33%  277061.87400  [.] main
     1.17%  277061.87500  [.] main
     1.08%  277061.87500  [.] f1
     1.08%  277061.87500  [.] f2
     1.00%  277061.87600  [.] main
     0.83%  277061.87600  [.] f1
     0.83%  277061.87600  [.] f2
     1.00%  277061.87700  [.] main

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

---
v2:
Use symbol_conf.time_quantum
v3:
Use _PER_NSEC defines
Fix fallback path for zero quantum
---
 tools/perf/Documentation/perf-report.txt |  2 ++
 tools/perf/util/hist.c                   | 11 +++++++
 tools/perf/util/hist.h                   |  1 +
 tools/perf/util/sort.c                   | 39 ++++++++++++++++++++++++
 tools/perf/util/sort.h                   |  2 ++
 5 files changed, 55 insertions(+)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 9ec1702bccdd..546d87221ad8 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -105,6 +105,8 @@ OPTIONS
 	guest machine
 	- sample: Number of sample
 	- period: Raw number of event count of sample
+	- time: Separate the samples by time stamp with the resolution specified by
+	--time-quantum (default 100ms). Specify with overhead and before it.
 
 	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 f9eb95bf3938..fd5d02c1a521 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -19,6 +19,7 @@
 #include <math.h>
 #include <inttypes.h>
 #include <sys/param.h>
+#include <linux/time64.h>
 
 static bool hists__filter_entry_by_dso(struct hists *hists,
 				       struct hist_entry *he);
@@ -192,6 +193,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	hists__new_col_len(hists, HISTC_MEM_LVL, 21 + 3);
 	hists__new_col_len(hists, HISTC_LOCAL_WEIGHT, 12);
 	hists__new_col_len(hists, HISTC_GLOBAL_WEIGHT, 12);
+	hists__new_col_len(hists, HISTC_TIME, 12);
 
 	if (h->srcline) {
 		len = MAX(strlen(h->srcline), strlen(sort_srcline.se_header));
@@ -246,6 +248,14 @@ static void he_stat__add_cpumode_period(struct he_stat *he_stat,
 	}
 }
 
+static long hist_time(unsigned long time)
+{
+	unsigned long time_quantum = symbol_conf.time_quantum;
+	if (time_quantum)
+		return (time / time_quantum) * time_quantum;
+	return time;
+}
+
 static void he_stat__add_period(struct he_stat *he_stat, u64 period,
 				u64 weight)
 {
@@ -635,6 +645,7 @@ __hists__add_entry(struct hists *hists,
 		.raw_data = sample->raw_data,
 		.raw_size = sample->raw_size,
 		.ops = ops,
+		.time = hist_time(sample->time),
 	}, *he = hists__findnew_entry(hists, &entry, al, sample_self);
 
 	if (!hists->has_callchains && he && he->callchain_size != 0)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 4af27fbab24f..6279eca56409 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -31,6 +31,7 @@ enum hist_filter {
 
 enum hist_column {
 	HISTC_SYMBOL,
+	HISTC_TIME,
 	HISTC_DSO,
 	HISTC_THREAD,
 	HISTC_COMM,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index d2299e912e59..bdd30cab51cb 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -3,6 +3,7 @@
 #include <inttypes.h>
 #include <regex.h>
 #include <linux/mman.h>
+#include <linux/time64.h>
 #include "sort.h"
 #include "hist.h"
 #include "comm.h"
@@ -15,6 +16,7 @@
 #include <traceevent/event-parse.h>
 #include "mem-events.h"
 #include "annotate.h"
+#include "time-utils.h"
 #include <linux/kernel.h>
 
 regex_t		parent_regex;
@@ -654,6 +656,42 @@ struct sort_entry sort_socket = {
 	.se_width_idx	= HISTC_SOCKET,
 };
 
+/* --sort time */
+
+static int64_t
+sort__time_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	return right->time - left->time;
+}
+
+static int hist_entry__time_snprintf(struct hist_entry *he, char *bf,
+				    size_t size, unsigned int width)
+{
+	unsigned long secs;
+	unsigned long long nsecs;
+	char he_time[32];
+
+	nsecs = he->time;
+	secs = nsecs / NSEC_PER_SEC;
+	nsecs -= secs * NSEC_PER_SEC;
+
+	if (symbol_conf.nanosecs)
+		snprintf(he_time, sizeof he_time, "%5lu.%09llu: ",
+			 secs, nsecs);
+	else
+		timestamp__scnprintf_usec(he->time, he_time,
+					  sizeof(he_time));
+
+	return repsep_snprintf(bf, size, "%-.*s", width, he_time);
+}
+
+struct sort_entry sort_time = {
+	.se_header      = "Time",
+	.se_cmp	        = sort__time_cmp,
+	.se_snprintf    = hist_entry__time_snprintf,
+	.se_width_idx	= HISTC_TIME,
+};
+
 /* --sort trace */
 
 static char *get_trace_output(struct hist_entry *he)
@@ -1634,6 +1672,7 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_DSO_SIZE, "dso_size", sort_dso_size),
 	DIM(SORT_CGROUP_ID, "cgroup_id", sort_cgroup_id),
 	DIM(SORT_SYM_IPC_NULL, "ipc_null", sort_sym_ipc_null),
+	DIM(SORT_TIME, "time", sort_time),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 2fbee0b1011c..19dceb7f6145 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -135,6 +135,7 @@ struct hist_entry {
 	char			*srcfile;
 	struct symbol		*parent;
 	struct branch_info	*branch_info;
+	long			time;
 	struct hists		*hists;
 	struct mem_info		*mem_info;
 	void			*raw_data;
@@ -231,6 +232,7 @@ enum sort_type {
 	SORT_DSO_SIZE,
 	SORT_CGROUP_ID,
 	SORT_SYM_IPC_NULL,
+	SORT_TIME,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
2.20.1

Re: [PATCH v6 03/11] perf tools report: Support time sort key

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:44:54AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add a time sort key to perf report to display samples for
> different time quantums separately. This allows easier
> analysis of workloads that change over time, and also
> will allow looking at the context of samples.
> 
> % perf record ...
> % perf report --sort time,overhead,symbol --time-quantum 1ms --stdio
> ...
>      0.67%  277061.87300  [.] _dl_start
>      0.50%  277061.87300  [.] f1
>      0.50%  277061.87300  [.] f2
>      0.33%  277061.87300  [.] main
>      0.29%  277061.87300  [.] _dl_lookup_symbol_x
>      0.29%  277061.87300  [.] dl_main
>      0.29%  277061.87300  [.] do_lookup_x
>      0.17%  277061.87300  [.] _dl_debug_initialize
>      0.17%  277061.87300  [.] _dl_init_paths
>      0.08%  277061.87300  [.] check_match
>      0.04%  277061.87300  [.] _dl_count_modids
>      1.33%  277061.87400  [.] f1
>      1.33%  277061.87400  [.] f2
>      1.33%  277061.87400  [.] main
>      1.17%  277061.87500  [.] main
>      1.08%  277061.87500  [.] f1
>      1.08%  277061.87500  [.] f2
>      1.00%  277061.87600  [.] main
>      0.83%  277061.87600  [.] f1
>      0.83%  277061.87600  [.] f2
>      1.00%  277061.87700  [.] main

Thanks, applied.

[PATCH v6 04/11] perf tools report: Use less for scripts output

[Andi Kleen]

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

The UI viewer for scripts output has a lot of limitations: limited size,
no search or save function, slow, and various other issues.

Just use 'less' to display directly on the terminal instead.

This won't work in gtk mode, but gtk doesn't support these
context menus anyways. If that is ever done could use an terminal
for the output.

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

---

v2:
Remove some unneeded headers
---
 tools/perf/ui/browsers/scripts.c | 130 ++++---------------------------
 1 file changed, 17 insertions(+), 113 deletions(-)

diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 90a32ac69e76..7f36630694bf 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -1,35 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0
-#include <elf.h>
-#include <inttypes.h>
-#include <sys/ttydefaults.h>
-#include <string.h>
 #include "../../util/sort.h"
 #include "../../util/util.h"
 #include "../../util/hist.h"
 #include "../../util/debug.h"
 #include "../../util/symbol.h"
 #include "../browser.h"
-#include "../helpline.h"
 #include "../libslang.h"
 
-/* 2048 lines should be enough for a script output */
-#define MAX_LINES		2048
-
-/* 160 bytes for one output line */
-#define AVERAGE_LINE_LEN	160
-
-struct script_line {
-	struct list_head node;
-	char line[AVERAGE_LINE_LEN];
-};
-
-struct perf_script_browser {
-	struct ui_browser b;
-	struct list_head entries;
-	const char *script_name;
-	int nr_lines;
-};
-
 #define SCRIPT_NAMELEN	128
 #define SCRIPT_MAX_NO	64
 /*
@@ -73,69 +50,29 @@ static int list_scripts(char *script_name)
 	return ret;
 }
 
-static void script_browser__write(struct ui_browser *browser,
-				   void *entry, int row)
+static void run_script(char *cmd)
 {
-	struct script_line *sline = list_entry(entry, struct script_line, node);
-	bool current_entry = ui_browser__is_current_entry(browser, row);
-
-	ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
-						       HE_COLORSET_NORMAL);
-
-	ui_browser__write_nstring(browser, sline->line, browser->width);
+	pr_debug("Running %s\n", cmd);
+	SLang_reset_tty();
+	if (system(cmd) < 0)
+		pr_warning("Cannot run %s\n", cmd);
+	/*
+	 * SLang doesn't seem to reset the whole terminal, so be more
+	 * forceful to get back to the original state.
+	 */
+	printf("\033[c\033[H\033[J");
+	fflush(stdout);
+	SLang_init_tty(0, 0, 0);
+	SLsmg_refresh();
 }
 
-static int script_browser__run(struct perf_script_browser *browser)
-{
-	int key;
-
-	if (ui_browser__show(&browser->b, browser->script_name,
-			     "Press ESC to exit") < 0)
-		return -1;
-
-	while (1) {
-		key = ui_browser__run(&browser->b, 0);
-
-		/* We can add some special key handling here if needed */
-		break;
-	}
-
-	ui_browser__hide(&browser->b);
-	return key;
-}
-
-
 int script_browse(const char *script_opt)
 {
 	char cmd[SCRIPT_FULLPATH_LEN*2], script_name[SCRIPT_FULLPATH_LEN];
-	char *line = NULL;
-	size_t len = 0;
-	ssize_t retlen;
-	int ret = -1, nr_entries = 0;
-	FILE *fp;
-	void *buf;
-	struct script_line *sline;
-
-	struct perf_script_browser script = {
-		.b = {
-			.refresh    = ui_browser__list_head_refresh,
-			.seek	    = ui_browser__list_head_seek,
-			.write	    = script_browser__write,
-		},
-		.script_name = script_name,
-	};
-
-	INIT_LIST_HEAD(&script.entries);
-
-	/* Save each line of the output in one struct script_line object. */
-	buf = zalloc((sizeof(*sline)) * MAX_LINES);
-	if (!buf)
-		return -1;
-	sline = buf;
 
 	memset(script_name, 0, SCRIPT_FULLPATH_LEN);
 	if (list_scripts(script_name))
-		goto exit;
+		return -1;
 
 	sprintf(cmd, "perf script -s %s ", script_name);
 
@@ -147,42 +84,9 @@ int script_browse(const char *script_opt)
 		strcat(cmd, input_name);
 	}
 
-	strcat(cmd, " 2>&1");
-
-	fp = popen(cmd, "r");
-	if (!fp)
-		goto exit;
-
-	while ((retlen = getline(&line, &len, fp)) != -1) {
-		strncpy(sline->line, line, AVERAGE_LINE_LEN);
-
-		/* If one output line is very large, just cut it short */
-		if (retlen >= AVERAGE_LINE_LEN) {
-			sline->line[AVERAGE_LINE_LEN - 1] = '\0';
-			sline->line[AVERAGE_LINE_LEN - 2] = '\n';
-		}
-		list_add_tail(&sline->node, &script.entries);
-
-		if (script.b.width < retlen)
-			script.b.width = retlen;
-
-		if (nr_entries++ >= MAX_LINES - 1)
-			break;
-		sline++;
-	}
-
-	if (script.b.width > AVERAGE_LINE_LEN)
-		script.b.width = AVERAGE_LINE_LEN;
-
-	free(line);
-	pclose(fp);
+	strcat(cmd, " 2>&1 | less");
 
-	script.nr_lines = nr_entries;
-	script.b.nr_entries = nr_entries;
-	script.b.entries = &script.entries;
+	run_script(cmd);
 
-	ret = script_browser__run(&script);
-exit:
-	free(buf);
-	return ret;
+	return 0;
 }
-- 
2.20.1

Re: [PATCH v6 04/11] perf tools report: Use less for scripts output

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:44:55AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The UI viewer for scripts output has a lot of limitations: limited size,
> no search or save function, slow, and various other issues.
> 
> Just use 'less' to display directly on the terminal instead.
> 
> This won't work in gtk mode, but gtk doesn't support these
> context menus anyways. If that is ever done could use an terminal
> for the output.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Thanks. applied.

[PATCH v6 05/11] perf tools report: Support running scripts for current time range

[Andi Kleen]

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

When using the time sort key, add new context menus to run
scripts for only the currently selected time range. Compute
the correct range for the selection add pass it as the --time option to
perf script.

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

---
v2:
Use symbol_conf.time_quantum
v3:
Work around broken gcc fortify code warning
Use _NSEC defines
---
 tools/perf/ui/browsers/hists.c | 83 +++++++++++++++++++++++++++++-----
 1 file changed, 72 insertions(+), 11 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index aef800d97ea1..f98aeac607dd 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -7,6 +7,7 @@
 #include <string.h>
 #include <linux/rbtree.h>
 #include <sys/ttydefaults.h>
+#include <linux/time64.h>
 
 #include "../../util/callchain.h"
 #include "../../util/evsel.h"
@@ -30,6 +31,7 @@
 #include "srcline.h"
 #include "string2.h"
 #include "units.h"
+#include "time-utils.h"
 
 #include "sane_ctype.h"
 
@@ -2338,6 +2340,7 @@ static int switch_data_file(void)
 }
 
 struct popup_action {
+	unsigned long		time;
 	struct thread 		*thread;
 	struct map_symbol 	ms;
 	int			socket;
@@ -2527,36 +2530,64 @@ static int
 do_run_script(struct hist_browser *browser __maybe_unused,
 	      struct popup_action *act)
 {
-	char script_opt[64];
-	memset(script_opt, 0, sizeof(script_opt));
+	char *script_opt;
+	int len;
+	int n = 0;
+
+	len = 100;
+	if (act->thread)
+		len += strlen(thread__comm_str(act->thread));
+	else if (act->ms.sym)
+		len += strlen(act->ms.sym->name);
+	script_opt = malloc(len);
+	if (!script_opt)
+		return -1;
 
+	script_opt[0] = 0;
 	if (act->thread) {
-		scnprintf(script_opt, sizeof(script_opt), " -c %s ",
+		n = scnprintf(script_opt, len, " -c %s ",
 			  thread__comm_str(act->thread));
 	} else if (act->ms.sym) {
-		scnprintf(script_opt, sizeof(script_opt), " -S %s ",
+		n = scnprintf(script_opt, len, " -S %s ",
 			  act->ms.sym->name);
 	}
 
+	if (act->time) {
+		char start[32], end[32];
+		unsigned long starttime = act->time;
+		unsigned long endtime = act->time + symbol_conf.time_quantum;
+
+		if (starttime == endtime) { /* Display 1ms as fallback */
+			starttime -= 1*NSEC_PER_MSEC;
+			endtime += 1*NSEC_PER_MSEC;
+		}
+		timestamp__scnprintf_usec(starttime, start, sizeof start);
+		timestamp__scnprintf_usec(endtime, end, sizeof end);
+		n += snprintf(script_opt + n, len - n, " --time %s,%s", start, end);
+	}
+
 	script_browse(script_opt);
+	free(script_opt);
 	return 0;
 }
 
 static int
-add_script_opt(struct hist_browser *browser __maybe_unused,
+add_script_opt_2(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act, char **optstr,
-	       struct thread *thread, struct symbol *sym)
+	       struct thread *thread, struct symbol *sym,
+	       const char *tstr)
 {
+
 	if (thread) {
-		if (asprintf(optstr, "Run scripts for samples of thread [%s]",
-			     thread__comm_str(thread)) < 0)
+		if (asprintf(optstr, "Run scripts for samples of thread [%s]%s",
+			     thread__comm_str(thread), tstr) < 0)
 			return 0;
 	} else if (sym) {
-		if (asprintf(optstr, "Run scripts for samples of symbol [%s]",
-			     sym->name) < 0)
+		if (asprintf(optstr, "Run scripts for samples of symbol [%s]%s",
+			     sym->name, tstr) < 0)
 			return 0;
 	} else {
-		if (asprintf(optstr, "Run scripts for all samples") < 0)
+		if (asprintf(optstr, "Run scripts for all samples%s", tstr) < 0)
 			return 0;
 	}
 
@@ -2566,6 +2597,36 @@ add_script_opt(struct hist_browser *browser __maybe_unused,
 	return 1;
 }
 
+static int
+add_script_opt(struct hist_browser *browser,
+	       struct popup_action *act, char **optstr,
+	       struct thread *thread, struct symbol *sym)
+{
+	int n, j;
+	struct hist_entry *he;
+
+	n = add_script_opt_2(browser, act, optstr, thread, sym, "");
+
+	he = hist_browser__selected_entry(browser);
+	if (sort_order && strstr(sort_order, "time")) {
+		char tstr[128];
+
+		optstr++;
+		act++;
+		j = sprintf(tstr, " in ");
+		j += timestamp__scnprintf_usec(he->time, tstr + j,
+					       sizeof tstr - j);
+		j += sprintf(tstr + j, "-");
+		timestamp__scnprintf_usec(he->time + symbol_conf.time_quantum,
+				          tstr + j,
+				          sizeof tstr - j);
+		n += add_script_opt_2(browser, act, optstr, thread, sym,
+					  tstr);
+		act->time = he->time;
+	}
+	return n;
+}
+
 static int
 do_switch_data(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act __maybe_unused)
-- 
2.20.1

Re: [PATCH v6 05/11] perf tools report: Support running scripts for current time range

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:44:56AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> When using the time sort key, add new context menus to run
> scripts for only the currently selected time range. Compute
> the correct range for the selection add pass it as the --time option to
> perf script.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Thanks, applied.


- Arnaldo

[PATCH v6 06/11] perf tools report: Support builtin perf script in scripts menu

[Andi Kleen]

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

The scripts menu traditionally only showed custom perf scripts.

Allow to run standard perf script with useful default options too.

- Normal perf script
- perf script with assembler (needs xed installed)
- perf script with source code output (needs debuginfo)
- perf script with custom arguments

Then we automatically select the right options to
display the information in the perf.data file.

For example with -b display branch contexts.

It's not easily possible to check for xed's existence
in advance.  perf script usually gives sensible error messages when
it's not available.

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

---
v2:
Pass --inline if needed
v3:
Avoid compiler warnings. Allocate cmd buffer dynamically.
Change formatting
---
 tools/perf/ui/browsers/annotate.c |   2 +-
 tools/perf/ui/browsers/hists.c    |  23 +++---
 tools/perf/ui/browsers/scripts.c  | 127 +++++++++++++++++++++++-------
 tools/perf/util/hist.h            |   8 +-
 4 files changed, 120 insertions(+), 40 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 35bdfd8b1e71..98d934a36d86 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -750,7 +750,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 			continue;
 		case 'r':
 			{
-				script_browse(NULL);
+				script_browse(NULL, NULL);
 				continue;
 			}
 		case 'k':
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f98aeac607dd..fb4430f8982c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2344,6 +2344,7 @@ struct popup_action {
 	struct thread 		*thread;
 	struct map_symbol 	ms;
 	int			socket;
+	struct perf_evsel	*evsel;
 
 	int (*fn)(struct hist_browser *browser, struct popup_action *act);
 };
@@ -2566,7 +2567,7 @@ do_run_script(struct hist_browser *browser __maybe_unused,
 		n += snprintf(script_opt + n, len - n, " --time %s,%s", start, end);
 	}
 
-	script_browse(script_opt);
+	script_browse(script_opt, act->evsel);
 	free(script_opt);
 	return 0;
 }
@@ -2575,7 +2576,7 @@ static int
 add_script_opt_2(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act, char **optstr,
 	       struct thread *thread, struct symbol *sym,
-	       const char *tstr)
+	       struct perf_evsel *evsel, const char *tstr)
 {
 
 	if (thread) {
@@ -2593,6 +2594,7 @@ add_script_opt_2(struct hist_browser *browser __maybe_unused,
 
 	act->thread = thread;
 	act->ms.sym = sym;
+	act->evsel = evsel;
 	act->fn = do_run_script;
 	return 1;
 }
@@ -2600,12 +2602,13 @@ add_script_opt_2(struct hist_browser *browser __maybe_unused,
 static int
 add_script_opt(struct hist_browser *browser,
 	       struct popup_action *act, char **optstr,
-	       struct thread *thread, struct symbol *sym)
+	       struct thread *thread, struct symbol *sym,
+	       struct perf_evsel *evsel)
 {
 	int n, j;
 	struct hist_entry *he;
 
-	n = add_script_opt_2(browser, act, optstr, thread, sym, "");
+	n = add_script_opt_2(browser, act, optstr, thread, sym, evsel, "");
 
 	he = hist_browser__selected_entry(browser);
 	if (sort_order && strstr(sort_order, "time")) {
@@ -2618,10 +2621,9 @@ add_script_opt(struct hist_browser *browser,
 					       sizeof tstr - j);
 		j += sprintf(tstr + j, "-");
 		timestamp__scnprintf_usec(he->time + symbol_conf.time_quantum,
-				          tstr + j,
-				          sizeof tstr - j);
+				          tstr + j, sizeof tstr - j);
 		n += add_script_opt_2(browser, act, optstr, thread, sym,
-					  tstr);
+					  evsel, tstr);
 		act->time = he->time;
 	}
 	return n;
@@ -3092,7 +3094,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				nr_options += add_script_opt(browser,
 							     &actions[nr_options],
 							     &options[nr_options],
-							     thread, NULL);
+							     thread, NULL, evsel);
 			}
 			/*
 			 * Note that browser->selection != NULL
@@ -3107,11 +3109,12 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				nr_options += add_script_opt(browser,
 							     &actions[nr_options],
 							     &options[nr_options],
-							     NULL, browser->selection->sym);
+							     NULL, browser->selection->sym,
+							     evsel);
 			}
 		}
 		nr_options += add_script_opt(browser, &actions[nr_options],
-					     &options[nr_options], NULL, NULL);
+					     &options[nr_options], NULL, NULL, evsel);
 		nr_options += add_switch_opt(browser, &actions[nr_options],
 					     &options[nr_options]);
 skip_scripting:
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 7f36630694bf..9e5f87558af6 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -17,36 +17,111 @@
  */
 #define SCRIPT_FULLPATH_LEN	256
 
+struct script_config {
+	const char **names;
+	char **paths;
+	int index;
+	const char *perf;
+	char extra_format[256];
+};
+
+void attr_to_script(char *extra_format, struct perf_event_attr *attr)
+{
+	extra_format[0] = 0;
+	if (attr->read_format & PERF_FORMAT_GROUP)
+		strcat(extra_format, " -F +metric");
+	if (attr->sample_type & PERF_SAMPLE_BRANCH_STACK)
+		strcat(extra_format, " -F +brstackinsn --xed");
+	if (attr->sample_type & PERF_SAMPLE_REGS_INTR)
+		strcat(extra_format, " -F +iregs");
+	if (attr->sample_type & PERF_SAMPLE_REGS_USER)
+		strcat(extra_format, " -F +uregs");
+	if (attr->sample_type & PERF_SAMPLE_PHYS_ADDR)
+		strcat(extra_format, " -F +phys_addr");
+}
+
+static int add_script_option(const char *name, const char *opt,
+			     struct script_config *c)
+{
+	c->names[c->index] = name;
+	if (asprintf(&c->paths[c->index],
+		     "%s script %s -F +metric %s %s",
+		     c->perf, opt, symbol_conf.inline_name ? " --inline" : "",
+		     c->extra_format) < 0)
+		return -1;
+	c->index++;
+	return 0;
+}
+
 /*
  * When success, will copy the full path of the selected script
  * into  the buffer pointed by script_name, and return 0.
  * Return -1 on failure.
  */
-static int list_scripts(char *script_name)
+static int list_scripts(char *script_name, bool *custom,
+			struct perf_evsel *evsel)
 {
-	char *buf, *names[SCRIPT_MAX_NO], *paths[SCRIPT_MAX_NO];
-	int i, num, choice, ret = -1;
+	char *buf, *paths[SCRIPT_MAX_NO], *names[SCRIPT_MAX_NO];
+	int i, num, choice;
+	int ret = 0;
+	int max_std, custom_perf;
+	char pbuf[256];
+	const char *perf = perf_exe(pbuf, sizeof pbuf);
+	struct script_config scriptc = {
+		.names = (const char **)names,
+		.paths = paths,
+		.perf = perf
+	};
+
+	script_name[0] = 0;
 
 	/* Preset the script name to SCRIPT_NAMELEN */
 	buf = malloc(SCRIPT_MAX_NO * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN));
 	if (!buf)
-		return ret;
+		return -1;
+
+	if (evsel)
+		attr_to_script(scriptc.extra_format, &evsel->attr);
+	add_script_option("Show individual samples", "", &scriptc);
+	add_script_option("Show individual samples with assembler", "-F +insn --xed",
+			  &scriptc);
+	add_script_option("Show individual samples with source", "-F +srcline,+srccode",
+			  &scriptc);
+	custom_perf = scriptc.index;
+	add_script_option("Show samples with custom perf script arguments", "", &scriptc);
+	i = scriptc.index;
+	max_std = i;
 
-	for (i = 0; i < SCRIPT_MAX_NO; i++) {
-		names[i] = buf + i * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
+	for (; i < SCRIPT_MAX_NO; i++) {
+		names[i] = buf + (i - max_std) * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
 		paths[i] = names[i] + SCRIPT_NAMELEN;
 	}
 
-	num = find_scripts(names, paths);
-	if (num > 0) {
-		choice = ui__popup_menu(num, names);
-		if (choice < num && choice >= 0) {
-			strcpy(script_name, paths[choice]);
-			ret = 0;
-		}
+	num = find_scripts(names + max_std, paths + max_std);
+	if (num < 0)
+		num = 0;
+	choice = ui__popup_menu(num + max_std, (char * const *)names);
+	if (choice < 0) {
+		ret = -1;
+		goto out;
 	}
+	if (choice == custom_perf) {
+		char script_args[50];
+		int key = ui_browser__input_window("perf script command",
+				"Enter perf script command line (without perf script prefix)",
+				script_args, "", 0);
+		if (key != K_ENTER)
+			return -1;
+		sprintf(script_name, "%s script %s", perf, script_args);
+	} else if (choice < num + max_std) {
+		strcpy(script_name, paths[choice]);
+	}
+	*custom = choice >= max_std;
 
+out:
 	free(buf);
+	for (i = 0; i < max_std; i++)
+		free(paths[i]);
 	return ret;
 }
 
@@ -66,27 +141,25 @@ static void run_script(char *cmd)
 	SLsmg_refresh();
 }
 
-int script_browse(const char *script_opt)
+int script_browse(const char *script_opt, struct perf_evsel *evsel)
 {
-	char cmd[SCRIPT_FULLPATH_LEN*2], script_name[SCRIPT_FULLPATH_LEN];
+	char *cmd, script_name[SCRIPT_FULLPATH_LEN];
+	bool custom = false;
 
 	memset(script_name, 0, SCRIPT_FULLPATH_LEN);
-	if (list_scripts(script_name))
+	if (list_scripts(script_name, &custom, evsel))
 		return -1;
 
-	sprintf(cmd, "perf script -s %s ", script_name);
-
-	if (script_opt)
-		strcat(cmd, script_opt);
-
-	if (input_name) {
-		strcat(cmd, " -i ");
-		strcat(cmd, input_name);
-	}
-
-	strcat(cmd, " 2>&1 | less");
+	if (asprintf(&cmd, "%s%s %s %s%s 2>&1 | less",
+			custom ? "perf script -s " : "",
+			script_name,
+			script_opt ? script_opt : "",
+			input_name ? "-i " : "",
+			input_name ? input_name : "") < 0)
+		return -1;
 
 	run_script(cmd);
+	free(cmd);
 
 	return 0;
 }
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 6279eca56409..2113a6639cea 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -436,6 +436,8 @@ struct annotation_options;
 
 #ifdef HAVE_SLANG_SUPPORT
 #include "../ui/keysyms.h"
+void attr_to_script(char *buf, struct perf_event_attr *attr);
+
 int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt,
 			     struct annotation_options *annotation_opts);
@@ -450,7 +452,8 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct perf_env *env,
 				  bool warn_lost_event,
 				  struct annotation_options *annotation_options);
-int script_browse(const char *script_opt);
+
+int script_browse(const char *script_opt, struct perf_evsel *evsel);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
@@ -479,7 +482,8 @@ static inline int hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
 	return 0;
 }
 
-static inline int script_browse(const char *script_opt __maybe_unused)
+static inline int script_browse(const char *script_opt __maybe_unused,
+				struct perf_evsel *evsel __maybe_unused)
 {
 	return 0;
 }
-- 
2.20.1

Re: [PATCH v6 06/11] perf tools report: Support builtin perf script in scripts menu

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:44:57AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The scripts menu traditionally only showed custom perf scripts.
> 
> Allow to run standard perf script with useful default options too.

Thanks, applied.
 
> - Normal perf script
> - perf script with assembler (needs xed installed)
> - perf script with source code output (needs debuginfo)
> - perf script with custom arguments
> 
> Then we automatically select the right options to
> display the information in the perf.data file.
> 
> For example with -b display branch contexts.
> 
> It's not easily possible to check for xed's existence
> in advance.  perf script usually gives sensible error messages when
> it's not available.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
> v2:
> Pass --inline if needed
> v3:
> Avoid compiler warnings. Allocate cmd buffer dynamically.
> Change formatting
> ---
>  tools/perf/ui/browsers/annotate.c |   2 +-
>  tools/perf/ui/browsers/hists.c    |  23 +++---
>  tools/perf/ui/browsers/scripts.c  | 127 +++++++++++++++++++++++-------
>  tools/perf/util/hist.h            |   8 +-
>  4 files changed, 120 insertions(+), 40 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 35bdfd8b1e71..98d934a36d86 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -750,7 +750,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  			continue;
>  		case 'r':
>  			{
> -				script_browse(NULL);
> +				script_browse(NULL, NULL);
>  				continue;
>  			}
>  		case 'k':
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index f98aeac607dd..fb4430f8982c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2344,6 +2344,7 @@ struct popup_action {
>  	struct thread 		*thread;
>  	struct map_symbol 	ms;
>  	int			socket;
> +	struct perf_evsel	*evsel;
>  
>  	int (*fn)(struct hist_browser *browser, struct popup_action *act);
>  };
> @@ -2566,7 +2567,7 @@ do_run_script(struct hist_browser *browser __maybe_unused,
>  		n += snprintf(script_opt + n, len - n, " --time %s,%s", start, end);
>  	}
>  
> -	script_browse(script_opt);
> +	script_browse(script_opt, act->evsel);
>  	free(script_opt);
>  	return 0;
>  }
> @@ -2575,7 +2576,7 @@ static int
>  add_script_opt_2(struct hist_browser *browser __maybe_unused,
>  	       struct popup_action *act, char **optstr,
>  	       struct thread *thread, struct symbol *sym,
> -	       const char *tstr)
> +	       struct perf_evsel *evsel, const char *tstr)
>  {
>  
>  	if (thread) {
> @@ -2593,6 +2594,7 @@ add_script_opt_2(struct hist_browser *browser __maybe_unused,
>  
>  	act->thread = thread;
>  	act->ms.sym = sym;
> +	act->evsel = evsel;
>  	act->fn = do_run_script;
>  	return 1;
>  }
> @@ -2600,12 +2602,13 @@ add_script_opt_2(struct hist_browser *browser __maybe_unused,
>  static int
>  add_script_opt(struct hist_browser *browser,
>  	       struct popup_action *act, char **optstr,
> -	       struct thread *thread, struct symbol *sym)
> +	       struct thread *thread, struct symbol *sym,
> +	       struct perf_evsel *evsel)
>  {
>  	int n, j;
>  	struct hist_entry *he;
>  
> -	n = add_script_opt_2(browser, act, optstr, thread, sym, "");
> +	n = add_script_opt_2(browser, act, optstr, thread, sym, evsel, "");
>  
>  	he = hist_browser__selected_entry(browser);
>  	if (sort_order && strstr(sort_order, "time")) {
> @@ -2618,10 +2621,9 @@ add_script_opt(struct hist_browser *browser,
>  					       sizeof tstr - j);
>  		j += sprintf(tstr + j, "-");
>  		timestamp__scnprintf_usec(he->time + symbol_conf.time_quantum,
> -				          tstr + j,
> -				          sizeof tstr - j);
> +				          tstr + j, sizeof tstr - j);
>  		n += add_script_opt_2(browser, act, optstr, thread, sym,
> -					  tstr);
> +					  evsel, tstr);
>  		act->time = he->time;
>  	}
>  	return n;
> @@ -3092,7 +3094,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>  				nr_options += add_script_opt(browser,
>  							     &actions[nr_options],
>  							     &options[nr_options],
> -							     thread, NULL);
> +							     thread, NULL, evsel);
>  			}
>  			/*
>  			 * Note that browser->selection != NULL
> @@ -3107,11 +3109,12 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>  				nr_options += add_script_opt(browser,
>  							     &actions[nr_options],
>  							     &options[nr_options],
> -							     NULL, browser->selection->sym);
> +							     NULL, browser->selection->sym,
> +							     evsel);
>  			}
>  		}
>  		nr_options += add_script_opt(browser, &actions[nr_options],
> -					     &options[nr_options], NULL, NULL);
> +					     &options[nr_options], NULL, NULL, evsel);
>  		nr_options += add_switch_opt(browser, &actions[nr_options],
>  					     &options[nr_options]);
>  skip_scripting:
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 7f36630694bf..9e5f87558af6 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -17,36 +17,111 @@
>   */
>  #define SCRIPT_FULLPATH_LEN	256
>  
> +struct script_config {
> +	const char **names;
> +	char **paths;
> +	int index;
> +	const char *perf;
> +	char extra_format[256];
> +};
> +
> +void attr_to_script(char *extra_format, struct perf_event_attr *attr)
> +{
> +	extra_format[0] = 0;
> +	if (attr->read_format & PERF_FORMAT_GROUP)
> +		strcat(extra_format, " -F +metric");
> +	if (attr->sample_type & PERF_SAMPLE_BRANCH_STACK)
> +		strcat(extra_format, " -F +brstackinsn --xed");
> +	if (attr->sample_type & PERF_SAMPLE_REGS_INTR)
> +		strcat(extra_format, " -F +iregs");
> +	if (attr->sample_type & PERF_SAMPLE_REGS_USER)
> +		strcat(extra_format, " -F +uregs");
> +	if (attr->sample_type & PERF_SAMPLE_PHYS_ADDR)
> +		strcat(extra_format, " -F +phys_addr");
> +}
> +
> +static int add_script_option(const char *name, const char *opt,
> +			     struct script_config *c)
> +{
> +	c->names[c->index] = name;
> +	if (asprintf(&c->paths[c->index],
> +		     "%s script %s -F +metric %s %s",
> +		     c->perf, opt, symbol_conf.inline_name ? " --inline" : "",
> +		     c->extra_format) < 0)
> +		return -1;
> +	c->index++;
> +	return 0;
> +}
> +
>  /*
>   * When success, will copy the full path of the selected script
>   * into  the buffer pointed by script_name, and return 0.
>   * Return -1 on failure.
>   */
> -static int list_scripts(char *script_name)
> +static int list_scripts(char *script_name, bool *custom,
> +			struct perf_evsel *evsel)
>  {
> -	char *buf, *names[SCRIPT_MAX_NO], *paths[SCRIPT_MAX_NO];
> -	int i, num, choice, ret = -1;
> +	char *buf, *paths[SCRIPT_MAX_NO], *names[SCRIPT_MAX_NO];
> +	int i, num, choice;
> +	int ret = 0;
> +	int max_std, custom_perf;
> +	char pbuf[256];
> +	const char *perf = perf_exe(pbuf, sizeof pbuf);
> +	struct script_config scriptc = {
> +		.names = (const char **)names,
> +		.paths = paths,
> +		.perf = perf
> +	};
> +
> +	script_name[0] = 0;
>  
>  	/* Preset the script name to SCRIPT_NAMELEN */
>  	buf = malloc(SCRIPT_MAX_NO * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN));
>  	if (!buf)
> -		return ret;
> +		return -1;
> +
> +	if (evsel)
> +		attr_to_script(scriptc.extra_format, &evsel->attr);
> +	add_script_option("Show individual samples", "", &scriptc);
> +	add_script_option("Show individual samples with assembler", "-F +insn --xed",
> +			  &scriptc);
> +	add_script_option("Show individual samples with source", "-F +srcline,+srccode",
> +			  &scriptc);
> +	custom_perf = scriptc.index;
> +	add_script_option("Show samples with custom perf script arguments", "", &scriptc);
> +	i = scriptc.index;
> +	max_std = i;
>  
> -	for (i = 0; i < SCRIPT_MAX_NO; i++) {
> -		names[i] = buf + i * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
> +	for (; i < SCRIPT_MAX_NO; i++) {
> +		names[i] = buf + (i - max_std) * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
>  		paths[i] = names[i] + SCRIPT_NAMELEN;
>  	}
>  
> -	num = find_scripts(names, paths);
> -	if (num > 0) {
> -		choice = ui__popup_menu(num, names);
> -		if (choice < num && choice >= 0) {
> -			strcpy(script_name, paths[choice]);
> -			ret = 0;
> -		}
> +	num = find_scripts(names + max_std, paths + max_std);
> +	if (num < 0)
> +		num = 0;
> +	choice = ui__popup_menu(num + max_std, (char * const *)names);
> +	if (choice < 0) {
> +		ret = -1;
> +		goto out;
>  	}
> +	if (choice == custom_perf) {
> +		char script_args[50];
> +		int key = ui_browser__input_window("perf script command",
> +				"Enter perf script command line (without perf script prefix)",
> +				script_args, "", 0);
> +		if (key != K_ENTER)
> +			return -1;
> +		sprintf(script_name, "%s script %s", perf, script_args);
> +	} else if (choice < num + max_std) {
> +		strcpy(script_name, paths[choice]);
> +	}
> +	*custom = choice >= max_std;
>  
> +out:
>  	free(buf);
> +	for (i = 0; i < max_std; i++)
> +		free(paths[i]);
>  	return ret;
>  }
>  
> @@ -66,27 +141,25 @@ static void run_script(char *cmd)
>  	SLsmg_refresh();
>  }
>  
> -int script_browse(const char *script_opt)
> +int script_browse(const char *script_opt, struct perf_evsel *evsel)
>  {
> -	char cmd[SCRIPT_FULLPATH_LEN*2], script_name[SCRIPT_FULLPATH_LEN];
> +	char *cmd, script_name[SCRIPT_FULLPATH_LEN];
> +	bool custom = false;
>  
>  	memset(script_name, 0, SCRIPT_FULLPATH_LEN);
> -	if (list_scripts(script_name))
> +	if (list_scripts(script_name, &custom, evsel))
>  		return -1;
>  
> -	sprintf(cmd, "perf script -s %s ", script_name);
> -
> -	if (script_opt)
> -		strcat(cmd, script_opt);
> -
> -	if (input_name) {
> -		strcat(cmd, " -i ");
> -		strcat(cmd, input_name);
> -	}
> -
> -	strcat(cmd, " 2>&1 | less");
> +	if (asprintf(&cmd, "%s%s %s %s%s 2>&1 | less",
> +			custom ? "perf script -s " : "",
> +			script_name,
> +			script_opt ? script_opt : "",
> +			input_name ? "-i " : "",
> +			input_name ? input_name : "") < 0)
> +		return -1;
>  
>  	run_script(cmd);
> +	free(cmd);
>  
>  	return 0;
>  }
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 6279eca56409..2113a6639cea 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -436,6 +436,8 @@ struct annotation_options;
>  
>  #ifdef HAVE_SLANG_SUPPORT
>  #include "../ui/keysyms.h"
> +void attr_to_script(char *buf, struct perf_event_attr *attr);
> +
>  int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>  			     struct hist_browser_timer *hbt,
>  			     struct annotation_options *annotation_opts);
> @@ -450,7 +452,8 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
>  				  struct perf_env *env,
>  				  bool warn_lost_event,
>  				  struct annotation_options *annotation_options);
> -int script_browse(const char *script_opt);
> +
> +int script_browse(const char *script_opt, struct perf_evsel *evsel);
>  #else
>  static inline
>  int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
> @@ -479,7 +482,8 @@ static inline int hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
>  	return 0;
>  }
>  
> -static inline int script_browse(const char *script_opt __maybe_unused)
> +static inline int script_browse(const char *script_opt __maybe_unused,
> +				struct perf_evsel *evsel __maybe_unused)
>  {
>  	return 0;
>  }
> -- 
> 2.20.1

-- 

- Arnaldo

[PATCH v6 07/11] perf tools report: Implement browsing of individual samples

[Andi Kleen]

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

Now report can show whole time periods with perf script,
but the user still has to find individual samples of interest
manually.

It would be expensive and complicated to search for the
right samples in the whole perf file. Typically users
only need to look at a small number of samples for useful
analysis.

Also the full scripts tend to show samples of all CPUs and all
threads mixed up, which can be very confusing on larger systems.

Add a new --samples option to save a small random number of samples
per hist entry

Use a reservoir sample technique to select a representatve
number of samples.

Then allow browsing the samples using perf script
as part of the hist entry context menu. This automatically
adds the right filters, so only the thread or cpu of the sample
is displayed. Then we use less' search functionality
to directly jump the to the time stamp of the selected
sample.

It uses different menus for assembler and source display.
Assembler needs xed installed and source needs debuginfo.

Currently it only supports as many samples as fit on
the screen due to some limitations in the slang ui code.

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

---
v2:
Free names on error path
Pass --inline and --show-*-event to child perf as needed.
v3:
Don't use num_res to keep track of total samples
Don't pass -1 cpus to perf script.
Don't pass extra event option with tid filtering.
Fix sampling bug.
v4:
Add perfconfig option for context
Use _PER_NSEC defines
v5:
Rebase
Free res_samples
---
 tools/perf/Documentation/perf-config.txt |  6 ++
 tools/perf/Documentation/perf-report.txt |  4 +
 tools/perf/builtin-report.c              |  2 +
 tools/perf/ui/browsers/Build             |  1 +
 tools/perf/ui/browsers/hists.c           | 47 ++++++++++++
 tools/perf/ui/browsers/res_sample.c      | 95 ++++++++++++++++++++++++
 tools/perf/ui/browsers/scripts.c         |  2 +-
 tools/perf/util/hist.c                   | 36 +++++++++
 tools/perf/util/hist.h                   | 22 ++++++
 tools/perf/util/sort.h                   |  8 ++
 tools/perf/util/symbol.c                 |  1 +
 tools/perf/util/symbol_conf.h            |  1 +
 12 files changed, 224 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/ui/browsers/res_sample.c

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 86f3dcc15f83..2d0fb7613134 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -584,6 +584,12 @@ llvm.*::
 	llvm.opts::
 		Options passed to llc.
 
+samples.*::
+
+	samples.context::
+		Define how many ns worth of time to show
+		around samples in perf report sample context browser.
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 546d87221ad8..f441baa794ce 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -461,6 +461,10 @@ include::itrace.txt[]
 --socket-filter::
 	Only report the samples on the processor socket that match with this filter
 
+--samples=N::
+	Save N individual samples for each histogram entry to show context in perf
+	report tui browser.
+
 --raw-trace::
 	When displaying traceevent output, do not use print fmt or plugins.
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 05c8dd41106c..1921aaa9cece 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1159,6 +1159,8 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
 		    "Enable kernel symbol demangling"),
 	OPT_BOOLEAN(0, "mem-mode", &report.mem_mode, "mem access profile"),
+	OPT_INTEGER(0, "samples", &symbol_conf.res_sample,
+		    "Number of samples to save per histogram entry for individual browsing"),
 	OPT_CALLBACK(0, "percent-limit", &report, "percent",
 		     "Don't show entries under that percent", parse_percent_limit),
 	OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
diff --git a/tools/perf/ui/browsers/Build b/tools/perf/ui/browsers/Build
index 8fee56b46502..fdf86f7981ca 100644
--- a/tools/perf/ui/browsers/Build
+++ b/tools/perf/ui/browsers/Build
@@ -3,6 +3,7 @@ perf-y += hists.o
 perf-y += map.o
 perf-y += scripts.o
 perf-y += header.o
+perf-y += res_sample.o
 
 CFLAGS_annotate.o += -DENABLE_SLFUTURE_CONST
 CFLAGS_hists.o    += -DENABLE_SLFUTURE_CONST
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fb4430f8982c..3421ecbdd3f0 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1226,6 +1226,8 @@ void hist_browser__init_hpp(void)
 				hist_browser__hpp_color_overhead_guest_us;
 	perf_hpp__format[PERF_HPP__OVERHEAD_ACC].color =
 				hist_browser__hpp_color_overhead_acc;
+
+	res_sample_init();
 }
 
 static int hist_browser__show_entry(struct hist_browser *browser,
@@ -2345,6 +2347,7 @@ struct popup_action {
 	struct map_symbol 	ms;
 	int			socket;
 	struct perf_evsel	*evsel;
+	enum rstype		rstype;
 
 	int (*fn)(struct hist_browser *browser, struct popup_action *act);
 };
@@ -2572,6 +2575,17 @@ do_run_script(struct hist_browser *browser __maybe_unused,
 	return 0;
 }
 
+static int
+do_res_sample_script(struct hist_browser *browser __maybe_unused,
+		     struct popup_action *act)
+{
+	struct hist_entry *he;
+
+	he = hist_browser__selected_entry(browser);
+	res_sample_browse(he->res_samples, he->num_res, act->evsel, act->rstype);
+	return 0;
+}
+
 static int
 add_script_opt_2(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act, char **optstr,
@@ -2629,6 +2643,27 @@ add_script_opt(struct hist_browser *browser,
 	return n;
 }
 
+static int
+add_res_sample_opt(struct hist_browser *browser __maybe_unused,
+		   struct popup_action *act, char **optstr,
+		   struct res_sample *res_sample,
+		   struct perf_evsel *evsel,
+		   enum rstype type)
+{
+	if (!res_sample)
+		return 0;
+
+	if (asprintf(optstr, "Show context for individual samples %s",
+		type == A_ASM ? "with assembler" :
+		type == A_SOURCE ? "with source" : "") < 0)
+		return 0;
+
+	act->fn = do_res_sample_script;
+	act->evsel = evsel;
+	act->rstype = type;
+	return 1;
+}
+
 static int
 do_switch_data(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act __maybe_unused)
@@ -3115,6 +3150,18 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		}
 		nr_options += add_script_opt(browser, &actions[nr_options],
 					     &options[nr_options], NULL, NULL, evsel);
+		nr_options += add_res_sample_opt(browser, &actions[nr_options],
+						 &options[nr_options],
+				 hist_browser__selected_entry(browser)->res_samples,
+				 evsel, A_NORMAL);
+		nr_options += add_res_sample_opt(browser, &actions[nr_options],
+						 &options[nr_options],
+				 hist_browser__selected_entry(browser)->res_samples,
+				 evsel, A_ASM);
+		nr_options += add_res_sample_opt(browser, &actions[nr_options],
+						 &options[nr_options],
+				 hist_browser__selected_entry(browser)->res_samples,
+				 evsel, A_SOURCE);
 		nr_options += add_switch_opt(browser, &actions[nr_options],
 					     &options[nr_options]);
 skip_scripting:
diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
new file mode 100644
index 000000000000..c2a8536904bc
--- /dev/null
+++ b/tools/perf/ui/browsers/res_sample.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Display a menu with individual samples to browse with perf script */
+#include "util.h"
+#include "hist.h"
+#include "evsel.h"
+#include "hists.h"
+#include "sort.h"
+#include "config.h"
+#include "time-utils.h"
+#include <linux/time64.h>
+
+static u64 context_len = 10*NSEC_PER_MSEC;
+
+static int res_sample_config(const char *var, const char *value, void *data
+			     __maybe_unused)
+{
+	if (!strcmp(var, "samples.context"))
+		return perf_config_u64(&context_len, var, value);
+	return 0;
+}
+
+void res_sample_init(void)
+{
+	perf_config(res_sample_config, NULL);
+}
+
+int res_sample_browse(struct res_sample *res_samples, int num_res,
+		      struct perf_evsel *evsel, enum rstype rstype)
+{
+	char **names;
+	int i, n;
+	int choice;
+	char *cmd;
+	char pbuf[256], tidbuf[32], cpubuf[32];
+	const char *perf = perf_exe(pbuf, sizeof pbuf);
+	char trange[128], tsample[64];
+	struct res_sample *r;
+	char extra_format[256];
+
+	/* For now since ui__popup_menu doesn't like lists that don't fit */
+	num_res = max(min(SLtt_Screen_Rows - 4, num_res), 0);
+
+	names = calloc(num_res, sizeof(char *));
+	if (!names)
+		return -1;
+	for (i = 0; i < num_res; i++) {
+		char tbuf[64];
+
+		timestamp__scnprintf_nsec(res_samples[i].time, tbuf, sizeof tbuf);
+		if (asprintf(&names[i], "%s: CPU %d tid %d", tbuf,
+			     res_samples[i].cpu, res_samples[i].tid) < 0) {
+			while (--i >= 0)
+				free(names[i]);
+			free(names);
+			return -1;
+		}
+	}
+	choice = ui__popup_menu(num_res, names);
+	for (i = 0; i < num_res; i++)
+		free(names[i]);
+	free(names);
+
+	if (choice < 0 || choice >= num_res)
+		return -1;
+	r = &res_samples[choice];
+
+	n = timestamp__scnprintf_nsec(r->time - context_len, trange, sizeof trange);
+	trange[n++] = ',';
+	timestamp__scnprintf_nsec(r->time + context_len, trange + n, sizeof trange - n);
+
+	timestamp__scnprintf_nsec(r->time, tsample, sizeof tsample);
+
+	attr_to_script(extra_format, &evsel->attr);
+
+	if (asprintf(&cmd, "%s script %s%s --time %s %s%s %s%s --ns %s %s %s %s %s | less +/%s",
+		     perf,
+		     input_name ? "-i " : "",
+		     input_name ? input_name : "",
+		     trange,
+		     r->cpu >= 0 ? "--cpu " : "",
+		     r->cpu >= 0 ? (sprintf(cpubuf, "%d", r->cpu), cpubuf) : "",
+		     r->tid ? "--tid " : "",
+		     r->tid ? (sprintf(tidbuf, "%d", r->tid), tidbuf) : "",
+		     extra_format,
+		     rstype == A_ASM ? "-F +insn --xed" :
+		     rstype == A_SOURCE ? "-F +srcline,+srccode" : "",
+		     symbol_conf.inline_name ? "--inline" : "",
+		     "--show-lost-events ",
+		     r->tid ? "--show-switch-events --show-task-events " : "",
+		     tsample) < 0)
+		return -1;
+	run_script(cmd);
+	free(cmd);
+	return 0;
+}
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 9e5f87558af6..cdba58447b85 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -125,7 +125,7 @@ static int list_scripts(char *script_name, bool *custom,
 	return ret;
 }
 
-static void run_script(char *cmd)
+void run_script(char *cmd)
 {
 	pr_debug("Running %s\n", cmd);
 	SLang_reset_tty();
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fd5d02c1a521..3a0e2bb40529 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -436,6 +436,13 @@ static int hist_entry__init(struct hist_entry *he,
 			goto err_rawdata;
 	}
 
+	if (symbol_conf.res_sample) {
+		he->res_samples = calloc(sizeof(struct res_sample),
+					symbol_conf.res_sample);
+		if (!he->res_samples)
+			return -ENOMEM;
+	}
+
 	INIT_LIST_HEAD(&he->pairs.node);
 	thread__get(he->thread);
 	he->hroot_in  = RB_ROOT_CACHED;
@@ -603,6 +610,32 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 	return he;
 }
 
+static unsigned random_max(unsigned high)
+{
+	unsigned thresh = -high % high;
+	for (;;) {
+		unsigned r = random();
+		if (r >= thresh)
+			return r % high;
+	}
+}
+
+static void hists__res_sample(struct hist_entry *he, struct perf_sample *sample)
+{
+	struct res_sample *r;
+	int j;
+
+	if (he->num_res < symbol_conf.res_sample) {
+		j = he->num_res++;
+	} else {
+		j = random_max(symbol_conf.res_sample);
+	}
+	r = &he->res_samples[j];
+	r->time = sample->time;
+	r->cpu = sample->cpu;
+	r->tid = sample->tid;
+}
+
 static struct hist_entry*
 __hists__add_entry(struct hists *hists,
 		   struct addr_location *al,
@@ -650,6 +683,8 @@ __hists__add_entry(struct hists *hists,
 
 	if (!hists->has_callchains && he && he->callchain_size != 0)
 		hists->has_callchains = true;
+	if (he && symbol_conf.res_sample)
+		hists__res_sample(he, sample);
 	return he;
 }
 
@@ -1173,6 +1208,7 @@ void hist_entry__delete(struct hist_entry *he)
 		mem_info__zput(he->mem_info);
 	}
 
+	zfree(&he->res_samples);
 	zfree(&he->stat_acc);
 	free_srcline(he->srcline);
 	if (he->srcfile && he->srcfile[0])
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2113a6639cea..76ff6c6d03b8 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -433,6 +433,13 @@ struct hist_browser_timer {
 };
 
 struct annotation_options;
+struct res_sample;
+
+enum rstype {
+	A_NORMAL,
+	A_ASM,
+	A_SOURCE
+};
 
 #ifdef HAVE_SLANG_SUPPORT
 #include "../ui/keysyms.h"
@@ -454,6 +461,11 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct annotation_options *annotation_options);
 
 int script_browse(const char *script_opt, struct perf_evsel *evsel);
+
+void run_script(char *cmd);
+int res_sample_browse(struct res_sample *res_samples, int num_res,
+		      struct perf_evsel *evsel, enum rstype rstype);
+void res_sample_init(void);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
@@ -488,6 +500,16 @@ static inline int script_browse(const char *script_opt __maybe_unused,
 	return 0;
 }
 
+static inline int res_sample_browse(struct res_sample *res_samples __maybe_unused,
+				    int num_res __maybe_unused,
+				    struct perf_evsel *evsel __maybe_unused,
+				    enum rstype rstype __maybe_unused)
+{
+	return 0;
+}
+
+static inline void res_sample_init(void) {}
+
 #define K_LEFT  -1000
 #define K_RIGHT -2000
 #define K_SWITCH_INPUT_DATA -3000
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 19dceb7f6145..bb9442ab7a0c 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -47,6 +47,12 @@ extern struct sort_entry sort_srcline;
 extern enum sort_type sort__first_dimension;
 extern const char default_mem_sort_order[];
 
+struct res_sample {
+	u64 time;
+	int cpu;
+	int tid;
+};
+
 struct he_stat {
 	u64			period;
 	u64			period_sys;
@@ -140,6 +146,8 @@ struct hist_entry {
 	struct mem_info		*mem_info;
 	void			*raw_data;
 	u32			raw_size;
+	int			num_res;
+	struct res_sample	*res_samples;
 	void			*trace_output;
 	struct perf_hpp_list	*hpp_list;
 	struct hist_entry	*parent_he;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 967066d4d899..8383184448a6 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -52,6 +52,7 @@ struct symbol_conf symbol_conf = {
 	.symfs			= "",
 	.event_group		= true,
 	.inline_name		= true,
+	.res_sample		= 0,
 };
 
 static enum dso_binary_type binary_type_symtab[] = {
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index a5684a71b78e..6c55fa6fccec 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -68,6 +68,7 @@ struct symbol_conf {
 	struct intlist	*pid_list,
 			*tid_list;
 	const char	*symfs;
+	int		res_sample;
 };
 
 extern struct symbol_conf symbol_conf;
-- 
2.20.1

Re: [PATCH v6 07/11] perf tools report: Implement browsing of individual samples

[Jiri Olsa]

On Mon, Mar 11, 2019 at 07:44:58AM -0700, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 9e5f87558af6..cdba58447b85 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -125,7 +125,7 @@ static int list_scripts(char *script_name, bool *custom,
>  	return ret;
>  }
>  
> -static void run_script(char *cmd)
> +void run_script(char *cmd)
>  {
>  	pr_debug("Running %s\n", cmd);
>  	SLang_reset_tty();
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index fd5d02c1a521..3a0e2bb40529 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -436,6 +436,13 @@ static int hist_entry__init(struct hist_entry *he,
>  			goto err_rawdata;
>  	}
>  
> +	if (symbol_conf.res_sample) {
> +		he->res_samples = calloc(sizeof(struct res_sample),
> +					symbol_conf.res_sample);
> +		if (!he->res_samples)
> +			return -ENOMEM;
> +	}

https://lore.kernel.org/lkml/20190311123227.GA26829@krava/

jirka

Re: [PATCH v6 07/11] perf tools report: Implement browsing of individual samples

[Andi Kleen]

On Mon, Mar 11, 2019 at 06:24:20PM +0100, Jiri Olsa wrote:
> On Mon, Mar 11, 2019 at 07:44:58AM -0700, Andi Kleen wrote:
> 
> SNIP
> 
> > diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> > index 9e5f87558af6..cdba58447b85 100644
> > --- a/tools/perf/ui/browsers/scripts.c
> > +++ b/tools/perf/ui/browsers/scripts.c
> > @@ -125,7 +125,7 @@ static int list_scripts(char *script_name, bool *custom,
> >  	return ret;
> >  }
> >  
> > -static void run_script(char *cmd)
> > +void run_script(char *cmd)
> >  {
> >  	pr_debug("Running %s\n", cmd);
> >  	SLang_reset_tty();
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index fd5d02c1a521..3a0e2bb40529 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -436,6 +436,13 @@ static int hist_entry__init(struct hist_entry *he,
> >  			goto err_rawdata;
> >  	}
> >  
> > +	if (symbol_conf.res_sample) {
> > +		he->res_samples = calloc(sizeof(struct res_sample),
> > +					symbol_conf.res_sample);
> > +		if (!he->res_samples)
> > +			return -ENOMEM;
> > +	}
> 
> https://lore.kernel.org/lkml/20190311123227.GA26829@krava/

Here's an updated patch.

---
perf tools report: Implement browsing of individual samples

Now report can show whole time periods with perf script,
but the user still has to find individual samples of interest
manually.

It would be expensive and complicated to search for the
right samples in the whole perf file. Typically users
only need to look at a small number of samples for useful
analysis.

Also the full scripts tend to show samples of all CPUs and all
threads mixed up, which can be very confusing on larger systems.

Add a new --samples option to save a small random number of samples
per hist entry

Use a reservoir sample technique to select a representatve
number of samples.

Then allow browsing the samples using perf script
as part of the hist entry context menu. This automatically
adds the right filters, so only the thread or cpu of the sample
is displayed. Then we use less' search functionality
to directly jump the to the time stamp of the selected
sample.

It uses different menus for assembler and source display.
Assembler needs xed installed and source needs debuginfo.

Currently it only supports as many samples as fit on
the screen due to some limitations in the slang ui code.

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

---
v2:
Free names on error path
Pass --inline and --show-*-event to child perf as needed.
v3:
Don't use num_res to keep track of total samples
Don't pass -1 cpus to perf script.
Don't pass extra event option with tid filtering.
Fix sampling bug.
v4:
Add perfconfig option for context
Use _PER_NSEC defines
v5:
Rebase
Free res_samples
v6:
Free other memory in ENOMEM error path
---
 tools/perf/Documentation/perf-config.txt |  6 ++
 tools/perf/Documentation/perf-report.txt |  4 +
 tools/perf/builtin-report.c              |  2 +
 tools/perf/ui/browsers/Build             |  1 +
 tools/perf/ui/browsers/hists.c           | 47 ++++++++++++
 tools/perf/ui/browsers/res_sample.c      | 95 ++++++++++++++++++++++++
 tools/perf/ui/browsers/scripts.c         |  2 +-
 tools/perf/util/hist.c                   | 39 ++++++++++
 tools/perf/util/hist.h                   | 22 ++++++
 tools/perf/util/sort.h                   |  8 ++
 tools/perf/util/symbol.c                 |  1 +
 tools/perf/util/symbol_conf.h            |  1 +
 12 files changed, 227 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/ui/browsers/res_sample.c

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 86f3dcc15f83..2d0fb7613134 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -584,6 +584,12 @@ llvm.*::
 	llvm.opts::
 		Options passed to llc.
 
+samples.*::
+
+	samples.context::
+		Define how many ns worth of time to show
+		around samples in perf report sample context browser.
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 546d87221ad8..f441baa794ce 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -461,6 +461,10 @@ include::itrace.txt[]
 --socket-filter::
 	Only report the samples on the processor socket that match with this filter
 
+--samples=N::
+	Save N individual samples for each histogram entry to show context in perf
+	report tui browser.
+
 --raw-trace::
 	When displaying traceevent output, do not use print fmt or plugins.
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 05c8dd41106c..1921aaa9cece 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1159,6 +1159,8 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
 		    "Enable kernel symbol demangling"),
 	OPT_BOOLEAN(0, "mem-mode", &report.mem_mode, "mem access profile"),
+	OPT_INTEGER(0, "samples", &symbol_conf.res_sample,
+		    "Number of samples to save per histogram entry for individual browsing"),
 	OPT_CALLBACK(0, "percent-limit", &report, "percent",
 		     "Don't show entries under that percent", parse_percent_limit),
 	OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
diff --git a/tools/perf/ui/browsers/Build b/tools/perf/ui/browsers/Build
index 8fee56b46502..fdf86f7981ca 100644
--- a/tools/perf/ui/browsers/Build
+++ b/tools/perf/ui/browsers/Build
@@ -3,6 +3,7 @@ perf-y += hists.o
 perf-y += map.o
 perf-y += scripts.o
 perf-y += header.o
+perf-y += res_sample.o
 
 CFLAGS_annotate.o += -DENABLE_SLFUTURE_CONST
 CFLAGS_hists.o    += -DENABLE_SLFUTURE_CONST
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fb4430f8982c..3421ecbdd3f0 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1226,6 +1226,8 @@ void hist_browser__init_hpp(void)
 				hist_browser__hpp_color_overhead_guest_us;
 	perf_hpp__format[PERF_HPP__OVERHEAD_ACC].color =
 				hist_browser__hpp_color_overhead_acc;
+
+	res_sample_init();
 }
 
 static int hist_browser__show_entry(struct hist_browser *browser,
@@ -2345,6 +2347,7 @@ struct popup_action {
 	struct map_symbol 	ms;
 	int			socket;
 	struct perf_evsel	*evsel;
+	enum rstype		rstype;
 
 	int (*fn)(struct hist_browser *browser, struct popup_action *act);
 };
@@ -2572,6 +2575,17 @@ do_run_script(struct hist_browser *browser __maybe_unused,
 	return 0;
 }
 
+static int
+do_res_sample_script(struct hist_browser *browser __maybe_unused,
+		     struct popup_action *act)
+{
+	struct hist_entry *he;
+
+	he = hist_browser__selected_entry(browser);
+	res_sample_browse(he->res_samples, he->num_res, act->evsel, act->rstype);
+	return 0;
+}
+
 static int
 add_script_opt_2(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act, char **optstr,
@@ -2629,6 +2643,27 @@ add_script_opt(struct hist_browser *browser,
 	return n;
 }
 
+static int
+add_res_sample_opt(struct hist_browser *browser __maybe_unused,
+		   struct popup_action *act, char **optstr,
+		   struct res_sample *res_sample,
+		   struct perf_evsel *evsel,
+		   enum rstype type)
+{
+	if (!res_sample)
+		return 0;
+
+	if (asprintf(optstr, "Show context for individual samples %s",
+		type == A_ASM ? "with assembler" :
+		type == A_SOURCE ? "with source" : "") < 0)
+		return 0;
+
+	act->fn = do_res_sample_script;
+	act->evsel = evsel;
+	act->rstype = type;
+	return 1;
+}
+
 static int
 do_switch_data(struct hist_browser *browser __maybe_unused,
 	       struct popup_action *act __maybe_unused)
@@ -3115,6 +3150,18 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		}
 		nr_options += add_script_opt(browser, &actions[nr_options],
 					     &options[nr_options], NULL, NULL, evsel);
+		nr_options += add_res_sample_opt(browser, &actions[nr_options],
+						 &options[nr_options],
+				 hist_browser__selected_entry(browser)->res_samples,
+				 evsel, A_NORMAL);
+		nr_options += add_res_sample_opt(browser, &actions[nr_options],
+						 &options[nr_options],
+				 hist_browser__selected_entry(browser)->res_samples,
+				 evsel, A_ASM);
+		nr_options += add_res_sample_opt(browser, &actions[nr_options],
+						 &options[nr_options],
+				 hist_browser__selected_entry(browser)->res_samples,
+				 evsel, A_SOURCE);
 		nr_options += add_switch_opt(browser, &actions[nr_options],
 					     &options[nr_options]);
 skip_scripting:
diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
new file mode 100644
index 000000000000..c2a8536904bc
--- /dev/null
+++ b/tools/perf/ui/browsers/res_sample.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Display a menu with individual samples to browse with perf script */
+#include "util.h"
+#include "hist.h"
+#include "evsel.h"
+#include "hists.h"
+#include "sort.h"
+#include "config.h"
+#include "time-utils.h"
+#include <linux/time64.h>
+
+static u64 context_len = 10*NSEC_PER_MSEC;
+
+static int res_sample_config(const char *var, const char *value, void *data
+			     __maybe_unused)
+{
+	if (!strcmp(var, "samples.context"))
+		return perf_config_u64(&context_len, var, value);
+	return 0;
+}
+
+void res_sample_init(void)
+{
+	perf_config(res_sample_config, NULL);
+}
+
+int res_sample_browse(struct res_sample *res_samples, int num_res,
+		      struct perf_evsel *evsel, enum rstype rstype)
+{
+	char **names;
+	int i, n;
+	int choice;
+	char *cmd;
+	char pbuf[256], tidbuf[32], cpubuf[32];
+	const char *perf = perf_exe(pbuf, sizeof pbuf);
+	char trange[128], tsample[64];
+	struct res_sample *r;
+	char extra_format[256];
+
+	/* For now since ui__popup_menu doesn't like lists that don't fit */
+	num_res = max(min(SLtt_Screen_Rows - 4, num_res), 0);
+
+	names = calloc(num_res, sizeof(char *));
+	if (!names)
+		return -1;
+	for (i = 0; i < num_res; i++) {
+		char tbuf[64];
+
+		timestamp__scnprintf_nsec(res_samples[i].time, tbuf, sizeof tbuf);
+		if (asprintf(&names[i], "%s: CPU %d tid %d", tbuf,
+			     res_samples[i].cpu, res_samples[i].tid) < 0) {
+			while (--i >= 0)
+				free(names[i]);
+			free(names);
+			return -1;
+		}
+	}
+	choice = ui__popup_menu(num_res, names);
+	for (i = 0; i < num_res; i++)
+		free(names[i]);
+	free(names);
+
+	if (choice < 0 || choice >= num_res)
+		return -1;
+	r = &res_samples[choice];
+
+	n = timestamp__scnprintf_nsec(r->time - context_len, trange, sizeof trange);
+	trange[n++] = ',';
+	timestamp__scnprintf_nsec(r->time + context_len, trange + n, sizeof trange - n);
+
+	timestamp__scnprintf_nsec(r->time, tsample, sizeof tsample);
+
+	attr_to_script(extra_format, &evsel->attr);
+
+	if (asprintf(&cmd, "%s script %s%s --time %s %s%s %s%s --ns %s %s %s %s %s | less +/%s",
+		     perf,
+		     input_name ? "-i " : "",
+		     input_name ? input_name : "",
+		     trange,
+		     r->cpu >= 0 ? "--cpu " : "",
+		     r->cpu >= 0 ? (sprintf(cpubuf, "%d", r->cpu), cpubuf) : "",
+		     r->tid ? "--tid " : "",
+		     r->tid ? (sprintf(tidbuf, "%d", r->tid), tidbuf) : "",
+		     extra_format,
+		     rstype == A_ASM ? "-F +insn --xed" :
+		     rstype == A_SOURCE ? "-F +srcline,+srccode" : "",
+		     symbol_conf.inline_name ? "--inline" : "",
+		     "--show-lost-events ",
+		     r->tid ? "--show-switch-events --show-task-events " : "",
+		     tsample) < 0)
+		return -1;
+	run_script(cmd);
+	free(cmd);
+	return 0;
+}
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 9e5f87558af6..cdba58447b85 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -125,7 +125,7 @@ static int list_scripts(char *script_name, bool *custom,
 	return ret;
 }
 
-static void run_script(char *cmd)
+void run_script(char *cmd)
 {
 	pr_debug("Running %s\n", cmd);
 	SLang_reset_tty();
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fd5d02c1a521..c2894013191a 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -436,6 +436,13 @@ static int hist_entry__init(struct hist_entry *he,
 			goto err_rawdata;
 	}
 
+	if (symbol_conf.res_sample) {
+		he->res_samples = calloc(sizeof(struct res_sample),
+					symbol_conf.res_sample);
+		if (!he->res_samples)
+			goto err_srcline;
+	}
+
 	INIT_LIST_HEAD(&he->pairs.node);
 	thread__get(he->thread);
 	he->hroot_in  = RB_ROOT_CACHED;
@@ -446,6 +453,9 @@ static int hist_entry__init(struct hist_entry *he,
 
 	return 0;
 
+err_srcline:
+	free(he->srcline);
+
 err_rawdata:
 	free(he->raw_data);
 
@@ -603,6 +613,32 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 	return he;
 }
 
+static unsigned random_max(unsigned high)
+{
+	unsigned thresh = -high % high;
+	for (;;) {
+		unsigned r = random();
+		if (r >= thresh)
+			return r % high;
+	}
+}
+
+static void hists__res_sample(struct hist_entry *he, struct perf_sample *sample)
+{
+	struct res_sample *r;
+	int j;
+
+	if (he->num_res < symbol_conf.res_sample) {
+		j = he->num_res++;
+	} else {
+		j = random_max(symbol_conf.res_sample);
+	}
+	r = &he->res_samples[j];
+	r->time = sample->time;
+	r->cpu = sample->cpu;
+	r->tid = sample->tid;
+}
+
 static struct hist_entry*
 __hists__add_entry(struct hists *hists,
 		   struct addr_location *al,
@@ -650,6 +686,8 @@ __hists__add_entry(struct hists *hists,
 
 	if (!hists->has_callchains && he && he->callchain_size != 0)
 		hists->has_callchains = true;
+	if (he && symbol_conf.res_sample)
+		hists__res_sample(he, sample);
 	return he;
 }
 
@@ -1173,6 +1211,7 @@ void hist_entry__delete(struct hist_entry *he)
 		mem_info__zput(he->mem_info);
 	}
 
+	zfree(&he->res_samples);
 	zfree(&he->stat_acc);
 	free_srcline(he->srcline);
 	if (he->srcfile && he->srcfile[0])
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2113a6639cea..76ff6c6d03b8 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -433,6 +433,13 @@ struct hist_browser_timer {
 };
 
 struct annotation_options;
+struct res_sample;
+
+enum rstype {
+	A_NORMAL,
+	A_ASM,
+	A_SOURCE
+};
 
 #ifdef HAVE_SLANG_SUPPORT
 #include "../ui/keysyms.h"
@@ -454,6 +461,11 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct annotation_options *annotation_options);
 
 int script_browse(const char *script_opt, struct perf_evsel *evsel);
+
+void run_script(char *cmd);
+int res_sample_browse(struct res_sample *res_samples, int num_res,
+		      struct perf_evsel *evsel, enum rstype rstype);
+void res_sample_init(void);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
@@ -488,6 +500,16 @@ static inline int script_browse(const char *script_opt __maybe_unused,
 	return 0;
 }
 
+static inline int res_sample_browse(struct res_sample *res_samples __maybe_unused,
+				    int num_res __maybe_unused,
+				    struct perf_evsel *evsel __maybe_unused,
+				    enum rstype rstype __maybe_unused)
+{
+	return 0;
+}
+
+static inline void res_sample_init(void) {}
+
 #define K_LEFT  -1000
 #define K_RIGHT -2000
 #define K_SWITCH_INPUT_DATA -3000
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 19dceb7f6145..bb9442ab7a0c 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -47,6 +47,12 @@ extern struct sort_entry sort_srcline;
 extern enum sort_type sort__first_dimension;
 extern const char default_mem_sort_order[];
 
+struct res_sample {
+	u64 time;
+	int cpu;
+	int tid;
+};
+
 struct he_stat {
 	u64			period;
 	u64			period_sys;
@@ -140,6 +146,8 @@ struct hist_entry {
 	struct mem_info		*mem_info;
 	void			*raw_data;
 	u32			raw_size;
+	int			num_res;
+	struct res_sample	*res_samples;
 	void			*trace_output;
 	struct perf_hpp_list	*hpp_list;
 	struct hist_entry	*parent_he;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 967066d4d899..8383184448a6 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -52,6 +52,7 @@ struct symbol_conf symbol_conf = {
 	.symfs			= "",
 	.event_group		= true,
 	.inline_name		= true,
+	.res_sample		= 0,
 };
 
 static enum dso_binary_type binary_type_symtab[] = {
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index a5684a71b78e..6c55fa6fccec 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -68,6 +68,7 @@ struct symbol_conf {
 	struct intlist	*pid_list,
 			*tid_list;
 	const char	*symfs;
+	int		res_sample;
 };
 
 extern struct symbol_conf symbol_conf;
-- 
2.20.1

Re: [PATCH v6 07/11] perf tools report: Implement browsing of individual samples

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 10:46:05AM -0700, Andi Kleen escreveu:
> On Mon, Mar 11, 2019 at 06:24:20PM +0100, Jiri Olsa wrote:
> > On Mon, Mar 11, 2019 at 07:44:58AM -0700, Andi Kleen wrote:
> > 
> > SNIP
> > 
> > > diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> > > index 9e5f87558af6..cdba58447b85 100644
> > > --- a/tools/perf/ui/browsers/scripts.c
> > > +++ b/tools/perf/ui/browsers/scripts.c
> > > @@ -125,7 +125,7 @@ static int list_scripts(char *script_name, bool *custom,
> > >  	return ret;
> > >  }
> > >  
> > > -static void run_script(char *cmd)
> > > +void run_script(char *cmd)
> > >  {
> > >  	pr_debug("Running %s\n", cmd);
> > >  	SLang_reset_tty();
> > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > > index fd5d02c1a521..3a0e2bb40529 100644
> > > --- a/tools/perf/util/hist.c
> > > +++ b/tools/perf/util/hist.c
> > > @@ -436,6 +436,13 @@ static int hist_entry__init(struct hist_entry *he,
> > >  			goto err_rawdata;
> > >  	}
> > >  
> > > +	if (symbol_conf.res_sample) {
> > > +		he->res_samples = calloc(sizeof(struct res_sample),
> > > +					symbol_conf.res_sample);
> > > +		if (!he->res_samples)
> > > +			return -ENOMEM;
> > > +	}
> > 
> > https://lore.kernel.org/lkml/20190311123227.GA26829@krava/
> 
> Here's an updated patch.

And here is the diff to what was before. Replaced the previous patch
with this one and I'm adding Jiri's Acked-by to the whole series, as he
provided me privately, thanks guys!

- Arnaldo


diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 3a0e2bb40529..c2894013191a 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -440,7 +440,7 @@ static int hist_entry__init(struct hist_entry *he,
 		he->res_samples = calloc(sizeof(struct res_sample),
 					symbol_conf.res_sample);
 		if (!he->res_samples)
-			return -ENOMEM;
+			goto err_srcline;
 	}
 
 	INIT_LIST_HEAD(&he->pairs.node);
@@ -453,6 +453,9 @@ static int hist_entry__init(struct hist_entry *he,
 
 	return 0;
 
+err_srcline:
+	free(he->srcline);
+
 err_rawdata:
 	free(he->raw_data);
 

Re: [PATCH v6 07/11] perf tools report: Implement browsing of individual samples

[Andi Kleen]

> And here is the diff to what was before. Replaced the previous patch
> with this one and I'm adding Jiri's Acked-by to the whole series, as he
> provided me privately, thanks guys!

Great. Thanks everyone!


-Andi

[PATCH v6 08/11] perf tools: Add some new tips describing the new options

[Andi Kleen]

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

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

---
v2:
Even more tips.
v3:
Even more tips
---
 tools/perf/Documentation/tips.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/Documentation/tips.txt b/tools/perf/Documentation/tips.txt
index 849599f39c5e..869965d629ce 100644
--- a/tools/perf/Documentation/tips.txt
+++ b/tools/perf/Documentation/tips.txt
@@ -15,6 +15,7 @@ To see callchains in a more compact form: perf report -g folded
 Show individual samples with: perf script
 Limit to show entries above 5% only: perf report --percent-limit 5
 Profiling branch (mis)predictions with: perf record -b / perf report
+To show assembler sample contexts use perf record -b / perf script -F +brstackinsn --xed
 Treat branches as callchains: perf report --branch-history
 To count events in every 1000 msec: perf stat -I 1000
 Print event counts in CSV format with: perf stat -x,
@@ -34,3 +35,9 @@ Show current config key-value pairs: perf config --list
 Show user configuration overrides: perf config --user --list
 To add Node.js USDT(User-Level Statically Defined Tracing): perf buildid-cache --add `which node`
 To report cacheline events from previous recording: perf c2c report
+To browse sample contexts use perf report --sample 10 and select in context menu
+To separate samples by time use perf report --sort time,overhead,sym
+To set sample time separation other than 100ms with --sort time use --time-quantum
+Add -I to perf report to sample register values visible in perf report context.
+To show IPC for sampling periods use perf record -e '{cycles,instructions}:S' and then browse context
+To show context switches in perf report sample context add --switch-events to perf record.
-- 
2.20.1

[PATCH v6 09/11] perf tools report: Add custom scripts to script menu

[Andi Kleen]

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

Add a way to define custom scripts through ~/.perfconfig, which
are then added to the scripts menu. The scripts get the same
arguments as perf script, in particular -i, --cpu, --tid.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-config.txt |  8 ++++++++
 tools/perf/ui/browsers/scripts.c         | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 2d0fb7613134..c3323228d3d0 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -590,6 +590,14 @@ samples.*::
 		Define how many ns worth of time to show
 		around samples in perf report sample context browser.
 
+script.*::
+
+	Any option defines a script that is added to the scripts menu
+	in the interactive perf browser and whose output is displayed.
+	The name of the option is the name, the value is a script command line.
+	The script gets the same options passed as a full perf script,
+	in particular -i perfdata file, --cpu, --tid
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index cdba58447b85..5d407ab834b5 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -6,6 +6,7 @@
 #include "../../util/symbol.h"
 #include "../browser.h"
 #include "../libslang.h"
+#include "config.h"
 
 #define SCRIPT_NAMELEN	128
 #define SCRIPT_MAX_NO	64
@@ -53,6 +54,24 @@ static int add_script_option(const char *name, const char *opt,
 	return 0;
 }
 
+static int scripts_config(const char *var, const char *value, void *data)
+{
+	struct script_config *c = data;
+
+	if (!strstarts(var, "script."))
+		return -1;
+	if (c->index >= SCRIPT_MAX_NO)
+		return -1;
+	c->names[c->index] = strdup(var + 7);
+	if (!c->names[c->index])
+		return -1;
+	if (asprintf(&c->paths[c->index], "%s %s", value,
+		     c->extra_format) < 0)
+		return -1;
+	c->index++;
+	return 0;
+}
+
 /*
  * When success, will copy the full path of the selected script
  * into  the buffer pointed by script_name, and return 0.
@@ -87,6 +106,7 @@ static int list_scripts(char *script_name, bool *custom,
 			  &scriptc);
 	add_script_option("Show individual samples with source", "-F +srcline,+srccode",
 			  &scriptc);
+	perf_config(scripts_config, &scriptc);
 	custom_perf = scriptc.index;
 	add_script_option("Show samples with custom perf script arguments", "", &scriptc);
 	i = scriptc.index;
-- 
2.20.1

Re: [PATCH v6 09/11] perf tools report: Add custom scripts to script menu

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:45:00AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add a way to define custom scripts through ~/.perfconfig, which
> are then added to the scripts menu. The scripts get the same
> arguments as perf script, in particular -i, --cpu, --tid.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/Documentation/perf-config.txt |  8 ++++++++
>  tools/perf/ui/browsers/scripts.c         | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index 2d0fb7613134..c3323228d3d0 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -590,6 +590,14 @@ samples.*::
>  		Define how many ns worth of time to show
>  		around samples in perf report sample context browser.
>  
> +script.*::
> +
> +	Any option defines a script that is added to the scripts menu
> +	in the interactive perf browser and whose output is displayed.
> +	The name of the option is the name, the value is a script command line.
> +	The script gets the same options passed as a full perf script,
> +	in particular -i perfdata file, --cpu, --tid
> +

Isn't it better to use 'scripts' for those scripts and leave 'script'
for configuring the 'perf script' command like we have options for
annotate, etc?

- Arnaldo

>  SEE ALSO
>  --------
>  linkperf:perf[1]
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index cdba58447b85..5d407ab834b5 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -6,6 +6,7 @@
>  #include "../../util/symbol.h"
>  #include "../browser.h"
>  #include "../libslang.h"
> +#include "config.h"
>  
>  #define SCRIPT_NAMELEN	128
>  #define SCRIPT_MAX_NO	64
> @@ -53,6 +54,24 @@ static int add_script_option(const char *name, const char *opt,
>  	return 0;
>  }
>  
> +static int scripts_config(const char *var, const char *value, void *data)
> +{
> +	struct script_config *c = data;
> +
> +	if (!strstarts(var, "script."))
> +		return -1;
> +	if (c->index >= SCRIPT_MAX_NO)
> +		return -1;
> +	c->names[c->index] = strdup(var + 7);
> +	if (!c->names[c->index])
> +		return -1;
> +	if (asprintf(&c->paths[c->index], "%s %s", value,
> +		     c->extra_format) < 0)
> +		return -1;
> +	c->index++;
> +	return 0;
> +}
> +
>  /*
>   * When success, will copy the full path of the selected script
>   * into  the buffer pointed by script_name, and return 0.
> @@ -87,6 +106,7 @@ static int list_scripts(char *script_name, bool *custom,
>  			  &scriptc);
>  	add_script_option("Show individual samples with source", "-F +srcline,+srccode",
>  			  &scriptc);
> +	perf_config(scripts_config, &scriptc);
>  	custom_perf = scriptc.index;
>  	add_script_option("Show samples with custom perf script arguments", "", &scriptc);
>  	i = scriptc.index;
> -- 
> 2.20.1

-- 

- Arnaldo

Re: [PATCH v6 09/11] perf tools report: Add custom scripts to script menu

[Andi Kleen]

> Isn't it better to use 'scripts' for those scripts and leave 'script'
> for configuring the 'perf script' command like we have options for
> annotate, etc?

Yes that's fine.

That's just two character updates in the doc and in the strstarts below. 
If there's nothing else can you please just do these changes when you apply?

Thanks.

> > +static int scripts_config(const char *var, const char *value, void *data)
> > +{
> > +	struct script_config *c = data;
> > +
> > +	if (!strstarts(var, "script."))
> > +		return -1;

Re: [PATCH v6 09/11] perf tools report: Add custom scripts to script menu

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 11:34:55AM -0700, Andi Kleen escreveu:
> > Isn't it better to use 'scripts' for those scripts and leave 'script'
> > for configuring the 'perf script' command like we have options for
> > annotate, etc?
> 
> Yes that's fine.
> 
> That's just two character updates in the doc and in the strstarts below. 
> If there's nothing else can you please just do these changes when you apply?

Sure, doing that now.
 
> Thanks.
> 
> > > +static int scripts_config(const char *var, const char *value, void *data)
> > > +{
> > > +	struct script_config *c = data;
> > > +
> > > +	if (!strstarts(var, "script."))
> > > +		return -1;

-- 

- Arnaldo

[PATCH v6 10/11] perf tools script: Add array bound checking to list_scripts

[Andi Kleen]

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

Don't overflow array when the scripts directory is too large,
or the script file name is too long.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/builtin-script.c      | 8 ++++++--
 tools/perf/builtin.h             | 3 ++-
 tools/perf/ui/browsers/scripts.c | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b695b20ffc8a..2f93d60c5a17 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2982,7 +2982,8 @@ static int check_ev_match(char *dir_name, char *scriptname,
  * will list all statically runnable scripts, select one, execute it and
  * show the output in a perf browser.
  */
-int find_scripts(char **scripts_array, char **scripts_path_array)
+int find_scripts(char **scripts_array, char **scripts_path_array, int num,
+		 int pathlen)
 {
 	struct dirent *script_dirent, *lang_dirent;
 	char scripts_path[MAXPATHLEN], lang_path[MAXPATHLEN];
@@ -3027,7 +3028,10 @@ int find_scripts(char **scripts_array, char **scripts_path_array)
 			/* Skip those real time scripts: xxxtop.p[yl] */
 			if (strstr(script_dirent->d_name, "top."))
 				continue;
-			sprintf(scripts_path_array[i], "%s/%s", lang_path,
+			if (i >= num)
+				break;
+			snprintf(scripts_path_array[i], pathlen, "%s/%s",
+				lang_path,
 				script_dirent->d_name);
 			temp = strchr(script_dirent->d_name, '.');
 			snprintf(scripts_array[i],
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index 05745f3ce912..999fe9170122 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -40,5 +40,6 @@ int cmd_mem(int argc, const char **argv);
 int cmd_data(int argc, const char **argv);
 int cmd_ftrace(int argc, const char **argv);
 
-int find_scripts(char **scripts_array, char **scripts_path_array);
+int find_scripts(char **scripts_array, char **scripts_path_array, int num,
+		 int pathlen);
 #endif
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 5d407ab834b5..6759d6657e1a 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -117,7 +117,8 @@ static int list_scripts(char *script_name, bool *custom,
 		paths[i] = names[i] + SCRIPT_NAMELEN;
 	}
 
-	num = find_scripts(names + max_std, paths + max_std);
+	num = find_scripts(names + max_std, paths + max_std, SCRIPT_MAX_NO - max_std,
+			SCRIPT_FULLPATH_LEN);
 	if (num < 0)
 		num = 0;
 	choice = ui__popup_menu(num + max_std, (char * const *)names);
-- 
2.20.1

Re: [PATCH v6 10/11] perf tools script: Add array bound checking to list_scripts

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:45:01AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Don't overflow array when the scripts directory is too large,
> or the script file name is too long.

Applied.

[PATCH v6 11/11] perf tools ui: Fix ui popup browser for many entries

[Andi Kleen]

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

Fix the argv ui browser code to correctly display more entries
than fit on the screen without crashing. The problem was some type
confusion with pointer types in the ->seek function. Do
the argv arithmetic correctly with char ** pointers. Also
add some asserts to find overruns and limit the display function
correctly.

Then finally remove a workaround for this in the res sample
browser.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/ui/browser.c             | 10 +++++++---
 tools/perf/ui/browsers/res_sample.c |  3 ---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 4f75561424ed..4ad37d8c7d6a 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -611,14 +611,16 @@ void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence)
 		browser->top = browser->entries;
 		break;
 	case SEEK_CUR:
-		browser->top = browser->top + browser->top_idx + offset;
+		browser->top = (char **)browser->top + offset;
 		break;
 	case SEEK_END:
-		browser->top = browser->top + browser->nr_entries - 1 + offset;
+		browser->top = (char **)browser->entries + browser->nr_entries - 1 + offset;
 		break;
 	default:
 		return;
 	}
+	assert((char **)browser->top < (char **)browser->entries + browser->nr_entries);
+	assert((char **)browser->top >= (char **)browser->entries);
 }
 
 unsigned int ui_browser__argv_refresh(struct ui_browser *browser)
@@ -630,7 +632,9 @@ unsigned int ui_browser__argv_refresh(struct ui_browser *browser)
 		browser->top = browser->entries;
 
 	pos = (char **)browser->top;
-	while (idx < browser->nr_entries) {
+	while (idx < browser->nr_entries &&
+	       row < (unsigned)SLtt_Screen_Rows - 1) {
+		assert(pos < (char **)browser->entries + browser->nr_entries);
 		if (!browser->filter || !browser->filter(browser, *pos)) {
 			ui_browser__gotorc(browser, row, 0);
 			browser->write(browser, pos, row);
diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
index c2a8536904bc..8befd8e984ab 100644
--- a/tools/perf/ui/browsers/res_sample.c
+++ b/tools/perf/ui/browsers/res_sample.c
@@ -37,9 +37,6 @@ int res_sample_browse(struct res_sample *res_samples, int num_res,
 	struct res_sample *r;
 	char extra_format[256];
 
-	/* For now since ui__popup_menu doesn't like lists that don't fit */
-	num_res = max(min(SLtt_Screen_Rows - 4, num_res), 0);
-
 	names = calloc(num_res, sizeof(char *));
 	if (!names)
 		return -1;
-- 
2.20.1

Re: [PATCH v6 11/11] perf tools ui: Fix ui popup browser for many entries

[Arnaldo Carvalho de Melo]

Em Mon, Mar 11, 2019 at 07:45:02AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Fix the argv ui browser code to correctly display more entries
> than fit on the screen without crashing. The problem was some type
> confusion with pointer types in the ->seek function. Do
> the argv arithmetic correctly with char ** pointers. Also
> add some asserts to find overruns and limit the display function
> correctly.
> 
> Then finally remove a workaround for this in the res sample
> browser.

Applied and added this, please do so next time:

Committer testing:

1) Resize the x terminal to have just some 5 lines

2) Use 'perf report --samples 1' to activate the sample browser options
   in the menu

3) Press ENTER, this will cause the crash:

  # perf report --samples 1
  perf: Segmentation fault
  -------- backtrace --------
  perf[0x5a514a]
  /lib64/libc.so.6(+0x385bf)[0x7f27281b55bf]
  /lib64/libc.so.6(+0x161a67)[0x7f27282dea67]
  /lib64/libslang.so.2(SLsmg_write_wrapped_string+0x82)[0x7f272874a0b2]
  perf(ui_browser__argv_refresh+0x77)[0x5939a7]
  perf[0x5924cc]
  perf(ui_browser__run+0x39)[0x593449]
  perf(ui__popup_menu+0x83)[0x5a5263]
  perf[0x59f421]
  perf(perf_evlist__tui_browse_hists+0x3a0)[0x5a3780]
  perf(cmd_report+0x2746)[0x447136]
  perf[0x4a95fe]
  perf(main+0x61c)[0x42dc6c]
  /lib64/libc.so.6(__libc_start_main+0xf2)[0x7f27281a1412]
  perf(_start+0x2d)[0x42de9d]
  #

After applying this patch no crash takes place in such situation.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>