Skip to contentSkip to conversation

Support sample context in perf report

PATCH Applied Andi Kleen 6 participants 36 messages Feb 24, 2019 – Mar 4, 2019 perf events

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

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.

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

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-1

author
#1 lore raw

PATCH 1/11 perf tools script: Handle missing fields with -F +..

From: Andi Kleen <[email protected]>

When using -F + syntax to add a field the existing defaults are currently all marked user_set. This can cause errors when some field is missing in the perf.data

This patch tracks the actually user set fields separately, so that we don't error out in this case.

Before:

% perf record true
% perf script -F +metric
Samples for 'cycles:ppp' event do not have CPU attribute set. Cannot print 'cpu' field. %

After

5 perf record true
% perf script -F +metric

            perf 28936 278636.237688:          1 cycles:ppp:  ffffffff8117da99 perf_event_exec+0x59 (/lib/modules/4.20.0-odilo/build/vmlinux)

...
%

---
 tools/perf/builtin-script.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/builtin-script.c +7 -3
index 8d5fe092525c..29e95a25c6e6 100644
@@ -149,6 +149,7 @@ static struct {
 	unsigned int print_ip_opts;
 	u64 fields;
 	u64 invalid_fields;
+	u64 user_set_fields;
 } output[OUTPUT_TYPE_MAX] = {
 
 	[PERF_TYPE_HARDWARE] = {
@@ -345,7 +346,7 @@ static int perf_evsel__do_check_stype(struct perf_evsel *evsel,
 	if (attr->sample_type & sample_type)
 		return 0;
 
-	if (output[type].user_set) {
+	if (output[type].user_set_fields & field) {
 		if (allow_user_set)
 			return 0;
 		evname = perf_evsel__name(evsel);
@@ -2628,10 +2629,13 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 					pr_warning("\'%s\' not valid for %s events. Ignoring.\n",
 						   all_output_options[i].str, event_type(j));
 				} else {
-					if (change == REMOVE)
+					if (change == REMOVE) {
 						output[j].fields &= ~all_output_options[i].field;
-					else
+						output[j].user_set_fields &= ~all_output_options[i].field;
+					} else {
 						output[j].fields |= all_output_options[i].field;
+						output[j].user_set_fields |= all_output_options[i].field;
+					}
 					output[j].user_set = true;
 					output[j].wildcard_set = true;
 				}
Signature · 1 line
2.17.2
Acked-by on v1 1/11
#2 lore raw

Re: [PATCH 01/11] perf tools script: Handle missing fields with -F +..

Quoted 18 lines · Andi Kleen

From: Andi Kleen <[email protected]>

When using -F + syntax to add a field the existing defaults are currently all marked user_set. This can cause errors when some field is missing in the perf.data

This patch tracks the actually user set fields separately, so that we don't error out in this case.

Before:

% perf record true
% perf script -F +metric
Samples for 'cycles:ppp' event do not have CPU attribute set. Cannot print 'cpu' field. %

After

Quoted 7 lines

5 perf record true
% perf script -F +metric

            perf 28936 278636.237688:          1 cycles:ppp:  ffffffff8117da99 perf_event_exec+0x59 (/lib/modules/4.20.0-odilo/build/vmlinux)

...
%

nice, that one was annoying

thanks,
jirka

Quoted 44 lines
---
 tools/perf/builtin-script.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/builtin-script.c +7 -3
index 8d5fe092525c..29e95a25c6e6 100644
@@ -149,6 +149,7 @@ static struct {
 	unsigned int print_ip_opts;
 	u64 fields;
 	u64 invalid_fields;
+	u64 user_set_fields;
 } output[OUTPUT_TYPE_MAX] = {
 
 	[PERF_TYPE_HARDWARE] = {
@@ -345,7 +346,7 @@ static int perf_evsel__do_check_stype(struct perf_evsel *evsel,
 	if (attr->sample_type & sample_type)
 		return 0;
 
-	if (output[type].user_set) {
+	if (output[type].user_set_fields & field) {
 		if (allow_user_set)
 			return 0;
 		evname = perf_evsel__name(evsel);
@@ -2628,10 +2629,13 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 					pr_warning("\'%s\' not valid for %s events. Ignoring.\n",
 						   all_output_options[i].str, event_type(j));
 				} else {
-					if (change == REMOVE)
+					if (change == REMOVE) {
 						output[j].fields &= ~all_output_options[i].field;
-					else
+						output[j].user_set_fields &= ~all_output_options[i].field;
+					} else {
 						output[j].fields |= all_output_options[i].field;
+						output[j].user_set_fields |= all_output_options[i].field;
+					}
 					output[j].user_set = true;
 					output[j].wildcard_set = true;
 				}
Signature · 1 line
2.17.2
#3 lore raw

Re: [PATCH 01/11] perf tools script: Handle missing fields with -F +..

Em Mon, Feb 25, 2019 at 01:56:42PM +0100, Jiri Olsa escreveu:

Quoted 25 lines · Andi Kleen
Quoted 24 lines

From: Andi Kleen <[email protected]>

When using -F + syntax to add a field the existing defaults are currently all marked user_set. This can cause errors when some field is missing in the perf.data

This patch tracks the actually user set fields separately, so that we don't error out in this case.

Before:

% perf record true
% perf script -F +metric
Samples for 'cycles:ppp' event do not have CPU attribute set. Cannot print 'cpu' field. %

After

5 perf record true
% perf script -F +metric

            perf 28936 278636.237688:          1 cycles:ppp:  ffffffff8117da99 perf_event_exec+0x59 (/lib/modules/4.20.0-odilo/build/vmlinux)

...
%

Quoted 5 lines
Quoted 1 line

nice, that one was annoying

Applied.

- Arnaldo

#4 lore raw

[tip:perf/core] perf script: Handle missing fields with -F +..

In reply to Andi Kleen
perf script: Handle missing fields with -F +..
Commit-ID:  4b6ac811bce46c83811b83cdf87b41251596b9fc
Gitweb:     https://git.kernel.org/tip/4b6ac811bce46c83811b83cdf87b41251596b9fc
Author:     Andi Kleen <[email protected]>
AuthorDate: Sun, 24 Feb 2019 07:37:12 -0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Feb 2019 10:58:07 -0300

perf script: Handle missing fields with -F +..

When using -F + syntax to add a field the existing defaults are currently all marked user_set. This can cause errors when some field is missing in the perf.data

This patch tracks the actually user set fields separately, so that we don't error out in this case.

Before:

% perf record true
% perf script -F +metric
Samples for 'cycles:ppp' event do not have CPU attribute set. Cannot print 'cpu' field. %

After:

5 perf record true
% perf script -F +metric

              perf 28936 278636.237688:          1 cycles:ppp:  ffffffff8117da99 perf_event_exec+0x59 (/lib/modules/4.20.0-odilo/build/vmlinux)

...
%

---
 tools/perf/builtin-script.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/builtin-script.c +7 -3
index 5b1543f42290..2d8cb1d1682c 100644
@@ -149,6 +149,7 @@ static struct {
 	unsigned int print_ip_opts;
 	u64 fields;
 	u64 invalid_fields;
+	u64 user_set_fields;
 } output[OUTPUT_TYPE_MAX] = {
 
 	[PERF_TYPE_HARDWARE] = {
@@ -345,7 +346,7 @@ static int perf_evsel__do_check_stype(struct perf_evsel *evsel,
 	if (attr->sample_type & sample_type)
 		return 0;
 
-	if (output[type].user_set) {
+	if (output[type].user_set_fields & field) {
 		if (allow_user_set)
 			return 0;
 		evname = perf_evsel__name(evsel);
@@ -2632,10 +2633,13 @@ parse:
 					pr_warning("\'%s\' not valid for %s events. Ignoring.\n",
 						   all_output_options[i].str, event_type(j));
 				} else {
-					if (change == REMOVE)
+					if (change == REMOVE) {
 						output[j].fields &= ~all_output_options[i].field;
-					else
+						output[j].user_set_fields &= ~all_output_options[i].field;
+					} else {
 						output[j].fields |= all_output_options[i].field;
+						output[j].user_set_fields |= all_output_options[i].field;
+					}
 					output[j].user_set = true;
 					output[j].wildcard_set = true;
 				}
author
#5 lore raw

PATCH 2/11 perf tools script: Support insn output for normal samples

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

perf script -F +insn was only working for PT traces because the PT instruction decoder was filling in the insn/insn_len sample attributes. Support it for non PT samples too on x86 using the existing x86 instruction decoder.

% perf record -a sleep 1
% perf script -F ip,sym,insn --xed
ffffffff811704c9 remote_function movl %eax, 0x18(%rbx)

 ffffffff8100bb50 intel_bts_enable_local                retq
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)

ffffffff810f1f79 generic_exec_single xor %eax, %eax
ffffffff811704c9 remote_function movl %eax, 0x18(%rbx)
ffffffff8100bb34 intel_bts_enable_local movl 0x2000(%rax), %edx
ffffffff81048610 native_apic_mem_write mov %edi, %edi
...

---
 tools/perf/arch/x86/util/Build      |  1 +
 tools/perf/arch/x86/util/archinsn.c | 41 +++++++++++++++++++++++++++++
 tools/perf/builtin-script.c         | 10 +++++++
 tools/perf/util/archinsn.h          | 12 +++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 tools/perf/arch/x86/util/archinsn.c
 create mode 100644 tools/perf/util/archinsn.h
tools/perf/arch/x86/util/Build +1 -0
index 7aab0be5fc5f..7b8e69bbbdfe 100644
@@ -6,6 +6,7 @@ perf-y += perf_regs.o
 perf-y += group.o
 perf-y += machine.o
 perf-y += event.o
+perf-y += archinsn.o
 
 perf-$(CONFIG_DWARF) += dwarf-regs.o
 perf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
tools/perf/arch/x86/util/archinsn.c new file +41 -0

41 lines not on this page · raw message

tools/perf/builtin-script.c +10 -0
index 29e95a25c6e6..261055302d08 100644
@@ -29,6 +29,7 @@
 #include "util/time-utils.h"
 #include "util/path.h"
 #include "print_binary.h"
+#include "archinsn.h"
 #include <linux/bitmap.h>
 #include <linux/kernel.h>
 #include <linux/stringify.h>
@@ -1227,6 +1228,12 @@ static int perf_sample__fprintf_callindent(struct perf_sample *sample,
 	return len + dlen;
 }
 
+__weak void arch_fetch_insn(struct perf_sample *sample __maybe_unused,
+			    struct thread *thread __maybe_unused,
+			    struct machine *machine __maybe_unused)
+{
+}
+
 static int perf_sample__fprintf_insn(struct perf_sample *sample,
 				     struct perf_event_attr *attr,
 				     struct thread *thread,
@@ -1234,6 +1241,9 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
 {
 	int printed = 0;
 
+	if (sample->insn_len == 0)
+		arch_fetch_insn(sample, thread, machine);
+
 	if (PRINT_FIELD(INSNLEN))
 		printed += fprintf(fp, " ilen: %d", sample->insn_len);
 	if (PRINT_FIELD(INSN)) {
tools/perf/util/archinsn.h new file +12 -0
new file mode 100644
index 000000000000..448cbb6b8d7e
@@ -0,0 +1,12 @@
+#ifndef INSN_H
+#define INSN_H 1
+
+struct perf_sample;
+struct machine;
+struct thread;
+
+void arch_fetch_insn(struct perf_sample *sample,
+		     struct thread *thread,
+		     struct machine *machine);
+
+#endif
Signature · 1 line
2.17.2
#6 lore raw

Re: [PATCH 02/11] perf tools script: Support insn output for normal samples

Quoted 10 lines · Andi Kleen

From: Andi Kleen <[email protected]>

perf script -F +insn was only working for PT traces because the PT instruction decoder was filling in the insn/insn_len sample attributes. Support it for non PT samples too on x86 using the existing x86 instruction decoder.

% perf record -a sleep 1
% perf script -F ip,sym,insn --xed
ffffffff811704c9 remote_function movl %eax, 0x18(%rbx)

Quoted 8 lines
 ffffffff8100bb50 intel_bts_enable_local                retq
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)

ffffffff810f1f79 generic_exec_single xor %eax, %eax
ffffffff811704c9 remote_function movl %eax, 0x18(%rbx)
ffffffff8100bb34 intel_bts_enable_local movl 0x2000(%rax), %edx
ffffffff81048610 native_apic_mem_write mov %edi, %edi

also for samples with empty instruction, where I guess we fail for some reason to retrieve it, we will show default (0?) mnemonic

[jolsa@krava perf]$ ./perf script -F insn
insn:
insn:
insn:
insn:
insn:
insn:
insn:
insn: 76 ea
insn:
insn: 0f a2
insn: 29 c8
insn: 8b 04 90
insn: 0f a2

[jolsa@krava perf]$ ./perf script -F insn --xed

                addb  %al, (%rax)
                addb  %al, (%rax)
                addb  %al, (%rax)
                addb  %al, (%rax)
                addb  %al, (%rax)
                addb  %al, (%rax)
                addb  %al, (%rax)
                jbe 0xffffffffffffffec
                addb  %al, (%rax)
                cpuid
                sub %ecx, %eax
                movl  (%rax,%rdx,4), %eax
                cpuid

jirka

#7 lore raw

Re: [PATCH 02/11] perf tools script: Support insn output for normal samples

In reply to Andi Kleen
Quoted 79 lines · Andi Kleen

From: Andi Kleen <[email protected]>

perf script -F +insn was only working for PT traces because the PT instruction decoder was filling in the insn/insn_len sample attributes. Support it for non PT samples too on x86 using the existing x86 instruction decoder.

% perf record -a sleep 1
% perf script -F ip,sym,insn --xed
ffffffff811704c9 remote_function movl %eax, 0x18(%rbx)

 ffffffff8100bb50 intel_bts_enable_local                retq
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)
 ffffffff81048612 native_apic_mem_write                 movl  %esi, -0xa04000(%rdi)

ffffffff810f1f79 generic_exec_single xor %eax, %eax
ffffffff811704c9 remote_function movl %eax, 0x18(%rbx)
ffffffff8100bb34 intel_bts_enable_local movl 0x2000(%rax), %edx
ffffffff81048610 native_apic_mem_write mov %edi, %edi
...

---
 tools/perf/arch/x86/util/Build      |  1 +
 tools/perf/arch/x86/util/archinsn.c | 41 +++++++++++++++++++++++++++++
 tools/perf/builtin-script.c         | 10 +++++++
 tools/perf/util/archinsn.h          | 12 +++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 tools/perf/arch/x86/util/archinsn.c
 create mode 100644 tools/perf/util/archinsn.h
tools/perf/arch/x86/util/Build +1 -0
index 7aab0be5fc5f..7b8e69bbbdfe 100644
@@ -6,6 +6,7 @@ perf-y += perf_regs.o
 perf-y += group.o
 perf-y += machine.o
 perf-y += event.o
+perf-y += archinsn.o
 
 perf-$(CONFIG_DWARF) += dwarf-regs.o
 perf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
tools/perf/arch/x86/util/archinsn.c new file +31 -0
new file mode 100644
index 000000000000..9e3b0828b018
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "perf.h"
+#include "archinsn.h"
+#include "util/intel-pt-decoder/insn.h"
+#include "machine.h"
+#include "thread.h"
+#include "symbol.h"
+#include "map.h"
+
+void arch_fetch_insn(struct perf_sample *sample,
+		     struct thread *thread,
+		     struct machine *machine)
+{
+	struct addr_location al;
+	u8 cpumode;
+	long offset;
+	struct insn insn;
+	int len;
+
+	if (!sample->ip)
+		return;
+
+	if (machine__kernel_ip(machine, sample->ip))
+		cpumode = PERF_RECORD_MISC_KERNEL;
+	else
+		cpumode = PERF_RECORD_MISC_USER;
+	if (!thread__find_map(thread, cpumode, sample->ip, &al) || !al.map->dso)
+		return;
+	if (al.map->dso->data.status == DSO_DATA_STATUS_ERROR)
+		return;
+	map__load(al.map);

tools/perf/arch/x86/util/archinsn.c line 41

Quoted 10 lines
tools/perf/arch/x86/util/archinsn.c new file +10 -0
+	offset = al.map->map_ip(al.map, sample->ip);
+	len = dso__data_read_offset(al.map->dso, machine, offset, (u8 *)sample->insn,
+				   sizeof(sample->insn));
+	if (len <= 0)
+		return;
+	insn_init(&insn, sample->insn, len, al.map->dso->is_64_bit);
+	insn_get_length(&insn);
+	if (insn_complete(&insn) && insn.length <= len)
+		sample->insn_len = insn.length;
+}

I saw this code around multiple times.. I think bts and pt are using same code to fetch instructions.. could we synchronize and have just one function to do this?

thanks,
jirka

author
#8 lore raw

PATCH 3/11 perf tools report: Support nano seconds

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

Upcoming changes add timestamp output in perf report. Add a --ns argument similar to perf script to support nanoseconds resolution when needed.

---
 tools/perf/Documentation/perf-report.txt | 3 +++
 tools/perf/builtin-report.c              | 1 +
 tools/perf/builtin-script.c              | 1 -
 tools/perf/util/sort.c                   | 1 +
 tools/perf/util/sort.h                   | 2 ++
 5 files changed, 7 insertions(+), 1 deletion(-)
tools/perf/Documentation/perf-report.txt +3 -0
index 1a27bfe05039..51dbc519dbce 100644
@@ -477,6 +477,9 @@ include::itrace.txt[]
 	Please note that not all mmaps are stored, options affecting which ones
 	are include 'perf record --data', for instance.
 
+--ns::
+	Show time stamps in nanoseconds.
+
 --stats::
 	Display overall events statistics without any further processing.
 	(like the one at the end of the perf report -D command)
tools/perf/builtin-report.c +1 -0
index 2e8c74d6430c..bb8918a747ba 100644
@@ -1147,6 +1147,7 @@ int cmd_report(int argc, const char **argv)
 	OPT_CALLBACK(0, "percent-type", &report.annotation_opts, "local-period",
 		     "Set percent type local/global-period/hits",
 		     annotate_parse_percent_type),
+	OPT_BOOLEAN(0, "ns", &nanosecs, "Show times in nanosecs"),
 	OPT_END()
 	};
 	struct perf_data data = {
tools/perf/builtin-script.c +0 -1
index 261055302d08..628c04543974 100644
@@ -59,7 +59,6 @@ static bool			no_callchain;
 static bool			latency_format;
 static bool			system_wide;
 static bool			print_flags;
-static bool			nanosecs;
 static const char		*cpu_list;
 static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 static struct perf_stat_config	stat_config;
tools/perf/util/sort.c +1 -0
index 2b6c1ccb878c..d1010a966f9b 100644
@@ -31,6 +31,7 @@ const char	*field_order;
 regex_t		ignore_callees_regex;
 int		have_ignore_callees = 0;
 enum sort_mode	sort__mode = SORT_MODE__NORMAL;
+bool		nanosecs = false;
 
 /*
  * Replaces all occurrences of a char used with the:
tools/perf/util/sort.h +2 -0
index 2fbee0b1011c..98223d952404 100644
@@ -278,6 +278,8 @@ struct sort_entry {
 extern struct sort_entry sort_thread;
 extern struct list_head hist_entry__sort_list;
 
+extern bool nanosecs;
+
 struct perf_evlist;
 struct tep_handle;
 int setup_sorting(struct perf_evlist *evlist);
Signature · 1 line
2.17.2
#9 lore raw

Re: [PATCH 03/11] perf tools report: Support nano seconds

Quoted 5 lines · Andi Kleen

From: Andi Kleen <[email protected]>

Upcoming changes add timestamp output in perf report. Add a --ns argument similar to perf script to support nanoseconds resolution when needed.

Is this a ISO-8601 date with nanoseconds ?

Quoted 74 lines
---
 tools/perf/Documentation/perf-report.txt | 3 +++
 tools/perf/builtin-report.c              | 1 +
 tools/perf/builtin-script.c              | 1 -
 tools/perf/util/sort.c                   | 1 +
 tools/perf/util/sort.h                   | 2 ++
 5 files changed, 7 insertions(+), 1 deletion(-)
tools/perf/Documentation/perf-report.txt +3 -0
index 1a27bfe05039..51dbc519dbce 100644
@@ -477,6 +477,9 @@ include::itrace.txt[]
 	Please note that not all mmaps are stored, options affecting which ones
 	are include 'perf record --data', for instance.
 
+--ns::
+	Show time stamps in nanoseconds.
+
 --stats::
 	Display overall events statistics without any further processing.
 	(like the one at the end of the perf report -D command)
tools/perf/builtin-report.c +1 -0
index 2e8c74d6430c..bb8918a747ba 100644
@@ -1147,6 +1147,7 @@ int cmd_report(int argc, const char **argv)
 	OPT_CALLBACK(0, "percent-type", &report.annotation_opts, "local-period",
 		     "Set percent type local/global-period/hits",
 		     annotate_parse_percent_type),
+	OPT_BOOLEAN(0, "ns", &nanosecs, "Show times in nanosecs"),
 	OPT_END()
 	};
 	struct perf_data data = {
tools/perf/builtin-script.c +0 -1
index 261055302d08..628c04543974 100644
@@ -59,7 +59,6 @@ static bool			no_callchain;
 static bool			latency_format;
 static bool			system_wide;
 static bool			print_flags;
-static bool			nanosecs;
 static const char		*cpu_list;
 static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 static struct perf_stat_config	stat_config;
tools/perf/util/sort.c +1 -0
index 2b6c1ccb878c..d1010a966f9b 100644
@@ -31,6 +31,7 @@ const char	*field_order;
 regex_t		ignore_callees_regex;
 int		have_ignore_callees = 0;
 enum sort_mode	sort__mode = SORT_MODE__NORMAL;
+bool		nanosecs = false;
 
 /*
  * Replaces all occurrences of a char used with the:
tools/perf/util/sort.h +2 -0
index 2fbee0b1011c..98223d952404 100644
@@ -278,6 +278,8 @@ struct sort_entry {
 extern struct sort_entry sort_thread;
 extern struct list_head hist_entry__sort_list;
 
+extern bool nanosecs;
+
 struct perf_evlist;
 struct tep_handle;
 int setup_sorting(struct perf_evlist *evlist);
#10 lore raw

Re: [PATCH 03/11] perf tools report: Support nano seconds

Quoted 10 lines · Sebastien Boisvert
Quoted 5 lines · Andi Kleen

From: Andi Kleen <[email protected]>

Upcoming changes add timestamp output in perf report. Add a --ns argument similar to perf script to support nanoseconds resolution when needed.

Is this a ISO-8601 date with nanoseconds ?

No.

-Andi

author
#11 lore raw

PATCH 4/11 perf tools report: Parse time quantum

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

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.

---
 tools/perf/Documentation/perf-report.txt |  4 +++
 tools/perf/builtin-report.c              | 37 ++++++++++++++++++++++++
 tools/perf/util/hist.c                   |  2 ++
 tools/perf/util/hist.h                   |  2 ++
 4 files changed, 45 insertions(+)
tools/perf/Documentation/perf-report.txt +4 -0
index 51dbc519dbce..9ec1702bccdd 100644
@@ -497,6 +497,10 @@ include::itrace.txt[]
 	The period/hits keywords set the base the percentage is computed
 	on - the samples period or the number of samples (hits).
 
+--time-quantum::
+	Configure time quantum for time sort key. Default 100ms.
+	Accepts s, us, ms, ns units.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO
tools/perf/builtin-report.c +37 -0
index bb8918a747ba..600e168f3b99 100644
@@ -47,6 +47,7 @@
 #include <errno.h>
 #include <inttypes.h>
 #include <regex.h>
+#include "sane_ctype.h"
 #include <signal.h>
 #include <linux/bitmap.h>
 #include <linux/stringify.h>
@@ -926,6 +927,39 @@ report_parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 	return parse_callchain_report_opt(arg);
 }
 
+static int
+parse_time_quantum(const struct option *opt, const char *arg,
+		   int unset __maybe_unused)
+{
+	unsigned long *time_q = opt->value;
+	char *end;
+
+	*time_q = strtoul(arg, &end, 0);
+	if (end == arg)
+		goto parse_err;
+	while (isspace(*end))
+		end++;
+	if (*end == 0)
+		return 0;
+	if (!strcmp(end, "s")) {
+		*time_q *= 1000000000;
+		return 0;
+	}
+	if (!strcmp(end, "ms")) {
+		*time_q *= 1000000;
+		return 0;
+	}
+	if (!strcmp(end, "us")) {
+		*time_q *= 1000;
+		return 0;
+	}
+	if (!strcmp(end, "ns"))
+		return 0;
+parse_err:
+	pr_err("Cannot parse time quantum `%s'\n", arg);
+	return -1;
+}
+
 int
 report_parse_ignore_callees_opt(const struct option *opt __maybe_unused,
 				const char *arg, int unset __maybe_unused)
@@ -1148,6 +1182,9 @@ int cmd_report(int argc, const char **argv)
 		     "Set percent type local/global-period/hits",
 		     annotate_parse_percent_type),
 	OPT_BOOLEAN(0, "ns", &nanosecs, "Show times in nanosecs"),
+	OPT_CALLBACK(0, "time-quantum", &time_quantum, "time (ms|us|ns)",
+		     "Set time quantum for time sort key (default 100ms)",
+		     parse_time_quantum),
 	OPT_END()
 	};
 	struct perf_data data = {
tools/perf/util/hist.c +2 -0
index 669f961316f0..d25ca5bb91e6 100644
@@ -20,6 +20,8 @@
 #include <inttypes.h>
 #include <sys/param.h>
 
+unsigned long time_quantum = 100000000;
+
 static bool hists__filter_entry_by_dso(struct hists *hists,
 				       struct hist_entry *he);
 static bool hists__filter_entry_by_thread(struct hists *hists,
tools/perf/util/hist.h +2 -0
index 4af27fbab24f..8e602b486c5c 100644
@@ -531,4 +531,6 @@ static inline int hists__scnprintf_title(struct hists *hists, char *bf, size_t s
 	return __hists__scnprintf_title(hists, bf, size, true);
 }
 
+extern unsigned long time_quantum;
+
 #endif	/* __PERF_HIST_H */
Signature · 1 line
2.17.2
#12 lore raw

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

On Sun, Feb 24, 2019 at 07:37:15AM -0800, Andi Kleen wrote:

SNIP

tools/perf/util/hist.c line 23

Quoted 9 lines
tools/perf/util/hist.c +1 -0
index 669f961316f0..d25ca5bb91e6 100644
@@ -20,6 +20,8 @@
 #include <inttypes.h>
 #include <sys/param.h>
 
+unsigned long time_quantum = 100000000;

could you please put this into symbol_conf? it's not ideal, but at least we have all the global configs in one place

thanks,
jirka

Quoted 18 lines
tools/perf/util/hist.c +1 -0
+
 static bool hists__filter_entry_by_dso(struct hists *hists,
 				       struct hist_entry *he);
 static bool hists__filter_entry_by_thread(struct hists *hists,
tools/perf/util/hist.h +2 -0
index 4af27fbab24f..8e602b486c5c 100644
@@ -531,4 +531,6 @@ static inline int hists__scnprintf_title(struct hists *hists, char *bf, size_t s
 	return __hists__scnprintf_title(hists, bf, size, true);
 }
 
+extern unsigned long time_quantum;
+
 #endif	/* __PERF_HIST_H */
Signature · 1 line
2.17.2
author
#13 lore raw

PATCH 5/11 perf tools report: Support time sort key

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

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
---
 tools/perf/Documentation/perf-report.txt |  2 ++
 tools/perf/util/hist.c                   |  9 ++++++
 tools/perf/util/hist.h                   |  1 +
 tools/perf/util/sort.c                   | 38 ++++++++++++++++++++++++
 tools/perf/util/sort.h                   |  2 ++
 5 files changed, 52 insertions(+)
tools/perf/Documentation/perf-report.txt +2 -0
index 9ec1702bccdd..546d87221ad8 100644
@@ -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)
tools/perf/util/hist.c +9 -0
index d25ca5bb91e6..7bf4d85a3021 100644
@@ -194,6 +194,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));
@@ -248,6 +249,13 @@ static void he_stat__add_cpumode_period(struct he_stat *he_stat,
 	}
 }
 
+static long hist_time(unsigned long time)
+{
+	if (time_quantum)
+		return (time / time_quantum) * time_quantum;
+	return (time / 1000000) * 1000000;
+}
+
 static void he_stat__add_period(struct he_stat *he_stat, u64 period,
 				u64 weight)
 {
@@ -628,6 +636,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)
tools/perf/util/hist.h +1 -0
index 8e602b486c5c..8b21b6d006be 100644
@@ -31,6 +31,7 @@ enum hist_filter {
 
 enum hist_column {
 	HISTC_SYMBOL,
+	HISTC_TIME,
 	HISTC_DSO,
 	HISTC_THREAD,
 	HISTC_COMM,
tools/perf/util/sort.c +38 -0
index d1010a966f9b..974a4c553f82 100644
@@ -15,6 +15,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;
@@ -649,6 +650,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 / 1000000000;
+	nsecs -= secs * 1000000000;
+
+	if (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)
@@ -1629,6 +1666,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
tools/perf/util/sort.h +2 -0
index 98223d952404..fc323f10f474 100644
@@ -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,
Signature · 1 line
2.17.2
author
#14 lore raw

PATCH 6/11 perf tools report: Use less for scripts output

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

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 every done could use an terminal for the output.

---
 tools/perf/ui/browsers/scripts.c | 125 +++++--------------------------
 1 file changed, 17 insertions(+), 108 deletions(-)
tools/perf/ui/browsers/scripts.c +17 -108
index 90a32ac69e76..92707be174e4 100644
@@ -12,24 +12,6 @@
 #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 +55,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 +89,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;
 }
Signature · 1 line
2.17.2
#15 lore raw

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

Quoted 7 lines · Andi Kleen

From: Andi Kleen <[email protected]>

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.

Quoted 8 lines

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

---
 tools/perf/ui/browsers/scripts.c | 125 +++++--------------------------
 1 file changed, 17 insertions(+), 108 deletions(-)

I like the stats ;-)

I assume some of the current includes can be removed as well:

#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"

jirka

author
#16 lore raw

PATCH 7/11 perf tools report: Support running scripts for current time range

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

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.

---
 tools/perf/ui/browsers/hists.c | 82 +++++++++++++++++++++++++++++-----
 1 file changed, 71 insertions(+), 11 deletions(-)
tools/perf/ui/browsers/hists.c +71 -11
index aef800d97ea1..2a4f363f8cc7 100644
@@ -30,6 +30,7 @@
 #include "srcline.h"
 #include "string2.h"
 #include "units.h"
+#include "time-utils.h"
 
 #include "sane_ctype.h"
 
@@ -2338,6 +2339,7 @@ static int switch_data_file(void)
 }
 
 struct popup_action {
+	unsigned long		time;
 	struct thread 		*thread;
 	struct map_symbol 	ms;
 	int			socket;
@@ -2527,36 +2529,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[64], end[64];
+		unsigned long starttime = act->time;
+		unsigned long endtime = act->time + time_quantum;
+
+		if (starttime == endtime) { /* Display 1ms as fallback */
+			starttime -= 1*1000000;
+			endtime += 1*1000000;
+		}
+		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 +2596,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 + 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)
Signature · 1 line
2.17.2
#17 lore raw

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

Quoted 8 lines · Andi Kleen

From: Andi Kleen <[email protected]>

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.

getting compilation error:

CC ui/browsers/hists.o
ui/browsers/hists.c: In function ‘perf_evsel__hists_browse’:
ui/browsers/hists.c:2565:8: error: ‘%s’ directive output may be truncated writing up to 63 bytes into a region of size between 28 and 91 [-Werror=format-truncation=]

   n += snprintf(script_opt + n, len - n, " --time %s,%s", start, end);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from /usr/include/stdio.h:862,
from ui/browsers/hists.c:5:
/usr/include/bits/stdio2.h:64:10: note: ‘__builtin___snprintf_chk’ output between 10 and 136 bytes into a destination of size 100
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,

          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[jolsa@krava perf]$ gcc --version
gcc (GCC) 8.2.1 20181215 (Red Hat 8.2.1-6)

jirka

#18 lore raw

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

In reply to Andi Kleen

On Sun, Feb 24, 2019 at 07:37:18AM -0800, Andi Kleen wrote:

SNIP

Quoted 27 lines
tools/perf/ui/browsers/hists.c +14 -4
+			endtime += 1*1000000;
+		}
+		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;

tools/perf/ui/browsers/hists.c static int · line 2590

Quoted 10 lines
tools/perf/ui/browsers/hists.c +3 -3
 	} 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;

can't see the time ranges in the menu.. what's the path to get them listed?

thanks,
jirka

#19 lore raw

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

Quoted 2 lines

can't see the time ranges in the menu.. what's the path to get them listed?

You need to use --sort time,overhead,sym

Without time sorting there are no available ranges.

-Andi

#20 lore raw

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

Quoted 6 lines · Andi Kleen
Quoted 2 lines

can't see the time ranges in the menu.. what's the path to get them listed?

You need to use --sort time,overhead,sym

Without time sorting there are no available ranges.

I did, must be those hacks I put in to compile it, will check in new version

jirka

author
#21 lore raw

PATCH 8/11 perf tools: Add perf_exe() helper to find perf binary

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

Also convert one existing user.

---
 tools/perf/util/header.c | 12 +++---------
 tools/perf/util/util.c   | 10 ++++++++++
 tools/perf/util/util.h   |  2 ++
 3 files changed, 15 insertions(+), 9 deletions(-)
tools/perf/util/header.c +3 -9
index 61ce197c5362..2f290a0343d4 100644
@@ -526,17 +526,11 @@ static int write_event_desc(struct feat_fd *ff,
 static int write_cmdline(struct feat_fd *ff,
 			 struct perf_evlist *evlist __maybe_unused)
 {
-	char buf[MAXPATHLEN];
-	u32 n;
-	int i, ret;
+	char pbuf[MAXPATHLEN], *buf;
+	int i, ret, n;
 
 	/* actual path to perf binary */
-	ret = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
-	if (ret <= 0)
-		return -1;
-
-	/* readlink() does not add null termination */
-	buf[ret] = '\0';
+	buf = perf_exe(pbuf, MAXPATHLEN);
 
 	/* account for binary path */
 	n = perf_env.nr_cmdline + 1;
tools/perf/util/util.c +10 -0
index 320b0fef249a..dd693e018aef 100644
@@ -507,3 +507,13 @@ const char *perf_tip(const char *dirpath)
 
 	return tip;
 }
+
+char *perf_exe(char *buf, int len)
+{
+	int n = readlink("/proc/self/exe", buf, len);
+	if (n > 0) {
+		buf[n] = 0;
+		return buf;
+	}
+	return strcpy(buf, "perf");
+}
tools/perf/util/util.h +2 -0
index ece040b799f6..dc2a99ad5c96 100644
@@ -76,6 +76,8 @@ extern bool perf_singlethreaded;
 void perf_set_singlethreaded(void);
 void perf_set_multithreaded(void);
 
+char *perf_exe(char *buf, int len);
+
 #ifndef O_CLOEXEC
 #ifdef __sparc__
 #define O_CLOEXEC      0x400000
Signature · 1 line
2.17.2
Acked-by on v1 8/11
#22 lore raw

Re: [PATCH 08/11] perf tools: Add perf_exe() helper to find perf binary

Quoted 5 lines · Andi Kleen

From: Andi Kleen <[email protected]>

Also convert one existing user.

thanks,
jirka

Quoted 65 lines
---
 tools/perf/util/header.c | 12 +++---------
 tools/perf/util/util.c   | 10 ++++++++++
 tools/perf/util/util.h   |  2 ++
 3 files changed, 15 insertions(+), 9 deletions(-)
tools/perf/util/header.c +3 -9
index 61ce197c5362..2f290a0343d4 100644
@@ -526,17 +526,11 @@ static int write_event_desc(struct feat_fd *ff,
 static int write_cmdline(struct feat_fd *ff,
 			 struct perf_evlist *evlist __maybe_unused)
 {
-	char buf[MAXPATHLEN];
-	u32 n;
-	int i, ret;
+	char pbuf[MAXPATHLEN], *buf;
+	int i, ret, n;
 
 	/* actual path to perf binary */
-	ret = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
-	if (ret <= 0)
-		return -1;
-
-	/* readlink() does not add null termination */
-	buf[ret] = '\0';
+	buf = perf_exe(pbuf, MAXPATHLEN);
 
 	/* account for binary path */
 	n = perf_env.nr_cmdline + 1;
tools/perf/util/util.c +10 -0
index 320b0fef249a..dd693e018aef 100644
@@ -507,3 +507,13 @@ const char *perf_tip(const char *dirpath)
 
 	return tip;
 }
+
+char *perf_exe(char *buf, int len)
+{
+	int n = readlink("/proc/self/exe", buf, len);
+	if (n > 0) {
+		buf[n] = 0;
+		return buf;
+	}
+	return strcpy(buf, "perf");
+}
tools/perf/util/util.h +2 -0
index ece040b799f6..dc2a99ad5c96 100644
@@ -76,6 +76,8 @@ extern bool perf_singlethreaded;
 void perf_set_singlethreaded(void);
 void perf_set_multithreaded(void);
 
+char *perf_exe(char *buf, int len);
+
 #ifndef O_CLOEXEC
 #ifdef __sparc__
 #define O_CLOEXEC      0x400000
Signature · 1 line
2.17.2
#24 lore raw

[tip:perf/core] perf tools: Add perf_exe() helper to find perf binary

In reply to Andi Kleen
perf tools: Add perf_exe() helper to find perf binary
Commit-ID:  94816add0005595ea33fc8456519be582330401e
Gitweb:     https://git.kernel.org/tip/94816add0005595ea33fc8456519be582330401e
Author:     Andi Kleen <[email protected]>
AuthorDate: Sun, 24 Feb 2019 07:37:19 -0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Feb 2019 10:58:28 -0300

perf tools: Add perf_exe() helper to find perf binary

Also convert one existing user.

---
 tools/perf/util/header.c | 12 +++---------
 tools/perf/util/util.c   | 10 ++++++++++
 tools/perf/util/util.h   |  2 ++
 3 files changed, 15 insertions(+), 9 deletions(-)
tools/perf/util/header.c +3 -9
index a2323d777dae..01b324c275b9 100644
@@ -527,17 +527,11 @@ static int write_event_desc(struct feat_fd *ff,
 static int write_cmdline(struct feat_fd *ff,
 			 struct perf_evlist *evlist __maybe_unused)
 {
-	char buf[MAXPATHLEN];
-	u32 n;
-	int i, ret;
+	char pbuf[MAXPATHLEN], *buf;
+	int i, ret, n;
 
 	/* actual path to perf binary */
-	ret = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
-	if (ret <= 0)
-		return -1;
-
-	/* readlink() does not add null termination */
-	buf[ret] = '\0';
+	buf = perf_exe(pbuf, MAXPATHLEN);
 
 	/* account for binary path */
 	n = perf_env.nr_cmdline + 1;
tools/perf/util/util.c +10 -0
index 706818693086..d388f80d8703 100644
@@ -568,3 +568,13 @@ out:
 
 	return tip;
 }
+
+char *perf_exe(char *buf, int len)
+{
+	int n = readlink("/proc/self/exe", buf, len);
+	if (n > 0) {
+		buf[n] = 0;
+		return buf;
+	}
+	return strcpy(buf, "perf");
+}
tools/perf/util/util.h +2 -0
index 01c538027c6f..09c1b0f91f65 100644
@@ -77,6 +77,8 @@ extern bool perf_singlethreaded;
 void perf_set_singlethreaded(void);
 void perf_set_multithreaded(void);
 
+char *perf_exe(char *buf, int len);
+
 #ifndef O_CLOEXEC
 #ifdef __sparc__
 #define O_CLOEXEC      0x400000
author
#25 lore raw

PATCH 9/11 perf tools report: Support builtin perf script in scripts menu

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

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.

---
 tools/perf/ui/browsers/annotate.c |   2 +-
 tools/perf/ui/browsers/hists.c    |  20 +++---
 tools/perf/ui/browsers/scripts.c  | 109 +++++++++++++++++++++++++-----
 tools/perf/util/hist.h            |   8 ++-
 4 files changed, 112 insertions(+), 27 deletions(-)
tools/perf/ui/browsers/annotate.c +1 -1
index 35bdfd8b1e71..98d934a36d86 100644
@@ -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':
tools/perf/ui/browsers/hists.c +12 -8
index 2a4f363f8cc7..9db8daa20044 100644
@@ -2343,6 +2343,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);
 };
@@ -2565,7 +2566,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;
 }
@@ -2574,7 +2575,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) {
@@ -2592,6 +2593,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;
 }
@@ -2599,12 +2601,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")) {
@@ -2620,7 +2623,7 @@ add_script_opt(struct hist_browser *browser,
 				          tstr + j,
 				          sizeof tstr - j);
 		n += add_script_opt_2(browser, act, optstr, thread, sym,
-					  tstr);
+					  evsel, tstr);
 		act->time = he->time;
 	}
 	return n;
@@ -3091,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
@@ -3106,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:
tools/perf/ui/browsers/scripts.c +93 -16

144 lines not on this page · raw message

tools/perf/util/hist.h +6 -2
index 8b21b6d006be..51309f887cb1 100644
@@ -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;
 }
Signature · 1 line
2.17.2
#26 lore raw

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

Quoted 14 lines · Andi Kleen

From: Andi Kleen <[email protected]>

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.

Quoted 7 lines

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.

cannot compile this one:

ui/browsers/scripts.c: In function ‘script_browse’:
ui/browsers/scripts.c:118:35: error: ‘%s’ directive writing up to 1023 bytes into a region of size 248 [-Werror=format-overflow=]

   sprintf(script_name, "%s script %s", perf, script_args);
                                   ^~         ~~~~~~~~~~~
In file included from /usr/include/stdio.h:862,
                 from ui/browsers/../../util/color.h:5,
                 from ui/browsers/../../util/sort.h:8,
                 from ui/browsers/scripts.c:6:
/usr/include/bits/stdio2.h:33:10: note: ‘__builtin___sprintf_chk’ output 9 or more bytes (assuming 1032) into a destination of size 256
   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       __bos (__s), __fmt, __va_arg_pack ());

[jolsa@krava perf]$ gcc --version
gcc (GCC) 8.2.1 20181215 (Red Hat 8.2.1-6)

jirka

author
#27 lore raw

PATCH 10/11 perf tools: Add utility function to print ns time stamps

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

Add a utility function to print nanosecond timestamps.

---
 tools/perf/util/time-utils.c | 8 ++++++++
 tools/perf/util/time-utils.h | 1 +
 2 files changed, 9 insertions(+)
tools/perf/util/time-utils.c +8 -0
index 6193b46050a5..a63bdf4cfd1b 100644
@@ -404,6 +404,14 @@ int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz)
 	return scnprintf(buf, sz, "%"PRIu64".%06"PRIu64, sec, usec);
 }
 
+int timestamp__scnprintf_nsec(u64 timestamp, char *buf, size_t sz)
+{
+	u64  sec = timestamp / NSEC_PER_SEC;
+	u64 nsec = timestamp % NSEC_PER_SEC;
+
+	return scnprintf(buf, sz, "%"PRIu64".%09"PRIu64, sec, nsec);
+}
+
 int fetch_current_timestamp(char *buf, size_t sz)
 {
 	struct timeval tv;
tools/perf/util/time-utils.h +1 -0
index 70b177d2b98c..9266cf4a8e58 100644
@@ -24,6 +24,7 @@ bool perf_time__ranges_skip_sample(struct perf_time_interval *ptime_buf,
 				   int num, u64 timestamp);
 
 int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz);
+int timestamp__scnprintf_nsec(u64 timestamp, char *buf, size_t sz);
 
 int fetch_current_timestamp(char *buf, size_t sz);
 
Signature · 1 line
2.17.2
author
#28 lore raw

PATCH 11/11 perf tools report: Implement browsing of individual samples

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

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.

---
 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      | 74 ++++++++++++++++++++++++
 tools/perf/ui/browsers/scripts.c         |  2 +-
 tools/perf/util/hist.c                   | 36 ++++++++++++
 tools/perf/util/hist.h                   | 19 ++++++
 tools/perf/util/sort.h                   |  8 +++
 tools/perf/util/symbol.c                 |  1 +
 tools/perf/util/symbol_conf.h            |  1 +
 11 files changed, 194 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/ui/browsers/res_sample.c
tools/perf/Documentation/perf-report.txt +4 -0
index 546d87221ad8..f441baa794ce 100644
@@ -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.
 
tools/perf/builtin-report.c +2 -0
index 600e168f3b99..9ad9dfa431d1 100644
@@ -1154,6 +1154,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",
tools/perf/ui/browsers/Build +1 -0
index 8fee56b46502..fdf86f7981ca 100644
@@ -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
tools/perf/ui/browsers/hists.c +47 -0

71 lines not on this page · raw message

tools/perf/ui/browsers/res_sample.c new file +74 -0

74 lines not on this page · raw message

tools/perf/ui/browsers/scripts.c +1 -1
index 8633ad323fe9..3f018aefe118 100644
@@ -128,7 +128,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();
tools/perf/util/hist.c +36 -0
index 7bf4d85a3021..4af7caa87907 100644
@@ -594,6 +594,40 @@ 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->res_samples) {
+		he->res_samples = calloc(sizeof(struct res_sample),
+					symbol_conf.res_sample);
+		if (!he->res_samples)
+			return;
+	}
+	if (he->num_res < symbol_conf.res_sample) {
+		j = he->num_res++;
+	} else {
+		j = random_max(++he->num_res + 1);
+		if (he->num_res > symbol_conf.res_sample)
+			return;
+	}
+	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,
@@ -641,6 +675,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;
 }
 
tools/perf/util/hist.h +19 -0
index 51309f887cb1..2d4766508f31 100644
@@ -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,10 @@ 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);
 #else
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
@@ -488,6 +499,14 @@ 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;
+}
+
 #define K_LEFT  -1000
 #define K_RIGHT -2000
 #define K_SWITCH_INPUT_DATA -3000
tools/perf/util/sort.h +8 -0
index fc323f10f474..70c33add1443 100644
@@ -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;
tools/perf/util/symbol.c +1 -0
index 758bf5f74e6e..112c3492decf 100644
@@ -48,6 +48,7 @@ struct symbol_conf symbol_conf = {
 	.symfs			= "",
 	.event_group		= true,
 	.inline_name		= true,
+	.res_sample		= 0,
 };
 
 static enum dso_binary_type binary_type_symtab[] = {
tools/perf/util/symbol_conf.h +1 -0
index fffea68c1203..8d21f8fb9cfb 100644
@@ -66,6 +66,7 @@ struct symbol_conf {
 	struct intlist	*pid_list,
 			*tid_list;
 	const char	*symfs;
+	int		res_sample;
 };
 
 extern struct symbol_conf symbol_conf;
Signature · 1 line
2.17.2
#29 lore raw

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

On Sun, Feb 24, 2019 at 07:37:22AM -0800, Andi Kleen wrote:

SNIP

Quoted 34 lines
tools/perf/ui/browsers/res_sample.c new file +28 -0
new file mode 100644
index 000000000000..5b4807c29c2c
@@ -0,0 +1,74 @@
+// 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 "time-utils.h"
+
+/* In ns. Could make configurable. */
+#define CONTEXT_LEN 100000000
+
+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);
+

tools/perf/ui/browsers/res_sample.c line 38

Quoted 10 lines
tools/perf/ui/browsers/res_sample.c new file +10 -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)
+			return -1;

leaking names

jirka

Quoted 36 lines
tools/perf/ui/browsers/res_sample.c new file +36 -0
+	}
+	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 | less +/%s",
+		     perf,
+		     input_name ? "-i " : "",
+		     input_name ? input_name : "",
+		     trange,
+		     r->cpu ? "--cpu " : "",
+		     r->cpu ? (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" : "",
+		     tsample) < 0)
+		return -1;
+	run_script(cmd);
+	free(cmd);
+	return 0;
+}
#30 lore raw

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

In reply to Andi Kleen

On Sun, Feb 24, 2019 at 07:37:22AM -0800, Andi Kleen wrote:

SNIP

Quoted 5 lines
tools/perf/util/hist.c +5 -0
+static void hists__res_sample(struct hist_entry *he, struct perf_sample *sample)
+{
+	struct res_sample *r;
+	int j;
+

tools/perf/util/hist.c static struct hist_entry *hists__findnew_entry(struct hists *hists, · line 621

Quoted 10 lines
tools/perf/util/hist.c +10 -0
+	if (!he->res_samples) {
+		he->res_samples = calloc(sizeof(struct res_sample),
+					symbol_conf.res_sample);
+		if (!he->res_samples)
+			return;
+	}
+	if (he->num_res < symbol_conf.res_sample) {
+		j = he->num_res++;
+	} else {
+		j = random_max(++he->num_res + 1);

will he->num_res keep raising all the time?

jirka

Quoted 9 lines
tools/perf/util/hist.c +9 -0
+		if (he->num_res > symbol_conf.res_sample)
+			return;
+	}
+	r = &he->res_samples[j];
+	r->time = sample->time;
+	r->cpu = sample->cpu;
+	r->tid = sample->tid;
+}
+

SNIP

#31 lore raw

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

Quoted 13 lines · Jiri Olsa

On Sun, Feb 24, 2019 at 07:37:22AM -0800, Andi Kleen wrote:

SNIP

Quoted 9 lines
tools/perf/util/hist.c +9 -0
+static void hists__res_sample(struct hist_entry *he, struct perf_sample *sample)
+{
+	struct res_sample *r;
+	int j;
+
+	if (!he->res_samples) {
+		he->res_samples = calloc(sizeof(struct res_sample),
+					symbol_conf.res_sample);
+		if (!he->res_samples)
Quoted 8 lines

tools/perf/util/hist.c static struct hist_entry *hists__findnew_entry(struct hists *hists, · line 621

Quoted 6 lines
tools/perf/util/hist.c +6 -0
+			return;
+	}
+	if (he->num_res < symbol_conf.res_sample) {
+		j = he->num_res++;
+	} else {
+		j = random_max(++he->num_res + 1);

will he->num_res keep raising all the time?

Yes, but the display code limits it to symbol_conf.res_samples

That was intentional so that we can keep track of the total number of samples. Plan was to eventually display it somewhere so that the users know how many of the samples are covered (currently it is not though)

-Andi

#32 lore raw

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

Quoted 23 lines · Andi Kleen
Quoted 13 lines · Jiri Olsa

On Sun, Feb 24, 2019 at 07:37:22AM -0800, Andi Kleen wrote:

SNIP

Quoted 9 lines
tools/perf/util/hist.c +9 -0
+static void hists__res_sample(struct hist_entry *he, struct perf_sample *sample)
+{
+	struct res_sample *r;
+	int j;
+
+	if (!he->res_samples) {
+		he->res_samples = calloc(sizeof(struct res_sample),
+					symbol_conf.res_sample);
+		if (!he->res_samples)
Quoted 8 lines

tools/perf/util/hist.c static struct hist_entry *hists__findnew_entry(struct hists *hists, · line 621

Quoted 6 lines
tools/perf/util/hist.c +6 -0
+			return;
+	}
+	if (he->num_res < symbol_conf.res_sample) {
+		j = he->num_res++;
+	} else {
+		j = random_max(++he->num_res + 1);

will he->num_res keep raising all the time?

Quoted 6 lines

Yes, but the display code limits it to symbol_conf.res_samples

That was intentional so that we can keep track of the total number of samples. Plan was to eventually display it somewhere so that the users know how many of the samples are covered (currently it is not though)

if it's not use please change it, so it's not confusing also there's hist_entry::stat.nr_events for that

jirka

#33 lore raw

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

In reply to Andi Kleen

On Sun, Feb 24, 2019 at 07:37:22AM -0800, Andi Kleen wrote:

SNIP

Quoted 11 lines

static int
do_switch_data(struct hist_browser *browser __maybe_unused,

tools/perf/ui/browsers/hists.c +4 -0
 	       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);

tools/perf/ui/browsers/hists.c static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events, · line 3166

Quoted 10 lines
tools/perf/ui/browsers/hists.c +8 -0
+		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]);

for some reason can't see those items in menu

jirka

#34 lore raw

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

Quoted 1 line

for some reason can't see those items in menu

These one needs --samples 10 or similar.

It's off by default currently.

-Andi

#35 lore raw

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

Quoted 5 lines · Andi Kleen
Quoted 1 line

for some reason can't see those items in menu

These one needs --samples 10 or similar.

It's off by default currently.

I did, must be those hacks I put in to compile it, will check in new version

jirka