[PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Andi Kleen]

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

Save the original array dimensions in xyarrays, so that users can retrieve them
later. Add some inline functions to access these fields.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/xyarray.c |  2 ++
 tools/perf/util/xyarray.h | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/tools/perf/util/xyarray.c b/tools/perf/util/xyarray.c
index 7251fdbabced..c8f415d9877b 100644
--- a/tools/perf/util/xyarray.c
+++ b/tools/perf/util/xyarray.c
@@ -12,6 +12,8 @@ struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size)
 		xy->entry_size = entry_size;
 		xy->row_size   = row_size;
 		xy->entries    = xlen * ylen;
+		xy->max_x      = xlen;
+		xy->max_y      = ylen;
 	}
 
 	return xy;
diff --git a/tools/perf/util/xyarray.h b/tools/perf/util/xyarray.h
index 7f30af371b7e..785884158137 100644
--- a/tools/perf/util/xyarray.h
+++ b/tools/perf/util/xyarray.h
@@ -7,6 +7,8 @@ struct xyarray {
 	size_t row_size;
 	size_t entry_size;
 	size_t entries;
+	size_t max_x;
+	size_t max_y;
 	char contents[];
 };
 
@@ -19,4 +21,14 @@ static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
 	return &xy->contents[x * xy->row_size + y * xy->entry_size];
 }
 
+static inline int xy_max_y(struct xyarray *xy)
+{
+	return xy->max_x;
+}
+
+static inline int xy_max_x(struct xyarray *xy)
+{
+	return xy->max_y;
+}
+
 #endif /* _PERF_XYARRAY_H_ */
-- 
2.9.4

[PATCH v2 02/19] perf, tools: Fix buffer overflow while freeing events

[Andi Kleen]

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

Fix buffer overflow for

% perf stat -e msr/tsc/,cstate_core/c7-residency/ true

that causes glibc free list corruption. For some reason
it doesn't trigger in valgrind, but it is visible in AS:

=================================================================
==32681==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000003f5c at pc 0x0000005671ef bp 0x7ffdaaac9ac0 sp 0x7ffdaaac9ab0
READ of size 4 at 0x603000003f5c thread T0
    #0 0x5671ee in perf_evsel__close_fd util/evsel.c:1196
    #1 0x56c57a in perf_evsel__close util/evsel.c:1717
    #2 0x55ed5f in perf_evlist__close util/evlist.c:1631
    #3 0x4647e1 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:749
    #4 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767
    #5 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785
    #6 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296
    #7 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348
    #8 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392
    #9 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530
    #10 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    #11 0x428419 in _start (/home/ak/hle/obj-perf/perf+0x428419)

0x603000003f5c is located 0 bytes to the right of 28-byte region [0x603000003f40,0x603000003f5c)
allocated by thread T0 here:
    #0 0x7f0675139020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x648a2d in zalloc util/util.h:23
    #2 0x648a88 in xyarray__new util/xyarray.c:9
    #3 0x566419 in perf_evsel__alloc_fd util/evsel.c:1039
    #4 0x56b427 in perf_evsel__open util/evsel.c:1529
    #5 0x56c620 in perf_evsel__open_per_thread util/evsel.c:1730
    #6 0x461dea in create_perf_stat_counter /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:263
    #7 0x4637d7 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:600
    #8 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767
    #9 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785
    #10 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296
    #11 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348
    #12 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392
    #13 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530
    #14 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400)

The event is allocated with cpus == 1, but freed with cpus == real number
When the evsel close function walks the file descriptors it exceeds the
fd xyarray boundaries and reads random memory.

v2:
Now that xyarrays save their original dimensions we can use these
to iterate the two dimensional fd arrays. Fix some users
(close, ioctl) in evsel.c to use these fields directly. This allows simplifying
the code and dropping quite a few function arguments. Adjust
all callers by removing the unneeded arguments.

The actual perf event reading still uses the original values from
the evsel list.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/tests/openat-syscall-all-cpus.c |  2 +-
 tools/perf/tests/openat-syscall.c          |  2 +-
 tools/perf/util/evlist.c                   | 12 +++-------
 tools/perf/util/evsel.c                    | 37 ++++++++++--------------------
 tools/perf/util/evsel.h                    |  7 +++---
 5 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/tools/perf/tests/openat-syscall-all-cpus.c b/tools/perf/tests/openat-syscall-all-cpus.c
index 1a74dd9fd067..c7a57ba5b061 100644
--- a/tools/perf/tests/openat-syscall-all-cpus.c
+++ b/tools/perf/tests/openat-syscall-all-cpus.c
@@ -115,7 +115,7 @@ int test__openat_syscall_event_on_all_cpus(int subtest __maybe_unused)
 
 	perf_evsel__free_counts(evsel);
 out_close_fd:
-	perf_evsel__close_fd(evsel, 1, threads->nr);
+	perf_evsel__close_fd(evsel);
 out_evsel_delete:
 	perf_evsel__delete(evsel);
 out_thread_map_delete:
diff --git a/tools/perf/tests/openat-syscall.c b/tools/perf/tests/openat-syscall.c
index e44506e21ee7..121c7317bfc7 100644
--- a/tools/perf/tests/openat-syscall.c
+++ b/tools/perf/tests/openat-syscall.c
@@ -56,7 +56,7 @@ int test__openat_syscall_event(int subtest __maybe_unused)
 
 	err = 0;
 out_close_fd:
-	perf_evsel__close_fd(evsel, 1, threads->nr);
+	perf_evsel__close_fd(evsel);
 out_evsel_delete:
 	perf_evsel__delete(evsel);
 out_thread_map_delete:
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 078b58511595..6a0d7ffbeba0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1419,8 +1419,6 @@ int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **e
 {
 	struct perf_evsel *evsel;
 	int err = 0;
-	const int ncpus = cpu_map__nr(evlist->cpus),
-		  nthreads = thread_map__nr(evlist->threads);
 
 	evlist__for_each_entry(evlist, evsel) {
 		if (evsel->filter == NULL)
@@ -1430,7 +1428,7 @@ int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **e
 		 * filters only work for tracepoint event, which doesn't have cpu limit.
 		 * So evlist and evsel should always be same.
 		 */
-		err = perf_evsel__apply_filter(evsel, ncpus, nthreads, evsel->filter);
+		err = perf_evsel__apply_filter(evsel, evsel->filter);
 		if (err) {
 			*err_evsel = evsel;
 			break;
@@ -1623,13 +1621,9 @@ void perf_evlist__set_selected(struct perf_evlist *evlist,
 void perf_evlist__close(struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel;
-	int ncpus = cpu_map__nr(evlist->cpus);
-	int nthreads = thread_map__nr(evlist->threads);
 
-	evlist__for_each_entry_reverse(evlist, evsel) {
-		int n = evsel->cpus ? evsel->cpus->nr : ncpus;
-		perf_evsel__close(evsel, n, nthreads);
-	}
+	evlist__for_each_entry_reverse(evlist, evsel)
+		perf_evsel__close(evsel);
 }
 
 static int perf_evlist__create_syswide_maps(struct perf_evlist *evlist)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3735c9e0080d..c9351564ed94 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1051,16 +1051,13 @@ static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthread
 	return evsel->fd != NULL ? 0 : -ENOMEM;
 }
 
-static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthreads,
+static int perf_evsel__run_ioctl(struct perf_evsel *evsel,
 			  int ioc,  void *arg)
 {
 	int cpu, thread;
 
-	if (evsel->system_wide)
-		nthreads = 1;
-
-	for (cpu = 0; cpu < ncpus; cpu++) {
-		for (thread = 0; thread < nthreads; thread++) {
+	for (cpu = 0; cpu < xy_max_x(evsel->fd); cpu++) {
+		for (thread = 0; thread < xy_max_y(evsel->fd); thread++) {
 			int fd = FD(evsel, cpu, thread),
 			    err = ioctl(fd, ioc, arg);
 
@@ -1072,10 +1069,9 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthrea
 	return 0;
 }
 
-int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
-			     const char *filter)
+int perf_evsel__apply_filter(struct perf_evsel *evsel, const char *filter)
 {
-	return perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+	return perf_evsel__run_ioctl(evsel,
 				     PERF_EVENT_IOC_SET_FILTER,
 				     (void *)filter);
 }
@@ -1122,20 +1118,14 @@ int perf_evsel__append_addr_filter(struct perf_evsel *evsel, const char *filter)
 
 int perf_evsel__enable(struct perf_evsel *evsel)
 {
-	int nthreads = thread_map__nr(evsel->threads);
-	int ncpus = cpu_map__nr(evsel->cpus);
-
-	return perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+	return perf_evsel__run_ioctl(evsel,
 				     PERF_EVENT_IOC_ENABLE,
 				     0);
 }
 
 int perf_evsel__disable(struct perf_evsel *evsel)
 {
-	int nthreads = thread_map__nr(evsel->threads);
-	int ncpus = cpu_map__nr(evsel->cpus);
-
-	return perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+	return perf_evsel__run_ioctl(evsel,
 				     PERF_EVENT_IOC_DISABLE,
 				     0);
 }
@@ -1185,15 +1175,12 @@ static void perf_evsel__free_config_terms(struct perf_evsel *evsel)
 	}
 }
 
-void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
+void perf_evsel__close_fd(struct perf_evsel *evsel)
 {
 	int cpu, thread;
 
-	if (evsel->system_wide)
-		nthreads = 1;
-
-	for (cpu = 0; cpu < ncpus; cpu++)
-		for (thread = 0; thread < nthreads; ++thread) {
+	for (cpu = 0; cpu < xy_max_x(evsel->fd); cpu++)
+		for (thread = 0; thread < xy_max_y(evsel->fd); ++thread) {
 			close(FD(evsel, cpu, thread));
 			FD(evsel, cpu, thread) = -1;
 		}
@@ -1854,12 +1841,12 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 	return err;
 }
 
-void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads)
+void perf_evsel__close(struct perf_evsel *evsel)
 {
 	if (evsel->fd == NULL)
 		return;
 
-	perf_evsel__close_fd(evsel, ncpus, nthreads);
+	perf_evsel__close_fd(evsel);
 	perf_evsel__free_fd(evsel);
 }
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index de03c18daaf0..351d3b2d8887 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -226,7 +226,7 @@ const char *perf_evsel__group_name(struct perf_evsel *evsel);
 int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size);
 
 int perf_evsel__alloc_id(struct perf_evsel *evsel, int ncpus, int nthreads);
-void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
+void perf_evsel__close_fd(struct perf_evsel *evsel);
 
 void __perf_evsel__set_sample_bit(struct perf_evsel *evsel,
 				  enum perf_event_sample_format bit);
@@ -246,8 +246,7 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
 int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter);
 int perf_evsel__append_addr_filter(struct perf_evsel *evsel,
 				   const char *filter);
-int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
-			     const char *filter);
+int perf_evsel__apply_filter(struct perf_evsel *evsel, const char *filter);
 int perf_evsel__enable(struct perf_evsel *evsel);
 int perf_evsel__disable(struct perf_evsel *evsel);
 
@@ -257,7 +256,7 @@ int perf_evsel__open_per_thread(struct perf_evsel *evsel,
 				struct thread_map *threads);
 int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 		     struct thread_map *threads);
-void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads);
+void perf_evsel__close(struct perf_evsel *evsel);
 
 struct perf_sample;
 
-- 
2.9.4

[PATCH v2 03/19] perf, tools, stat: Fix saved values rbtree lookup

[Andi Kleen]

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

The stat shadow saved values rbtree is indexed by a pointer.
Fix the comparison function:

- We cannot return a pointer delta as an int because
that loses bits on 64bit.
- Doing pointer arithmetic on the struct pointer
only works if the objects are spaced by the multiple
of the object size, which is not guaranteed for individual
malloc'ed object

Replace it with a proper comparison.

This fixes various problems with values not being found.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/stat-shadow.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 719d6cb86952..a04cf56d3517 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -70,7 +70,11 @@ static int saved_value_cmp(struct rb_node *rb_node, const void *entry)
 		return a->ctx - b->ctx;
 	if (a->cpu != b->cpu)
 		return a->cpu - b->cpu;
-	return a->evsel - b->evsel;
+	if (a->evsel == b->evsel)
+		return 0;
+	if ((char *)a->evsel < (char *)b->evsel)
+		return -1;
+	return +1;
 }
 
 static struct rb_node *saved_value_new(struct rblist *rblist __maybe_unused,
-- 
2.9.4

[PATCH v2 04/19] perf, tools: Tighten detection of BPF events

[Andi Kleen]

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

perf stat -e cpu/uops_executed.core,cmask=1/

would be detected as a BPF source event because the .c matches the .c
source BPF pattern.

v2:

Originally I tried to use lex lookahead, but it doesn't seem to work.

This now extends the BPF pattern to match longer events, but then
does an extra check in the C code to reject BPF matches that
do not end with .c/.o/.obj

This uses REJECT, which makes the flex scanner slower, but
that shouldn't be a big problem for the perf events.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/parse-events.l | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 660fca05bc93..c42edeac451f 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -53,6 +53,21 @@ static int str(yyscan_t scanner, int token)
 	return token;
 }
 
+static bool isbpf(yyscan_t scanner)
+{
+	char *text = parse_events_get_text(scanner);
+	int len = strlen(text);
+
+	if (len < 2)
+		return false;
+	if ((text[len - 1] == 'c' || text[len - 1] == 'o') &&
+	    text[len - 2] == '.')
+		return true;
+	if (len > 4 && !strcmp(text + len - 4, ".obj"))
+		return true;
+	return false;
+}
+
 /*
  * This function is called when the parser gets two kind of input:
  *
@@ -136,8 +151,8 @@ do {							\
 group		[^,{}/]*[{][^}]*[}][^,{}/]*
 event_pmu	[^,{}/]+[/][^/]*[/][^,{}/]*
 event		[^,{}/]+
-bpf_object	[^,{}]+\.(o|bpf)
-bpf_source	[^,{}]+\.c
+bpf_object	[^,{}]+\.(o|bpf)[a-zA-Z0-9._]*
+bpf_source	[^,{}]+\.c[a-zA-Z0-9._]*
 
 num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
@@ -307,8 +322,8 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 {num_hex}		{ return value(yyscanner, 16); }
 
 {modifier_event}	{ return str(yyscanner, PE_MODIFIER_EVENT); }
-{bpf_object}		{ return str(yyscanner, PE_BPF_OBJECT); }
-{bpf_source}		{ return str(yyscanner, PE_BPF_SOURCE); }
+{bpf_object}		{ if (!isbpf(yyscanner)) REJECT; return str(yyscanner, PE_BPF_OBJECT); }
+{bpf_source}		{ if (!isbpf(yyscanner)) REJECT; return str(yyscanner, PE_BPF_SOURCE); }
 {name}			{ return pmu_str_check(yyscanner); }
 "/"			{ BEGIN(config); return '/'; }
 -			{ return '-'; }
-- 
2.9.4

[PATCH v2 05/19] perf, tools: Support weak groups

[Andi Kleen]

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

Setting up groups can be complicated due to the
complicated scheduling restrictions of different PMUs.
User tools usually don't understand all these restrictions.
Still in many cases it is useful to set up groups and
they work most of the time. However if the group
is set up wrong some members will not reported any values
because they never get scheduled.

Add a concept of a 'weak group': try to set up a group,
but if it's not schedulable fallback to not using
a group. That gives us the best of both worlds:
groups if they work, but still a usable fallback if they don't.

In theory it would be possible to have more complex fallback
strategies (e.g. try to split the group in half), but
the simple fallback of not using a group seems to work for now.

So far the weak group is only implemented for perf stat,
not for record.

Here's an unschedulable group (on IvyBridge with SMT on)

% perf stat -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}' -a sleep 1

        73,806,067      branches
         4,848,144      branch-misses             #    6.57% of all branches
        14,754,458      l1d.replacement
        24,905,558      l2_lines_in.all
   <not supported>      l2_rqsts.all_code_rd         <------- will never report anything

With the weak group:

% perf stat -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1

       125,366,055      branches                                                      (80.02%)
         9,208,402      branch-misses             #    7.35% of all branches          (80.01%)
        24,560,249      l1d.replacement                                               (80.00%)
        43,174,971      l2_lines_in.all                                               (80.05%)
        31,891,457      l2_rqsts.all_code_rd                                          (79.92%)

The extra event scheduled with some extra multiplexing

v2: Move fallback code to separate function.
Add comment on for_each_group_member
Adjust to new perf_evsel__close interface
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-list.txt |  1 +
 tools/perf/builtin-stat.c              | 35 ++++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.h                |  1 +
 tools/perf/util/parse-events.c         |  8 +++++++-
 tools/perf/util/parse-events.l         |  2 +-
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index f709de54707b..d432965d728d 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -47,6 +47,7 @@ counted. The following modifiers exist:
  P - use maximum detected precise level
  S - read sample value (PERF_SAMPLE_READ)
  D - pin the event to the PMU
+ W - group is weak and will fallback to non-group if not schedulable
 
 The 'p' modifier can be used for specifying how precise the instruction
 address should be. The 'p' modifier can be specified multiple times:
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 866da7aa54bf..ac605caf1a01 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
 	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
 }
 
+static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
+{
+	struct perf_evsel *c2, *leader;
+	bool is_open = true;
+
+	leader = counter->leader;
+	pr_debug("Weak group for %s/%d failed\n",
+			leader->name, counter->nr_members);
+
+	/*
+	 * for_each_group_member doesn't work here because it doesn't
+	 * include the first entry.
+	 */
+	evlist__for_each_entry(evsel_list, c2) {
+		if (c2 == counter)
+			is_open = false;
+		if (c2->leader == leader) {
+			if (is_open)
+				perf_evsel__close(c2);
+			c2->leader = c2;
+			c2->nr_members = 0;
+		}
+	}
+	return leader;
+}
+
 static int __run_perf_stat(int argc, const char **argv)
 {
 	int interval = stat_config.interval;
@@ -618,6 +644,15 @@ static int __run_perf_stat(int argc, const char **argv)
 	evlist__for_each_entry(evsel_list, counter) {
 try_again:
 		if (create_perf_stat_counter(counter) < 0) {
+
+			/* Weak group failed. Reset the group. */
+			if (errno == EINVAL &&
+			    counter->leader != counter &&
+			    counter->weak_group) {
+				counter = reset_weak_group(counter);
+				goto try_again;
+			}
+
 			/*
 			 * PPC returns ENXIO for HW counters until 2.6.37
 			 * (behavior changed with commit b0a873e).
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 351d3b2d8887..f538c3530227 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -136,6 +136,7 @@ struct perf_evsel {
 	const char *		metric_name;
 	struct perf_evsel	**metric_events;
 	bool			collect_stat;
+	bool			weak_group;
 };
 
 union u64_swap {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 84e301073885..cd89b5cba8d2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1358,6 +1358,7 @@ struct event_modifier {
 	int exclude_GH;
 	int sample_read;
 	int pinned;
+	int weak;
 };
 
 static int get_event_modifier(struct event_modifier *mod, char *str,
@@ -1376,6 +1377,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 
 	int exclude = eu | ek | eh;
 	int exclude_GH = evsel ? evsel->exclude_GH : 0;
+	int weak = 0;
 
 	memset(mod, 0, sizeof(*mod));
 
@@ -1413,6 +1415,8 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 			sample_read = 1;
 		} else if (*str == 'D') {
 			pinned = 1;
+		} else if (*str == 'W') {
+			weak = 1;
 		} else
 			break;
 
@@ -1443,6 +1447,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	mod->exclude_GH = exclude_GH;
 	mod->sample_read = sample_read;
 	mod->pinned = pinned;
+	mod->weak = weak;
 
 	return 0;
 }
@@ -1456,7 +1461,7 @@ static int check_modifier(char *str)
 	char *p = str;
 
 	/* The sizeof includes 0 byte as well. */
-	if (strlen(str) > (sizeof("ukhGHpppPSDI") - 1))
+	if (strlen(str) > (sizeof("ukhGHpppPSDIW") - 1))
 		return -1;
 
 	while (*p) {
@@ -1496,6 +1501,7 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add)
 		evsel->exclude_GH          = mod.exclude_GH;
 		evsel->sample_read         = mod.sample_read;
 		evsel->precise_max         = mod.precise_max;
+		evsel->weak_group	   = mod.weak;
 
 		if (perf_evsel__is_group_leader(evsel))
 			evsel->attr.pinned = mod.pinned;
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index c42edeac451f..fdb5bb52f01f 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -161,7 +161,7 @@ name		[a-zA-Z_*?][a-zA-Z0-9_*?.]*
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
 drv_cfg_term	[a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
 /* If you add a modifier you need to update check_modifier() */
-modifier_event	[ukhpPGHSDI]+
+modifier_event	[ukhpPGHSDIW]+
 modifier_bp	[rwx]{1,3}
 
 %%
-- 
2.9.4

[PATCH v2 06/19] perf, tools: Add missing newline to expr parser error messages

[Andi Kleen]

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

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/expr.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 954556bea36e..953e65ba2cc7 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -62,7 +62,7 @@ all_expr: expr			{ *final_val = $1; }
 
 expr:	  NUMBER
 	| ID			{ if (lookup_id(ctx, $1, &$$) < 0) {
-					pr_debug("%s not found", $1);
+					pr_debug("%s not found\n", $1);
 					YYABORT;
 				  }
 				}
-- 
2.9.4

[PATCH v2 07/19] perf, tools: Add utility function to detect SMT status

[Andi Kleen]

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

Add an smt_on() function to return if SMT is enabled or disabled.
Used in the next patch.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/Build |  1 +
 tools/perf/util/smt.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/smt.h |  6 ++++++
 3 files changed, 51 insertions(+)
 create mode 100644 tools/perf/util/smt.c
 create mode 100644 tools/perf/util/smt.h

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 8d49a989f193..94518c1bf8b6 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -22,6 +22,7 @@ libperf-y += rbtree.o
 libperf-y += libstring.o
 libperf-y += bitmap.o
 libperf-y += hweight.o
+libperf-y += smt.o
 libperf-y += quote.o
 libperf-y += strbuf.o
 libperf-y += string.o
diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
new file mode 100644
index 000000000000..453f6f6f29f3
--- /dev/null
+++ b/tools/perf/util/smt.c
@@ -0,0 +1,44 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <linux/bitops.h>
+#include "api/fs/fs.h"
+#include "smt.h"
+
+int smt_on(void)
+{
+	static bool cached;
+	static int cached_result;
+	int cpu;
+	int ncpu;
+
+	if (cached)
+		return cached_result;
+
+	ncpu = sysconf(_SC_NPROCESSORS_CONF);
+	for (cpu = 0; cpu < ncpu; cpu++) {
+		unsigned long long siblings;
+		char *str;
+		size_t strlen;
+		char fn[256];
+
+		snprintf(fn, sizeof fn,
+			"devices/system/cpu/cpu%d/topology/thread_siblings",
+			cpu);
+		if (sysfs__read_str(fn, &str, &strlen) < 0)
+			continue;
+		/* Entry is hex, but does not have 0x, so need custom parser */
+		siblings = strtoull(str, NULL, 16);
+		free(str);
+		if (hweight64(siblings) > 1) {
+			cached_result = 1;
+			cached = true;
+			break;
+		}
+	}
+	if (!cached) {
+		cached_result = 0;
+		cached = true;
+	}
+	return cached_result;
+}
diff --git a/tools/perf/util/smt.h b/tools/perf/util/smt.h
new file mode 100644
index 000000000000..b8414b7bcbc8
--- /dev/null
+++ b/tools/perf/util/smt.h
@@ -0,0 +1,6 @@
+#ifndef SMT_H
+#define SMT_H 1
+
+int smt_on(void);
+
+#endif
-- 
2.9.4

[PATCH v2 08/19] perf, tools: Expression parser enhancements for metrics

[Andi Kleen]

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

Enhance the expression parser for more complex metric formulas.

- Support python style IF ELSE operators
- Add an #SMT_On magic variable for formulas that depend on the SMT
status.

Example: 4 *( CPU_CLK_UNHALTED.THREAD_ANY / 2 ) if #SMT_on else cycles

- Support MIN/MAX operations

Example: min(1 , IDQ.MITE_UOPS / ( UPI * 16 * ( ICACHE.HIT + ICACHE.MISSES ) / 4.0 ) )

This is useful to fix up problems caused by multiplexing.

- Support | & ^ operators
- Minor cleanups and fixes
- Support an \ escape for operators. This allows to specify event names
like c2-residency
- Support @ as an alternative for / to be able to specify pmus without
conflicts with operators (like msr/tsc/ as msr@tsc@)

Example: (cstate_core@c3\\-residency@ / msr@tsc@) * 100

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/tests/expr.c |  5 ++++
 tools/perf/util/expr.y  | 61 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 6c6a3749aaf6..e75365098f23 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -31,6 +31,11 @@ int test__expr(int subtest __maybe_unused)
 	ret |= test(&ctx, "(BAR/2)%2", 1);
 	ret |= test(&ctx, "1 - -4",  5);
 	ret |= test(&ctx, "(FOO-1)*2 + (BAR/2)%2 - -4",  5);
+	ret |= test(&ctx, "1-1 | 1", 1);
+	ret |= test(&ctx, "1-1 & 1", 0);
+	ret |= test(&ctx, "min(1,2) + 1", 2);
+	ret |= test(&ctx, "max(1,2) + 1", 3);
+	ret |= test(&ctx, "1+1 if 3*4 else 0", 2);
 
 	if (ret)
 		return ret;
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 953e65ba2cc7..5753c4f21534 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -4,6 +4,7 @@
 #include "util/debug.h"
 #define IN_EXPR_Y 1
 #include "expr.h"
+#include "smt.h"
 #include <string.h>
 
 #define MAXIDLEN 256
@@ -22,13 +23,15 @@
 
 %token <num> NUMBER
 %token <id> ID
+%token MIN MAX IF ELSE SMT_ON
+%left MIN MAX IF
 %left '|'
 %left '^'
 %left '&'
 %left '-' '+'
 %left '*' '/' '%'
 %left NEG NOT
-%type <num> expr
+%type <num> expr if_expr
 
 %{
 static int expr__lex(YYSTYPE *res, const char **pp);
@@ -57,7 +60,12 @@ static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
 %}
 %%
 
-all_expr: expr			{ *final_val = $1; }
+all_expr: if_expr			{ *final_val = $1; }
+	;
+
+if_expr:
+	expr IF expr ELSE expr { $$ = $3 ? $1 : $5; }
+	| expr
 	;
 
 expr:	  NUMBER
@@ -66,13 +74,19 @@ expr:	  NUMBER
 					YYABORT;
 				  }
 				}
+	| expr '|' expr		{ $$ = (long)$1 | (long)$3; }
+	| expr '&' expr		{ $$ = (long)$1 & (long)$3; }
+	| expr '^' expr		{ $$ = (long)$1 ^ (long)$3; }
 	| expr '+' expr		{ $$ = $1 + $3; }
 	| expr '-' expr		{ $$ = $1 - $3; }
 	| expr '*' expr		{ $$ = $1 * $3; }
 	| expr '/' expr		{ if ($3 == 0) YYABORT; $$ = $1 / $3; }
 	| expr '%' expr		{ if ((long)$3 == 0) YYABORT; $$ = (long)$1 % (long)$3; }
 	| '-' expr %prec NEG	{ $$ = -$2; }
-	| '(' expr ')'		{ $$ = $2; }
+	| '(' if_expr ')'	{ $$ = $2; }
+	| MIN '(' expr ',' expr ')' { $$ = $3 < $5 ? $3 : $5; }
+	| MAX '(' expr ',' expr ')' { $$ = $3 > $5 ? $3 : $5; }
+	| SMT_ON		 { $$ = smt_on() > 0; }
 	;
 
 %%
@@ -82,13 +96,47 @@ static int expr__symbol(YYSTYPE *res, const char *p, const char **pp)
 	char *dst = res->id;
 	const char *s = p;
 
-	while (isalnum(*p) || *p == '_' || *p == '.') {
+	if (*p == '#')
+		*dst++ = *p++;
+
+	while (isalnum(*p) || *p == '_' || *p == '.' || *p == ':' || *p == '@' || *p == '\\') {
 		if (p - s >= MAXIDLEN)
 			return -1;
-		*dst++ = *p++;
+		/*
+		 * Allow @ instead of / to be able to specify pmu/event/ without
+		 * conflicts with normal division.
+		 */
+		if (*p == '@')
+			*dst++ = '/';
+		else if (*p == '\\')
+			*dst++ = *++p;
+		else
+			*dst++ = *p;
+		p++;
 	}
 	*dst = 0;
 	*pp = p;
+	dst = res->id;
+	switch (dst[0]) {
+	case 'm':
+		if (!strcmp(dst, "min"))
+			return MIN;
+		if (!strcmp(dst, "max"))
+			return MAX;
+		break;
+	case 'i':
+		if (!strcmp(dst, "if"))
+			return IF;
+		break;
+	case 'e':
+		if (!strcmp(dst, "else"))
+			return ELSE;
+		break;
+	case '#':
+		if (!strcasecmp(dst, "#smt_on"))
+			return SMT_ON;
+		break;
+	}
 	return ID;
 }
 
@@ -102,6 +150,7 @@ static int expr__lex(YYSTYPE *res, const char **pp)
 		p++;
 	s = p;
 	switch (*p++) {
+	case '#':
 	case 'a' ... 'z':
 	case 'A' ... 'Z':
 		return expr__symbol(res, p - 1, pp);
@@ -151,7 +200,7 @@ int expr__find_other(const char *p, const char *one, const char ***other,
 			err = 0;
 			break;
 		}
-		if (tok == ID && strcasecmp(one, val.id)) {
+		if (tok == ID && (!one || strcasecmp(one, val.id))) {
 			if (num_other >= EXPR_MAX_OTHER - 1) {
 				pr_debug("Too many extra events in %s\n", orig);
 				break;
-- 
2.9.4

[PATCH v2 09/19] perf, tools: Increase maximum number of events in expressions

[Andi Kleen]

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

Some of the upcoming metrics need more than 8 events. Increase the maximum
number the parser supports.

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

diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9c2760a1a96e..400ef9eab00a 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -1,7 +1,7 @@
 #ifndef PARSE_CTX_H
 #define PARSE_CTX_H 1
 
-#define EXPR_MAX_OTHER 8
+#define EXPR_MAX_OTHER 15
 #define MAX_PARSE_ID EXPR_MAX_OTHER
 
 struct parse_id {
-- 
2.9.4

[PATCH v2 10/19] perf, tools: Dedup events in expression parsing

[Andi Kleen]

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

Avoid adding redundant events while parsing an expression.
When we add an "other" event check first if it already exists.

v2: Fix perf test failure.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/expr.y | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 5753c4f21534..432b8560cf51 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -181,6 +181,19 @@ void expr__ctx_init(struct parse_ctx *ctx)
 	ctx->num_ids = 0;
 }
 
+static bool already_seen(const char *val, const char *one, const char **other,
+			 int num_other)
+{
+	int i;
+
+	if (one && !strcasecmp(one, val))
+		return true;
+	for (i = 0; i < num_other; i++)
+		if (!strcasecmp(other[i], val))
+			return true;
+	return false;
+}
+
 int expr__find_other(const char *p, const char *one, const char ***other,
 		     int *num_otherp)
 {
@@ -200,7 +213,7 @@ int expr__find_other(const char *p, const char *one, const char ***other,
 			err = 0;
 			break;
 		}
-		if (tok == ID && (!one || strcasecmp(one, val.id))) {
+		if (tok == ID && !already_seen(val.id, one, *other, num_other)) {
 			if (num_other >= EXPR_MAX_OTHER - 1) {
 				pr_debug("Too many extra events in %s\n", orig);
 				break;
-- 
2.9.4

[PATCH v2 11/19] perf, tools: Support metric_group and no event name in json parser

[Andi Kleen]

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

Some enhancements to the JSON parser to prepare for metrics support

- Parse the new MetricGroup field
- Support JSON events with no event name, that have only MetricName.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/pmu-events/jevents.c    | 24 ++++++++++++++++++------
 tools/perf/pmu-events/jevents.h    |  2 +-
 tools/perf/pmu-events/pmu-events.h |  1 +
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 2350f6099a46..8e7ced6a0517 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -290,7 +290,7 @@ static int print_events_table_entry(void *data, char *name, char *event,
 				    char *desc, char *long_desc,
 				    char *pmu, char *unit, char *perpkg,
 				    char *metric_expr,
-				    char *metric_name)
+				    char *metric_name, char *metric_group)
 {
 	struct perf_entry_data *pd = data;
 	FILE *outfp = pd->outfp;
@@ -302,8 +302,10 @@ static int print_events_table_entry(void *data, char *name, char *event,
 	 */
 	fprintf(outfp, "{\n");
 
-	fprintf(outfp, "\t.name = \"%s\",\n", name);
-	fprintf(outfp, "\t.event = \"%s\",\n", event);
+	if (name)
+		fprintf(outfp, "\t.name = \"%s\",\n", name);
+	if (event)
+		fprintf(outfp, "\t.event = \"%s\",\n", event);
 	fprintf(outfp, "\t.desc = \"%s\",\n", desc);
 	fprintf(outfp, "\t.topic = \"%s\",\n", topic);
 	if (long_desc && long_desc[0])
@@ -318,6 +320,8 @@ static int print_events_table_entry(void *data, char *name, char *event,
 		fprintf(outfp, "\t.metric_expr = \"%s\",\n", metric_expr);
 	if (metric_name)
 		fprintf(outfp, "\t.metric_name = \"%s\",\n", metric_name);
+	if (metric_group)
+		fprintf(outfp, "\t.metric_group = \"%s\",\n", metric_group);
 	fprintf(outfp, "},\n");
 
 	return 0;
@@ -355,6 +359,9 @@ static char *real_event(const char *name, char *event)
 {
 	int i;
 
+	if (!name)
+		return NULL;
+
 	for (i = 0; fixed[i].name; i++)
 		if (!strcasecmp(name, fixed[i].name))
 			return (char *)fixed[i].event;
@@ -367,7 +374,7 @@ int json_events(const char *fn,
 		      char *long_desc,
 		      char *pmu, char *unit, char *perpkg,
 		      char *metric_expr,
-		      char *metric_name),
+		      char *metric_name, char *metric_group),
 	  void *data)
 {
 	int err = -EIO;
@@ -395,6 +402,7 @@ int json_events(const char *fn,
 		char *unit = NULL;
 		char *metric_expr = NULL;
 		char *metric_name = NULL;
+		char *metric_group = NULL;
 		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
@@ -474,6 +482,8 @@ int json_events(const char *fn,
 				addfield(map, &perpkg, "", "", val);
 			} else if (json_streq(map, field, "MetricName")) {
 				addfield(map, &metric_name, "", "", val);
+			} else if (json_streq(map, field, "MetricGroup")) {
+				addfield(map, &metric_group, "", "", val);
 			} else if (json_streq(map, field, "MetricExpr")) {
 				addfield(map, &metric_expr, "", "", val);
 				for (s = metric_expr; *s; s++)
@@ -499,10 +509,11 @@ int json_events(const char *fn,
 			addfield(map, &event, ",", filter, NULL);
 		if (msr != NULL)
 			addfield(map, &event, ",", msr->pname, msrval);
-		fixname(name);
+		if (name)
+			fixname(name);
 
 		err = func(data, name, real_event(name, event), desc, long_desc,
-				pmu, unit, perpkg, metric_expr, metric_name);
+			   pmu, unit, perpkg, metric_expr, metric_name, metric_group);
 		free(event);
 		free(desc);
 		free(name);
@@ -514,6 +525,7 @@ int json_events(const char *fn,
 		free(unit);
 		free(metric_expr);
 		free(metric_name);
+		free(metric_group);
 		if (err)
 			break;
 		tok += j;
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index 611fac01913d..557994754410 100644
--- a/tools/perf/pmu-events/jevents.h
+++ b/tools/perf/pmu-events/jevents.h
@@ -6,7 +6,7 @@ int json_events(const char *fn,
 				char *long_desc,
 				char *pmu,
 				char *unit, char *perpkg, char *metric_expr,
-				char *metric_name),
+				char *metric_name, char *metric_group),
 		void *data);
 char *get_cpu_str(void);
 
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index 569eab3688dd..94fa1720f6fd 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -15,6 +15,7 @@ struct pmu_event {
 	const char *perpkg;
 	const char *metric_expr;
 	const char *metric_name;
+	const char *metric_group;
 };
 
 /*
-- 
2.9.4

[PATCH v2 12/19] perf, tools, stat: Factor out generic metric printing

[Andi Kleen]

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

The perf stat shadow metric printing already supports generic metrics.
Factor out the code doing that into a separate function that can be re-used
in a later patch.

No behavior changes.

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

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index a04cf56d3517..b6cbb6e67167 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -627,6 +627,46 @@ static void print_smi_cost(int cpu, struct perf_evsel *evsel,
 	out->print_metric(out->ctx, NULL, "%4.0f", "SMI#", smi_num);
 }
 
+static void generic_metric(const char *metric_expr,
+			   struct perf_evsel **metric_events,
+			   char *name,
+			   const char *metric_name,
+			   double avg,
+			   int cpu,
+			   int ctx,
+			   struct perf_stat_output_ctx *out)
+{
+	print_metric_t print_metric = out->print_metric;
+	struct parse_ctx pctx;
+	double ratio;
+	int i;
+	void *ctxp = out->ctx;
+
+	expr__ctx_init(&pctx);
+	expr__add_id(&pctx, name, avg);
+	for (i = 0; metric_events[i]; i++) {
+		struct saved_value *v;
+
+		v = saved_value_lookup(metric_events[i], cpu, ctx, false);
+		if (!v)
+			break;
+		expr__add_id(&pctx, metric_events[i]->name, avg_stats(&v->stats));
+	}
+	if (!metric_events[i]) {
+		const char *p = metric_expr;
+
+		if (expr__parse(&ratio, &pctx, &p) == 0)
+			print_metric(ctxp, NULL, "%8.1f",
+				metric_name ?
+				metric_name :
+			out->force_header ?  name : "",
+				ratio);
+		else
+			print_metric(ctxp, NULL, NULL, "", 0);
+	} else
+		print_metric(ctxp, NULL, NULL, "", 0);
+}
+
 void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 				   double avg, int cpu,
 				   struct perf_stat_output_ctx *out)
@@ -819,33 +859,8 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 		else
 			print_metric(ctxp, NULL, NULL, name, 0);
 	} else if (evsel->metric_expr) {
-		struct parse_ctx pctx;
-		int i;
-
-		expr__ctx_init(&pctx);
-		expr__add_id(&pctx, evsel->name, avg);
-		for (i = 0; evsel->metric_events[i]; i++) {
-			struct saved_value *v;
-
-			v = saved_value_lookup(evsel->metric_events[i], cpu, ctx, false);
-			if (!v)
-				break;
-			expr__add_id(&pctx, evsel->metric_events[i]->name,
-					     avg_stats(&v->stats));
-		}
-		if (!evsel->metric_events[i]) {
-			const char *p = evsel->metric_expr;
-
-			if (expr__parse(&ratio, &pctx, &p) == 0)
-				print_metric(ctxp, NULL, "%8.1f",
-					evsel->metric_name ?
-					evsel->metric_name :
-					out->force_header ?  evsel->name : "",
-					ratio);
-			else
-				print_metric(ctxp, NULL, NULL, "", 0);
-		} else
-			print_metric(ctxp, NULL, NULL, "", 0);
+		generic_metric(evsel->metric_expr, evsel->metric_events, evsel->name,
+				evsel->metric_name, avg, cpu, ctx, out);
 	} else if (runtime_nsecs_stats[cpu].n != 0) {
 		char unit = 'M';
 		char unit_buf[10];
-- 
2.9.4

[PATCH v2 13/19] perf, tools: Print generic metric header even for failed expressions

[Andi Kleen]

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

Print the generic metric header even when the expression evaluation
failed. Otherwise an expression that fails on the first collections
due to division by zero may suddenly reappear later without
an header.

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

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index b6cbb6e67167..4ec2d8d15344 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -662,7 +662,9 @@ static void generic_metric(const char *metric_expr,
 			out->force_header ?  name : "",
 				ratio);
 		else
-			print_metric(ctxp, NULL, NULL, "", 0);
+			print_metric(ctxp, NULL, NULL,
+				     out->force_header ?
+				     (metric_name ? metric_name : name) : "", 0);
 	} else
 		print_metric(ctxp, NULL, NULL, "", 0);
 }
-- 
2.9.4

[PATCH v2 14/19] perf, tools, stat: Support JSON metrics in perf stat

[Andi Kleen]

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

Add generic support for standalone metrics specified in JSON files
to perf stat. A metric is a formula that uses multiple events
to compute a higher level result (e.g. IPC).

Previously metrics were always tied to an event and automatically
enabled with that event. But now change it that we can have
standalone metrics. They are in the same JSON data structure
as events, but don't have an event name.

We also allow to organize the metrics in metric groups, which
allows a short cut to select several related metrics at once.

Add a new -M / --metrics option to perf stat that adds the metrics
or metric groups specified.

Add the core code to manage and parse the metric groups. They
are collected from the JSON data structures into a separate rblist.
When computing shadow values look for metrics in that list.
Then they are computed using the existing saved values infrastructure
in stat-shadow.c

The actual JSON metrics are in a separate pull request.

% perf stat -M Summary --metric-only -a sleep 1

 Performance counter stats for 'system wide':

Instructions                              CLKS                 CPU_Utilization      GFLOPs               SMT_2T_Utilization   Kernel_Utilization
317614222.0                              1392930775.0             0.0                 0.0                 0.2                 0.1

       1.001497549 seconds time elapsed

% perf stat -M GFLOPs flops

 Performance counter stats for 'flops':

     3,999,541,471      fp_comp_ops_exe.sse_scalar_single #      1.2 GFLOPs                   (66.65%)
                14      fp_comp_ops_exe.sse_scalar_double                                     (66.65%)
                 0      fp_comp_ops_exe.sse_packed_double                                     (66.67%)
                 0      fp_comp_ops_exe.sse_packed_single                                     (66.70%)
                 0      simd_fp_256.packed_double                                     (66.70%)
                 0      simd_fp_256.packed_single                                     (66.67%)
                 0      duration_time

       3.238372845 seconds time elapsed

v2: Add missing header file
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-stat.txt |   7 +
 tools/perf/builtin-stat.c              |  18 +-
 tools/perf/util/Build                  |   1 +
 tools/perf/util/metricgroup.c          | 334 +++++++++++++++++++++++++++++++++
 tools/perf/util/metricgroup.h          |  31 +++
 tools/perf/util/pmu.c                  |  31 +--
 tools/perf/util/pmu.h                  |   2 +
 tools/perf/util/stat-shadow.c          |  22 ++-
 tools/perf/util/stat.h                 |   4 +-
 9 files changed, 434 insertions(+), 16 deletions(-)
 create mode 100644 tools/perf/util/metricgroup.c
 create mode 100644 tools/perf/util/metricgroup.h

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 698076313606..be8935717678 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -199,6 +199,13 @@ Aggregate counts per processor socket for system-wide mode measurements.
 --per-core::
 Aggregate counts per physical processor for system-wide mode measurements.
 
+-M::
+--metrics::
+Print metrics or metricgroups specified in a comma separated list.
+For a group all metrics from the group are added.
+The events from the metrics are automatically measured.
+See perf list output for the possble metrics and metricgroups.
+
 -A::
 --no-aggr::
 Do not aggregate counts across all monitored CPUs.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index ac605caf1a01..830fd72a7fd0 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -65,6 +65,7 @@
 #include "util/tool.h"
 #include "util/group.h"
 #include "util/string2.h"
+#include "util/metricgroup.h"
 #include "asm/bug.h"
 
 #include <linux/time64.h>
@@ -133,6 +134,8 @@ static const char *smi_cost_attrs = {
 
 static struct perf_evlist	*evsel_list;
 
+static struct rblist		 metric_events;
+
 static struct target target = {
 	.uid	= UINT_MAX,
 };
@@ -1234,7 +1237,7 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval,
 
 	perf_stat__print_shadow_stats(counter, uval,
 				first_shadow_cpu(counter, id),
-				&out);
+				&out, &metric_events);
 	if (!csv_output && !metric_only) {
 		print_noise(counter, noise);
 		print_running(run, ena);
@@ -1565,7 +1568,8 @@ static void print_metric_headers(const char *prefix, bool no_indent)
 		os.evsel = counter;
 		perf_stat__print_shadow_stats(counter, 0,
 					      0,
-					      &out);
+					      &out,
+					      &metric_events);
 	}
 	fputc('\n', stat_config.output);
 }
@@ -1789,6 +1793,13 @@ static int enable_metric_only(const struct option *opt __maybe_unused,
 	return 0;
 }
 
+static int parse_metric_groups(const struct option *opt,
+			       const char *str,
+			       int unset __maybe_unused)
+{
+	return metricgroup__parse_groups(opt, str, &metric_events);
+}
+
 static const struct option stat_options[] = {
 	OPT_BOOLEAN('T', "transaction", &transaction_run,
 		    "hardware transaction statistics"),
@@ -1854,6 +1865,9 @@ static const struct option stat_options[] = {
 			"measure topdown level 1 statistics"),
 	OPT_BOOLEAN(0, "smi-cost", &smi_cost,
 			"measure SMI cost"),
+	OPT_CALLBACK('M', "metrics", &evsel_list, "metric/metric group list",
+		     "monitor specified metrics or metric groups (separated by ,)",
+		     parse_metric_groups),
 	OPT_END()
 };
 
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 94518c1bf8b6..71ab8466714d 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -34,6 +34,7 @@ libperf-y += dso.o
 libperf-y += symbol.o
 libperf-y += symbol_fprintf.o
 libperf-y += color.o
+libperf-y += metricgroup.o
 libperf-y += header.o
 libperf-y += callchain.o
 libperf-y += values.o
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
new file mode 100644
index 000000000000..6c1137125904
--- /dev/null
+++ b/tools/perf/util/metricgroup.c
@@ -0,0 +1,334 @@
+/*
+ * Copyright (c) 2017, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+/* Manage metrics and groups of metrics from JSON files */
+
+#include "metricgroup.h"
+#include "evlist.h"
+#include "strbuf.h"
+#include "pmu.h"
+#include "expr.h"
+#include "rblist.h"
+#include "pmu.h"
+#include <string.h>
+#include <stdbool.h>
+#include <errno.h>
+#include "pmu-events/pmu-events.h"
+#include "strbuf.h"
+#include "strlist.h"
+#include <assert.h>
+#include <ctype.h>
+
+struct metric_event *metricgroup__lookup(struct rblist *metric_events,
+					 struct perf_evsel *evsel,
+					 bool create)
+{
+	struct rb_node *nd;
+	struct metric_event me = {
+		.evsel = evsel
+	};
+	nd = rblist__find(metric_events, &me);
+	if (nd)
+		return container_of(nd, struct metric_event, nd);
+	if (create) {
+		rblist__add_node(metric_events, &me);
+		nd = rblist__find(metric_events, &me);
+		if (nd)
+			return container_of(nd, struct metric_event, nd);
+	}
+	return NULL;
+}
+
+static int metric_event_cmp(struct rb_node *rb_node, const void *entry)
+{
+	struct metric_event *a = container_of(rb_node,
+					      struct metric_event,
+					      nd);
+	const struct metric_event *b = entry;
+
+	if (a->evsel == b->evsel)
+		return 0;
+	if ((char *)a->evsel < (char *)b->evsel)
+		return -1;
+	return +1;
+}
+
+static struct rb_node *metric_event_new(struct rblist *rblist __maybe_unused,
+					const void *entry)
+{
+	struct metric_event *me = malloc(sizeof(struct metric_event));
+
+	if (!me)
+		return NULL;
+	memcpy(me, entry, sizeof(struct metric_event));
+	me->evsel = ((struct metric_event *)entry)->evsel;
+	INIT_LIST_HEAD(&me->head);
+	return &me->nd;
+}
+
+static void metricgroup__rblist_init(struct rblist *metric_events)
+{
+	rblist__init(metric_events);
+	metric_events->node_cmp = metric_event_cmp;
+	metric_events->node_new = metric_event_new;
+}
+
+struct egroup {
+	struct list_head nd;
+	int idnum;
+	const char **ids;
+	const char *metric_name;
+	const char *metric_expr;
+};
+
+static struct perf_evsel *find_evsel(struct perf_evlist *perf_evlist,
+				     const char **ids,
+				     int idnum,
+				     struct perf_evsel **metric_events)
+{
+	struct perf_evsel *ev, *start = NULL;
+	int ind = 0;
+
+	evlist__for_each_entry (perf_evlist, ev) {
+		if (!strcmp(ev->name, ids[ind])) {
+			metric_events[ind] = ev;
+			if (ind == 0)
+				start = ev;
+			if (++ind == idnum) {
+				metric_events[ind] = NULL;
+				return start;
+			}
+		} else {
+			ind = 0;
+			start = NULL;
+		}
+	}
+	/*
+	 * This can happen when an alias expands to multiple
+	 * events, like for uncore events.
+	 * We don't support this case for now.
+	 */
+	return NULL;
+}
+
+static int metricgroup__setup_events(struct list_head *groups,
+				     struct perf_evlist *perf_evlist,
+				     struct rblist *metric_events_list)
+{
+	struct metric_event *me;
+	struct metric_expr *expr;
+	int i = 0;
+	int ret = 0;
+	struct egroup *eg;
+	struct perf_evsel *evsel;
+
+	list_for_each_entry (eg, groups, nd) {
+		struct perf_evsel **metric_events;
+
+		metric_events = calloc(sizeof(void *), eg->idnum + 1);
+		if (!metric_events) {
+			ret = -ENOMEM;
+			break;
+		}
+		evsel = find_evsel(perf_evlist, eg->ids, eg->idnum,
+				   metric_events);
+		if (!evsel) {
+			pr_debug("Cannot resolve %s: %s\n",
+					eg->metric_name, eg->metric_expr);
+			continue;
+		}
+		for (i = 0; i < eg->idnum; i++)
+			metric_events[i]->collect_stat = true;
+		me = metricgroup__lookup(metric_events_list, evsel, true);
+		if (!me) {
+			ret = -ENOMEM;
+			break;
+		}
+		expr = malloc(sizeof(struct metric_expr));
+		if (!expr) {
+			ret = -ENOMEM;
+			break;
+		}
+		expr->metric_expr = eg->metric_expr;
+		expr->metric_name = eg->metric_name;
+		expr->metric_events = metric_events;
+		list_add(&expr->nd, &me->head);
+	}
+	return ret;
+}
+
+static bool match_metric(const char *n, const char *list)
+{
+	int len;
+	char *m;
+
+	if (!list)
+		return false;
+	if (!strcmp(list, "all"))
+		return true;
+	if (!n)
+		return !strcasecmp(list, "No_group");
+	len = strlen(list);
+	m = strcasestr(n, list);
+	if (!m)
+		return false;
+	if ((m == n || m[-1] == ';' || m[-1] == ' ') &&
+	    (m[len] == 0 || m[len] == ';'))
+		return true;
+	return false;
+}
+
+static struct pmu_events_map *find_map(void)
+{
+	struct pmu_events_map *map;
+	char *cpuid = perf_pmu__getcpuid();
+	int i;
+
+	i = 0;
+	for (;;) {
+		map = &pmu_events_map[i++];
+		if (!map->table) {
+			map = NULL;
+			break;
+		}
+
+		if (!strcmp(map->cpuid, cpuid))
+			break;
+	}
+	free(cpuid);
+	return map;
+}
+
+static int metricgroup__add_metric(const char *metric, struct strbuf *events,
+				   struct list_head *group_list)
+{
+	struct pmu_events_map *map = find_map();
+	struct pmu_event *pe;
+	int ret = -EINVAL;
+	int i, j;
+
+	strbuf_init(events, 100);
+	strbuf_addf(events, "%s", "");
+
+	if (!map)
+		return 0;
+
+	for (i = 0; ; i++) {
+		pe = &map->table[i];
+
+		if (!pe->name && !pe->metric_group && !pe->metric_name)
+			break;
+		if (!pe->metric_expr)
+			continue;
+		if (match_metric(pe->metric_group, metric) ||
+		    match_metric(pe->metric_name, metric)) {
+			const char **ids;
+			int idnum;
+			struct egroup *eg;
+
+			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
+
+			if (expr__find_other(pe->metric_expr,
+					     NULL, &ids, &idnum) < 0)
+				continue;
+			if (events->len > 0)
+				strbuf_addf(events, ",");
+			for (j = 0; j < idnum; j++) {
+				pr_debug("found event %s\n", ids[j]);
+				strbuf_addf(events, "%s%s",
+					j == 0 ? "{" : ",",
+					ids[j]);
+			}
+			strbuf_addf(events, "}:W");
+
+			eg = malloc(sizeof(struct egroup));
+			if (!eg) {
+				ret = -ENOMEM;
+				break;
+			}
+			eg->ids = ids;
+			eg->idnum = idnum;
+			eg->metric_name = pe->metric_name;
+			eg->metric_expr = pe->metric_expr;
+			list_add_tail(&eg->nd, group_list);
+			ret = 0;
+		}
+	}
+	return ret;
+}
+
+static int metricgroup__add_metric_list(const char *list, struct strbuf *events,
+				        struct list_head *group_list)
+{
+	char *llist, *nlist, *p;
+	int ret = -EINVAL;
+
+	nlist = strdup(list);
+	if (!nlist)
+		return -ENOMEM;
+	llist = nlist;
+	while ((p = strsep(&llist, ",")) != NULL) {
+		ret = metricgroup__add_metric(p, events, group_list);
+		if (ret == -EINVAL) {
+			fprintf(stderr, "Cannot find metric or group `%s'\n",
+					p);
+			break;
+		}
+	}
+	free(nlist);
+	return ret;
+}
+
+static void metricgroup__free_egroups(struct list_head *group_list)
+{
+	struct egroup *eg, *egtmp;
+	int i;
+
+	list_for_each_entry_safe (eg, egtmp, group_list, nd) {
+		for (i = 0; i < eg->idnum; i++)
+			free((char *)eg->ids[i]);
+		free(eg->ids);
+		free(eg);
+	}
+}
+
+int metricgroup__parse_groups(const struct option *opt,
+			   const char *str,
+			   struct rblist *metric_events)
+{
+	struct parse_events_error parse_error;
+	struct perf_evlist *perf_evlist = *(struct perf_evlist **)opt->value;
+	struct strbuf extra_events;
+	LIST_HEAD(group_list);
+	int ret;
+
+	if (metric_events->nr_entries == 0)
+		metricgroup__rblist_init(metric_events);
+	ret = metricgroup__add_metric_list(str, &extra_events, &group_list);
+	if (ret)
+		return ret;
+	pr_debug("adding %s\n", extra_events.buf);
+	memset(&parse_error, 0, sizeof(struct parse_events_error));
+	ret = parse_events(perf_evlist, extra_events.buf, &parse_error);
+	if (ret) {
+		pr_err("Cannot set up events %s\n", extra_events.buf);
+		goto out;
+	}
+	strbuf_release(&extra_events);
+	ret = metricgroup__setup_events(&group_list, perf_evlist,
+					metric_events);
+out:
+	metricgroup__free_egroups(&group_list);
+	return ret;
+}
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
new file mode 100644
index 000000000000..06854e125ee7
--- /dev/null
+++ b/tools/perf/util/metricgroup.h
@@ -0,0 +1,31 @@
+#ifndef METRICGROUP_H
+#define METRICGROUP_H 1
+
+#include "linux/list.h"
+#include "rblist.h"
+#include <subcmd/parse-options.h>
+#include "evlist.h"
+#include "strbuf.h"
+
+struct metric_event {
+	struct rb_node nd;
+	struct perf_evsel *evsel;
+	struct list_head head; /* list of metric_expr */
+};
+
+struct metric_expr {
+	struct list_head nd;
+	const char *metric_expr;
+	const char *metric_name;
+	struct perf_evsel **metric_events;
+};
+
+struct metric_event *metricgroup__lookup(struct rblist *metric_events,
+					 struct perf_evsel *evsel,
+					 bool create);
+int metricgroup__parse_groups(const struct option *opt,
+			const char *str,
+			struct rblist *metric_events);
+
+void metricgroup__print(bool metrics, bool groups, char *filter, bool raw);
+#endif
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ac16a9db1fb5..4f049285e3ac 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -516,16 +516,8 @@ char * __weak get_cpuid_str(void)
 	return NULL;
 }
 
-/*
- * From the pmu_events_map, find the table of PMU events that corresponds
- * to the current running CPU. Then, add all PMU events from that table
- * as aliases.
- */
-static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
+char *perf_pmu__getcpuid(void)
 {
-	int i;
-	struct pmu_events_map *map;
-	struct pmu_event *pe;
 	char *cpuid;
 	static bool printed;
 
@@ -535,12 +527,26 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 	if (!cpuid)
 		cpuid = get_cpuid_str();
 	if (!cpuid)
-		return;
+		return NULL;
 
 	if (!printed) {
 		pr_debug("Using CPUID %s\n", cpuid);
 		printed = true;
 	}
+	return cpuid;
+}
+
+/*
+ * From the pmu_events_map, find the table of PMU events that corresponds
+ * to the current running CPU. Then, add all PMU events from that table
+ * as aliases.
+ */
+static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
+{
+	int i;
+	struct pmu_events_map *map;
+	struct pmu_event *pe;
+	char *cpuid = perf_pmu__getcpuid();
 
 	i = 0;
 	while (1) {
@@ -560,8 +566,11 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 		const char *pname;
 
 		pe = &map->table[i++];
-		if (!pe->name)
+		if (!pe->name) {
+			if (pe->metric_group || pe->metric_name)
+				continue;
 			break;
+		}
 
 		pname = pe->pmu ? pe->pmu : "cpu";
 		if (strncmp(pname, name, strlen(pname)))
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 389e9729331f..b4c807fd4761 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -90,4 +90,6 @@ int perf_pmu__test(void);
 
 struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu);
 
+char *perf_pmu__getcpuid(void);
+
 #endif /* __PMU_H */
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 4ec2d8d15344..49b9931a7f63 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -6,6 +6,7 @@
 #include "rblist.h"
 #include "evlist.h"
 #include "expr.h"
+#include "metricgroup.h"
 
 enum {
 	CTX_BIT_USER	= 1 << 0,
@@ -671,13 +672,16 @@ static void generic_metric(const char *metric_expr,
 
 void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 				   double avg, int cpu,
-				   struct perf_stat_output_ctx *out)
+				   struct perf_stat_output_ctx *out,
+				   struct rblist *metric_events)
 {
 	void *ctxp = out->ctx;
 	print_metric_t print_metric = out->print_metric;
 	double total, ratio = 0.0, total2;
 	const char *color = NULL;
 	int ctx = evsel_context(evsel);
+	struct metric_event *me;
+	int num = 1;
 
 	if (perf_evsel__match(evsel, HARDWARE, HW_INSTRUCTIONS)) {
 		total = avg_stats(&runtime_cycles_stats[ctx][cpu]);
@@ -880,6 +884,20 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 	} else if (perf_stat_evsel__is(evsel, SMI_NUM)) {
 		print_smi_cost(cpu, evsel, out);
 	} else {
-		print_metric(ctxp, NULL, NULL, NULL, 0);
+		num = 0;
 	}
+
+	if ((me = metricgroup__lookup(metric_events, evsel, false)) != NULL) {
+		struct metric_expr *mexp;
+
+		list_for_each_entry (mexp, &me->head, nd) {
+			if (num++ > 0)
+				out->new_line(ctxp);
+			generic_metric(mexp->metric_expr, mexp->metric_events,
+					evsel->name, mexp->metric_name,
+					avg, cpu, ctx, out);
+		}
+	}
+	if (num == 0)
+		print_metric(ctxp, NULL, NULL, NULL, 0);
 }
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index eacaf958e19d..47915df346fb 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -91,9 +91,11 @@ struct perf_stat_output_ctx {
 	bool force_header;
 };
 
+struct rblist;
 void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 				   double avg, int cpu,
-				   struct perf_stat_output_ctx *out);
+				   struct perf_stat_output_ctx *out,
+				   struct rblist *metric_events);
 void perf_stat__collect_metric_expr(struct perf_evlist *);
 
 int perf_evlist__alloc_stats(struct perf_evlist *evlist, bool alloc_raw);
-- 
2.9.4

[PATCH v2 15/19] perf, tools, list: Add metric groups to perf list

[Andi Kleen]

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

Add code to perf list to print metric groups, and metrics
that don't have an event name. The metricgroup code collects
the eventgroups and events into a rblist, and then prints
them according to the configured filters.

The metricgroups are printed by default, but can be
limited by perf list metric or perf list metricgroup

% perf list metricgroup
..
Metric Groups:

DSB:
  DSB_Coverage
        [Fraction of Uops delivered by the DSB (aka Decoded Icache; or Uop Cache)]
FLOPS:
  GFLOPs
        [Giga Floating Point Operations Per Second]
Frontend:
  IFetch_Line_Utilization
        [Rough Estimation of fraction of fetched lines bytes that were likely consumed by program instructions]
Frontend_Bandwidth:
  DSB_Coverage
        [Fraction of Uops delivered by the DSB (aka Decoded Icache; or Uop Cache)]
Memory_BW:
  MLP
        [Memory-Level-Parallelism (average number of L1 miss demand load when there is at least 1 such miss)]

v2: Check return value of asprintf to fix warning on FC26
Fix key in lookup/addition for the groups list
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-list.txt |   7 +-
 tools/perf/builtin-list.c              |   7 ++
 tools/perf/util/metricgroup.c          | 176 +++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.c         |   3 +
 4 files changed, 192 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index d432965d728d..ca6369fd83c1 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -8,7 +8,8 @@ perf-list - List all symbolic event types
 SYNOPSIS
 --------
 [verse]
-'perf list' [--no-desc] [--long-desc] [hw|sw|cache|tracepoint|pmu|sdt|event_glob]
+'perf list' [--no-desc] [--long-desc]
+            [hw|sw|cache|tracepoint|pmu|sdt|metric|metricgroup|event_glob]
 
 DESCRIPTION
 -----------
@@ -247,6 +248,10 @@ To limit the list use:
 
 . 'sdt' to list all Statically Defined Tracepoint events.
 
+. 'metric' to list metrics
+
+. 'metricgroup' to list metricgroups with metrics.
+
 . If none of the above is matched, it will apply the supplied glob to all
   events, printing the ones that match.
 
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 4bf2cb4d25aa..b2d2ad3dd478 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -15,6 +15,7 @@
 #include "util/cache.h"
 #include "util/pmu.h"
 #include "util/debug.h"
+#include "util/metricgroup.h"
 #include <subcmd/parse-options.h>
 
 static bool desc_flag = true;
@@ -79,6 +80,10 @@ int cmd_list(int argc, const char **argv)
 						long_desc_flag, details_flag);
 		else if (strcmp(argv[i], "sdt") == 0)
 			print_sdt_events(NULL, NULL, raw_dump);
+		else if (strcmp(argv[i], "metric") == 0)
+			metricgroup__print(true, false, NULL, raw_dump);
+		else if (strcmp(argv[i], "metricgroup") == 0)
+			metricgroup__print(false, true, NULL, raw_dump);
 		else if ((sep = strchr(argv[i], ':')) != NULL) {
 			int sep_idx;
 
@@ -96,6 +101,7 @@ int cmd_list(int argc, const char **argv)
 			s[sep_idx] = '\0';
 			print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
 			print_sdt_events(s, s + sep_idx + 1, raw_dump);
+			metricgroup__print(true, true, s, raw_dump);
 			free(s);
 		} else {
 			if (asprintf(&s, "*%s*", argv[i]) < 0) {
@@ -112,6 +118,7 @@ int cmd_list(int argc, const char **argv)
 						details_flag);
 			print_tracepoint_events(NULL, s, raw_dump);
 			print_sdt_events(NULL, s, raw_dump);
+			metricgroup__print(true, true, NULL, raw_dump);
 			free(s);
 		}
 	}
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6c1137125904..4ec644aa1679 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -210,6 +210,182 @@ static struct pmu_events_map *find_map(void)
 	return map;
 }
 
+struct mep {
+	struct rb_node nd;
+	const char *name;
+	struct strlist *metrics;
+};
+
+static int mep_cmp(struct rb_node *rb_node, const void *entry)
+{
+	struct mep *a = container_of(rb_node, struct mep, nd);
+	struct mep *b = (struct mep *)entry;
+
+	return strcmp(a->name, b->name);
+}
+
+static struct rb_node *mep_new(struct rblist *rl __maybe_unused,
+					const void *entry)
+{
+	struct mep *me = malloc(sizeof(struct mep));
+
+	if (!me)
+		return NULL;
+	memcpy(me, entry, sizeof(struct mep));
+	me->name = strdup(me->name);
+	if (!me->name)
+		goto out_me;
+	me->metrics = strlist__new(NULL, NULL);
+	if (!me->metrics)
+		goto out_name;
+	return &me->nd;
+out_name:
+	free((char *)me->name);
+out_me:
+	free(me);
+	return NULL;
+}
+
+static struct mep *mep_lookup(struct rblist *groups, const char *name)
+{
+	struct rb_node *nd;
+	struct mep me = {
+		.name = name
+	};
+	nd = rblist__find(groups, &me);
+	if (nd)
+		return container_of(nd, struct mep, nd);
+	rblist__add_node(groups, &me);
+	nd = rblist__find(groups, &me);
+	if (nd)
+		return container_of(nd, struct mep, nd);
+	return NULL;
+}
+
+static void mep_delete(struct rblist *rl __maybe_unused,
+		       struct rb_node *nd)
+{
+	struct mep *me = container_of(nd, struct mep, nd);
+
+	strlist__delete(me->metrics);
+	free((void *)me->name);
+	free(me);
+}
+
+static void metricgroup__print_strlist(struct strlist *metrics, bool raw)
+{
+	struct str_node *sn;
+	int n = 0;
+
+	strlist__for_each_entry (sn, metrics) {
+		if (raw)
+			printf("%s%s", n > 0 ? " " : "", sn->s);
+		else
+			printf("  %s\n", sn->s);
+		n++;
+	}
+	if (raw)
+		putchar('\n');
+}
+
+void metricgroup__print(bool metrics, bool metricgroups, char *filter,
+			bool raw)
+{
+	struct pmu_events_map *map = find_map();
+	struct pmu_event *pe;
+	int i;
+	struct rblist groups;
+	struct rb_node *node, *next;
+	struct strlist *metriclist = NULL;
+
+	if (!map)
+		return;
+
+	if (!metricgroups) {
+		metriclist = strlist__new(NULL, NULL);
+		if (!metriclist)
+			return;
+	}
+
+	rblist__init(&groups);
+	groups.node_new = mep_new;
+	groups.node_cmp = mep_cmp;
+	groups.node_delete = mep_delete;
+	for (i = 0; ; i++) {
+		const char *g;
+		pe = &map->table[i];
+
+		if (!pe->name && !pe->metric_group && !pe->metric_name)
+			break;
+		if (!pe->metric_expr)
+			continue;
+		g = pe->metric_group;
+		if (!g && pe->metric_name) {
+			if (pe->name)
+				continue;
+			g = "No_group";
+		}
+		if (g) {
+			char *omg;
+			char *mg = strdup(g);
+
+			if (!mg)
+				return;
+			omg = mg;
+			while ((g = strsep(&mg, ";")) != NULL) {
+				struct mep *me;
+				char *s;
+
+				if (*g == 0)
+					g = "No_group";
+				while (isspace(*g))
+					g++;
+				if (filter && !strstr(g, filter))
+					continue;
+				if (raw)
+					s = (char *)pe->metric_name;
+				else {
+					if (asprintf(&s, "%s\n\t[%s]",
+						     pe->metric_name, pe->desc) < 0)
+						return;
+				}
+
+				if (!s)
+					continue;
+
+				if (!metricgroups) {
+					strlist__add(metriclist, s);
+				} else {
+					me = mep_lookup(&groups, g);
+					if (!me)
+						continue;
+					strlist__add(me->metrics, s);
+				}
+			}
+			free(omg);
+		}
+	}
+
+	if (metricgroups && !raw)
+		printf("\nMetric Groups:\n\n");
+	else if (metrics && !raw)
+		printf("\nMetrics:\n\n");
+
+	for (node = rb_first(&groups.entries); node; node = next) {
+		struct mep *me = container_of(node, struct mep, nd);
+
+		if (metricgroups)
+			printf("%s%s%s", me->name, metrics ? ":" : "", raw ? " " : "\n");
+		if (metrics)
+			metricgroup__print_strlist(me->metrics, raw);
+		next = rb_next(node);
+		rblist__remove_node(&groups, node);
+	}
+	if (!metricgroups)
+		metricgroup__print_strlist(metriclist, raw);
+	strlist__delete(metriclist);
+}
+
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				   struct list_head *group_list)
 {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index cd89b5cba8d2..0604c676ad1d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -28,6 +28,7 @@
 #include "probe-file.h"
 #include "asm/bug.h"
 #include "util/parse-branch-options.h"
+#include "metricgroup.h"
 
 #define MAX_NAME_LEN 100
 
@@ -2372,6 +2373,8 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
 	print_tracepoint_events(NULL, NULL, name_only);
 
 	print_sdt_events(NULL, NULL, name_only);
+
+	metricgroup__print(true, true, NULL, name_only);
 }
 
 int parse_events__is_hardcoded_term(struct parse_events_term *term)
-- 
2.9.4

[PATCH v2 16/19] perf, tools, stat: Don't use ctx for saved values lookup

[Andi Kleen]

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

We don't need to use ctx to look up events for saved values.
The context is already part of the evsel pointer, which is the
primary key.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/stat-shadow.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 49b9931a7f63..11be64223196 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -56,7 +56,6 @@ struct saved_value {
 	struct rb_node rb_node;
 	struct perf_evsel *evsel;
 	int cpu;
-	int ctx;
 	struct stats stats;
 };
 
@@ -67,8 +66,6 @@ static int saved_value_cmp(struct rb_node *rb_node, const void *entry)
 					     rb_node);
 	const struct saved_value *b = entry;
 
-	if (a->ctx != b->ctx)
-		return a->ctx - b->ctx;
 	if (a->cpu != b->cpu)
 		return a->cpu - b->cpu;
 	if (a->evsel == b->evsel)
@@ -90,13 +87,12 @@ static struct rb_node *saved_value_new(struct rblist *rblist __maybe_unused,
 }
 
 static struct saved_value *saved_value_lookup(struct perf_evsel *evsel,
-					      int cpu, int ctx,
+					      int cpu,
 					      bool create)
 {
 	struct rb_node *nd;
 	struct saved_value dm = {
 		.cpu = cpu,
-		.ctx = ctx,
 		.evsel = evsel,
 	};
 	nd = rblist__find(&runtime_saved_values, &dm);
@@ -232,8 +228,7 @@ void perf_stat__update_shadow_stats(struct perf_evsel *counter, u64 *count,
 		update_stats(&runtime_aperf_stats[ctx][cpu], count[0]);
 
 	if (counter->collect_stat) {
-		struct saved_value *v = saved_value_lookup(counter, cpu, ctx,
-							   true);
+		struct saved_value *v = saved_value_lookup(counter, cpu, true);
 		update_stats(&v->stats, count[0]);
 	}
 }
@@ -634,7 +629,6 @@ static void generic_metric(const char *metric_expr,
 			   const char *metric_name,
 			   double avg,
 			   int cpu,
-			   int ctx,
 			   struct perf_stat_output_ctx *out)
 {
 	print_metric_t print_metric = out->print_metric;
@@ -648,7 +642,7 @@ static void generic_metric(const char *metric_expr,
 	for (i = 0; metric_events[i]; i++) {
 		struct saved_value *v;
 
-		v = saved_value_lookup(metric_events[i], cpu, ctx, false);
+		v = saved_value_lookup(metric_events[i], cpu, false);
 		if (!v)
 			break;
 		expr__add_id(&pctx, metric_events[i]->name, avg_stats(&v->stats));
@@ -866,7 +860,7 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 			print_metric(ctxp, NULL, NULL, name, 0);
 	} else if (evsel->metric_expr) {
 		generic_metric(evsel->metric_expr, evsel->metric_events, evsel->name,
-				evsel->metric_name, avg, cpu, ctx, out);
+				evsel->metric_name, avg, cpu, out);
 	} else if (runtime_nsecs_stats[cpu].n != 0) {
 		char unit = 'M';
 		char unit_buf[10];
@@ -895,7 +889,7 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 				out->new_line(ctxp);
 			generic_metric(mexp->metric_expr, mexp->metric_events,
 					evsel->name, mexp->metric_name,
-					avg, cpu, ctx, out);
+					avg, cpu, out);
 		}
 	}
 	if (num == 0)
-- 
2.9.4

[PATCH v2 17/19] perf, tools, stat: Support duration_time for metrics

[Andi Kleen]

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

Some of the metrics formulas (like GFLOPs) need to know how long
the measurement period is. Support an internal event called duration_time, which
reports time in second. It maps to the dummy event, but is
special cased for statistics to report the walltime duration.

So far it is not printed, but only used internally for metrics.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/parse-events.l |  1 +
 tools/perf/util/stat-shadow.c  | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index fdb5bb52f01f..ea2426daf7e8 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -288,6 +288,7 @@ cpu-migrations|migrations			{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COU
 alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
 emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
 dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
+duration_time					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
 bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
 
 	/*
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 11be64223196..ec0f3fa89489 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -641,11 +641,20 @@ static void generic_metric(const char *metric_expr,
 	expr__add_id(&pctx, name, avg);
 	for (i = 0; metric_events[i]; i++) {
 		struct saved_value *v;
+		struct stats *stats;
+		double scale;
 
-		v = saved_value_lookup(metric_events[i], cpu, false);
-		if (!v)
-			break;
-		expr__add_id(&pctx, metric_events[i]->name, avg_stats(&v->stats));
+		if (!strcmp(metric_events[i]->name, "duration_time")) {
+			stats = &walltime_nsecs_stats;
+			scale = 1e-9;
+		} else {
+			v = saved_value_lookup(metric_events[i], cpu, false);
+			if (!v)
+				break;
+			stats = &v->stats;
+			scale = 1.0;
+		}
+		expr__add_id(&pctx, metric_events[i]->name, avg_stats(stats)*scale);
 	}
 	if (!metric_events[i]) {
 		const char *p = metric_expr;
-- 
2.9.4

[PATCH v2 18/19] perf, tools, stat: Hide internal duration_time counter

[Andi Kleen]

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

Some perf stat metrics use an internal "duration_time" metric. It is not
correctly printed however. So hide it during output to avoid confusing users
with 0 counts.

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

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 830fd72a7fd0..ce18c08a60e0 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -195,6 +195,11 @@ static struct perf_stat_config stat_config = {
 	.scale		= true,
 };
 
+static bool is_duration_time(struct perf_evsel *evsel)
+{
+	return !strcmp(evsel->name, "duration_time");
+}
+
 static inline void diff_timespec(struct timespec *r, struct timespec *a,
 				 struct timespec *b)
 {
@@ -1363,6 +1368,9 @@ static void print_aggr(char *prefix)
 		ad.id = id = aggr_map->map[s];
 		first = true;
 		evlist__for_each_entry(evsel_list, counter) {
+			if (is_duration_time(counter))
+				continue;
+
 			ad.val = ad.ena = ad.run = 0;
 			ad.nr = 0;
 			if (!collect_data(counter, aggr_cb, &ad))
@@ -1506,6 +1514,8 @@ static void print_no_aggr_metric(char *prefix)
 		if (prefix)
 			fputs(prefix, stat_config.output);
 		evlist__for_each_entry(evsel_list, counter) {
+			if (is_duration_time(counter))
+				continue;
 			if (first) {
 				aggr_printout(counter, cpu, 0);
 				first = false;
@@ -1560,6 +1570,8 @@ static void print_metric_headers(const char *prefix, bool no_indent)
 
 	/* Print metrics headers only */
 	evlist__for_each_entry(evsel_list, counter) {
+		if (is_duration_time(counter))
+			continue;
 		os.evsel = counter;
 		out.ctx = &os;
 		out.print_metric = print_metric_header;
@@ -1707,12 +1719,18 @@ static void print_counters(struct timespec *ts, int argc, const char **argv)
 		print_aggr(prefix);
 		break;
 	case AGGR_THREAD:
-		evlist__for_each_entry(evsel_list, counter)
+		evlist__for_each_entry(evsel_list, counter) {
+			if (is_duration_time(counter))
+				continue;
 			print_aggr_thread(counter, prefix);
+		}
 		break;
 	case AGGR_GLOBAL:
-		evlist__for_each_entry(evsel_list, counter)
+		evlist__for_each_entry(evsel_list, counter) {
+			if (is_duration_time(counter))
+				continue;
 			print_counter_aggr(counter, prefix);
+		}
 		if (metric_only)
 			fputc('\n', stat_config.output);
 		break;
@@ -1720,8 +1738,11 @@ static void print_counters(struct timespec *ts, int argc, const char **argv)
 		if (metric_only)
 			print_no_aggr_metric(prefix);
 		else {
-			evlist__for_each_entry(evsel_list, counter)
+			evlist__for_each_entry(evsel_list, counter) {
+				if (is_duration_time(counter))
+					continue;
 				print_counter(counter, prefix);
+			}
 		}
 		break;
 	case AGGR_UNSET:
-- 
2.9.4

[PATCH v2 19/19] perf, tools, stat: Update walltime_nsecs_stats in interval mode

[Andi Kleen]

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

Some metrics (like GFLOPs) need walltime_nsecs_stats for each interval.
Compute it for each interval instead of only at the end.

Pointed out by Jiri.

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

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index ce18c08a60e0..2d3354d7fcdc 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -415,6 +415,8 @@ static void process_interval(void)
 			pr_err("failed to write stat round event\n");
 	}
 
+	init_stats(&walltime_nsecs_stats);
+	update_stats(&walltime_nsecs_stats, stat_config.interval * 1000000);
 	print_counters(&rs, 0, NULL);
 }
 
-- 
2.9.4

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Save the original array dimensions in xyarrays, so that users can retrieve them
> later. Add some inline functions to access these fields.

hi,
is there a branch with this?

also I recall sending feedback for some of those patches,
any changes..?

jirka

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Andi Kleen]

On Mon, Aug 14, 2017 at 10:15:49PM +0200, Jiri Olsa wrote:
> On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Save the original array dimensions in xyarrays, so that users can retrieve them
> > later. Add some inline functions to access these fields.
> 
> hi,
> is there a branch with this?

Yes 

   git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git perf/metric-group-5

You didn't see the 0/0 message?

> 
> also I recall sending feedback for some of those patches,
> any changes..?

I fixed all feedback. See the individual patches.

-Andi

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Jiri Olsa]

On Mon, Aug 14, 2017 at 01:39:28PM -0700, Andi Kleen wrote:
> On Mon, Aug 14, 2017 at 10:15:49PM +0200, Jiri Olsa wrote:
> > On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> > > From: Andi Kleen <ak@linux.intel.com>
> > > 
> > > Save the original array dimensions in xyarrays, so that users can retrieve them
> > > later. Add some inline functions to access these fields.
> > 
> > hi,
> > is there a branch with this?
> 
> Yes 
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git perf/metric-group-5
> 
> You didn't see the 0/0 message?

nope, it seems like 1/19 was the In-reply-to message

6102 r   Aug 11 Andi Kleen      (1.4K) [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray
6103 O   Aug 11 Andi Kleen      (0.6K) ├─>[PATCH v2 06/19] perf, tools: Add missing newline to expr parser error messages

> 
> > 
> > also I recall sending feedback for some of those patches,
> > any changes..?
> 
> I fixed all feedback. See the individual patches.

ok

jirka

Re: [PATCH v2 04/19] perf, tools: Tighten detection of BPF events

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:19PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> perf stat -e cpu/uops_executed.core,cmask=1/
> 
> would be detected as a BPF source event because the .c matches the .c
> source BPF pattern.
> 
> v2:
> 
> Originally I tried to use lex lookahead, but it doesn't seem to work.
> 
> This now extends the BPF pattern to match longer events, but then
> does an extra check in the C code to reject BPF matches that
> do not end with .c/.o/.obj
> 
> This uses REJECT, which makes the flex scanner slower, but
> that shouldn't be a big problem for the perf events.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

seems ok, Arnaldo, could you run it with BPF/clang test machinery? ;-)

thanks,
jirka

Re: [PATCH v2 02/19] perf, tools: Fix buffer overflow while freeing events

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:17PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Fix buffer overflow for
> 
> % perf stat -e msr/tsc/,cstate_core/c7-residency/ true
> 
> that causes glibc free list corruption. For some reason
> it doesn't trigger in valgrind, but it is visible in AS:
> 
> =================================================================
> ==32681==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000003f5c at pc 0x0000005671ef bp 0x7ffdaaac9ac0 sp 0x7ffdaaac9ab0
> READ of size 4 at 0x603000003f5c thread T0
>     #0 0x5671ee in perf_evsel__close_fd util/evsel.c:1196
>     #1 0x56c57a in perf_evsel__close util/evsel.c:1717
>     #2 0x55ed5f in perf_evlist__close util/evlist.c:1631
>     #3 0x4647e1 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:749
>     #4 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767
>     #5 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785
>     #6 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296
>     #7 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348
>     #8 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392
>     #9 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530
>     #10 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400)
>     #11 0x428419 in _start (/home/ak/hle/obj-perf/perf+0x428419)
> 
> 0x603000003f5c is located 0 bytes to the right of 28-byte region [0x603000003f40,0x603000003f5c)
> allocated by thread T0 here:
>     #0 0x7f0675139020 in calloc (/lib64/libasan.so.3+0xc7020)
>     #1 0x648a2d in zalloc util/util.h:23
>     #2 0x648a88 in xyarray__new util/xyarray.c:9
>     #3 0x566419 in perf_evsel__alloc_fd util/evsel.c:1039
>     #4 0x56b427 in perf_evsel__open util/evsel.c:1529
>     #5 0x56c620 in perf_evsel__open_per_thread util/evsel.c:1730
>     #6 0x461dea in create_perf_stat_counter /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:263
>     #7 0x4637d7 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:600
>     #8 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767
>     #9 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785
>     #10 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296
>     #11 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348
>     #12 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392
>     #13 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530
>     #14 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400)
> 
> The event is allocated with cpus == 1, but freed with cpus == real number
> When the evsel close function walks the file descriptors it exceeds the
> fd xyarray boundaries and reads random memory.
> 
> v2:
> Now that xyarrays save their original dimensions we can use these
> to iterate the two dimensional fd arrays. Fix some users
> (close, ioctl) in evsel.c to use these fields directly. This allows simplifying
> the code and dropping quite a few function arguments. Adjust
> all callers by removing the unneeded arguments.
> 
> The actual perf event reading still uses the original values from
> the evsel list.

i was wondering how much code change this would be,
but it turned out not that bad ;-)

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

thanks,
jirka

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Save the original array dimensions in xyarrays, so that users can retrieve them
> later. Add some inline functions to access these fields.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/xyarray.c |  2 ++
>  tools/perf/util/xyarray.h | 12 ++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/tools/perf/util/xyarray.c b/tools/perf/util/xyarray.c
> index 7251fdbabced..c8f415d9877b 100644
> --- a/tools/perf/util/xyarray.c
> +++ b/tools/perf/util/xyarray.c
> @@ -12,6 +12,8 @@ struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size)
>  		xy->entry_size = entry_size;
>  		xy->row_size   = row_size;
>  		xy->entries    = xlen * ylen;
> +		xy->max_x      = xlen;
> +		xy->max_y      = ylen;
>  	}
>  
>  	return xy;
> diff --git a/tools/perf/util/xyarray.h b/tools/perf/util/xyarray.h
> index 7f30af371b7e..785884158137 100644
> --- a/tools/perf/util/xyarray.h
> +++ b/tools/perf/util/xyarray.h
> @@ -7,6 +7,8 @@ struct xyarray {
>  	size_t row_size;
>  	size_t entry_size;
>  	size_t entries;
> +	size_t max_x;
> +	size_t max_y;
>  	char contents[];
>  };
>  
> @@ -19,4 +21,14 @@ static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
>  	return &xy->contents[x * xy->row_size + y * xy->entry_size];
>  }
>  
> +static inline int xy_max_y(struct xyarray *xy)
> +{
> +	return xy->max_x;
> +}
> +
> +static inline int xy_max_x(struct xyarray *xy)
> +{
> +	return xy->max_y;
> +}

I think you should follow the standard naming
and use xyarray__max_x and xyarray__max_y

jirka

Re: [PATCH v2 05/19] perf, tools: Support weak groups

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index f709de54707b..d432965d728d 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -47,6 +47,7 @@ counted. The following modifiers exist:
>   P - use maximum detected precise level
>   S - read sample value (PERF_SAMPLE_READ)
>   D - pin the event to the PMU
> + W - group is weak and will fallback to non-group if not schedulable
>  
>  The 'p' modifier can be used for specifying how precise the instruction
>  address should be. The 'p' modifier can be specified multiple times:
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 866da7aa54bf..ac605caf1a01 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
>  	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
>  }
>  
> +static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
> +{
> +	struct perf_evsel *c2, *leader;
> +	bool is_open = true;
> +
> +	leader = counter->leader;
> +	pr_debug("Weak group for %s/%d failed\n",
> +			leader->name, counter->nr_members);

I'm getting 'branches/0' in here for you example:

[root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1
Using CPUID GenuineIntel-6-3D
l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
l2_rqsts.all_code_rd -> cpu/umask=0xe4,period=200003,event=0x24/
Weak group for branches/0 failed

I'd expect nr_members == 5 ... any idea?

thanks,
jirka

Re: [PATCH v2 05/19] perf, tools: Support weak groups

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Setting up groups can be complicated due to the
> complicated scheduling restrictions of different PMUs.
> User tools usually don't understand all these restrictions.
> Still in many cases it is useful to set up groups and
> they work most of the time. However if the group
> is set up wrong some members will not reported any values
> because they never get scheduled.
> 
> Add a concept of a 'weak group': try to set up a group,
> but if it's not schedulable fallback to not using
> a group. That gives us the best of both worlds:
> groups if they work, but still a usable fallback if they don't.
> 
> In theory it would be possible to have more complex fallback
> strategies (e.g. try to split the group in half), but
> the simple fallback of not using a group seems to work for now.
> 
> So far the weak group is only implemented for perf stat,
> not for record.
> 
> Here's an unschedulable group (on IvyBridge with SMT on)
> 
> % perf stat -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}' -a sleep 1
> 
>         73,806,067      branches
>          4,848,144      branch-misses             #    6.57% of all branches
>         14,754,458      l1d.replacement
>         24,905,558      l2_lines_in.all
>    <not supported>      l2_rqsts.all_code_rd         <------- will never report anything

also if I put 'cycles' instead of the l2_rqsts.all_code_rd,
I get clean open but 'not counted' as result.. I wonder
there's some counter scheduling issue

[root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,cycles}:W' -a sleep 1
Using CPUID GenuineIntel-6-3D
l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
branches: 0 4004293853 0
branch-misses: 0 4004293853 0
l1d.replacement: 0 4004293853 0
l2_lines_in.all: 0 4004293853 0
cycles: 0 4004293853 0

 Performance counter stats for 'system wide':

     <not counted>      branches                                                      (0.00%)
     <not counted>      branch-misses                                                 (0.00%)
     <not counted>      l1d.replacement                                               (0.00%)
     <not counted>      l2_lines_in.all                                               (0.00%)
     <not counted>      cycles                                                        (0.00%)

       1.001088589 seconds time elapsed

jirka

Re: [PATCH v2 07/19] perf, tools: Add utility function to detect SMT status

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:22PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add an smt_on() function to return if SMT is enabled or disabled.
> Used in the next patch.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

thanks,
jirka

Re: [PATCH v2 08/19] perf, tools: Expression parser enhancements for metrics

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:23PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Enhance the expression parser for more complex metric formulas.
> 
> - Support python style IF ELSE operators
> - Add an #SMT_On magic variable for formulas that depend on the SMT
> status.
> 
> Example: 4 *( CPU_CLK_UNHALTED.THREAD_ANY / 2 ) if #SMT_on else cycles
> 
> - Support MIN/MAX operations
> 
> Example: min(1 , IDQ.MITE_UOPS / ( UPI * 16 * ( ICACHE.HIT + ICACHE.MISSES ) / 4.0 ) )
> 
> This is useful to fix up problems caused by multiplexing.
> 
> - Support | & ^ operators
> - Minor cleanups and fixes
> - Support an \ escape for operators. This allows to specify event names
> like c2-residency
> - Support @ as an alternative for / to be able to specify pmus without
> conflicts with operators (like msr/tsc/ as msr@tsc@)
> 
> Example: (cstate_core@c3\\-residency@ / msr@tsc@) * 100
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

thanks,
jirka

Re: [PATCH v2 09/19] perf, tools: Increase maximum number of events in expressions

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:24PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Some of the upcoming metrics need more than 8 events. Increase the maximum
> number the parser supports.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

thanks,
jirka

Re: [PATCH v2 10/19] perf, tools: Dedup events in expression parsing

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:25PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Avoid adding redundant events while parsing an expression.
> When we add an "other" event check first if it already exists.
> 
> v2: Fix perf test failure.
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

thanks,
jirka

> ---
>  tools/perf/util/expr.y | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> index 5753c4f21534..432b8560cf51 100644
> --- a/tools/perf/util/expr.y
> +++ b/tools/perf/util/expr.y
> @@ -181,6 +181,19 @@ void expr__ctx_init(struct parse_ctx *ctx)
>  	ctx->num_ids = 0;
>  }
>  
> +static bool already_seen(const char *val, const char *one, const char **other,
> +			 int num_other)
> +{
> +	int i;
> +
> +	if (one && !strcasecmp(one, val))
> +		return true;
> +	for (i = 0; i < num_other; i++)
> +		if (!strcasecmp(other[i], val))
> +			return true;
> +	return false;
> +}
> +
>  int expr__find_other(const char *p, const char *one, const char ***other,
>  		     int *num_otherp)
>  {
> @@ -200,7 +213,7 @@ int expr__find_other(const char *p, const char *one, const char ***other,
>  			err = 0;
>  			break;
>  		}
> -		if (tok == ID && (!one || strcasecmp(one, val.id))) {
> +		if (tok == ID && !already_seen(val.id, one, *other, num_other)) {
>  			if (num_other >= EXPR_MAX_OTHER - 1) {
>  				pr_debug("Too many extra events in %s\n", orig);
>  				break;
> -- 
> 2.9.4
> 

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Arnaldo Carvalho de Melo]

Em Tue, Aug 22, 2017 at 10:20:28AM +0200, Jiri Olsa escreveu:
> On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > +static inline int xy_max_y(struct xyarray *xy)
> > +{
> > +	return xy->max_x;
> > +}
> > +
> > +static inline int xy_max_x(struct xyarray *xy)
> > +{
> > +	return xy->max_y;
> > +}
> 
> I think you should follow the standard naming
> and use xyarray__max_x and xyarray__max_y

Yeah, will take just a bit longer to process, I'll fix that up...

- Arnaldo

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Jiri Olsa]

On Tue, Aug 22, 2017 at 11:40:50AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 22, 2017 at 10:20:28AM +0200, Jiri Olsa escreveu:
> > On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> > > From: Andi Kleen <ak@linux.intel.com>
> > > +static inline int xy_max_y(struct xyarray *xy)
> > > +{
> > > +	return xy->max_x;
> > > +}
> > > +
> > > +static inline int xy_max_x(struct xyarray *xy)
> > > +{
> > > +	return xy->max_y;
> > > +}
> > 
> > I think you should follow the standard naming
> > and use xyarray__max_x and xyarray__max_y
> 
> Yeah, will take just a bit longer to process, I'll fix that up...

fyi it's used in following patch

jirka

Re: [PATCH v2 04/19] perf, tools: Tighten detection of BPF events

[Arnaldo Carvalho de Melo]

Em Tue, Aug 22, 2017 at 10:20:07AM +0200, Jiri Olsa escreveu:
> On Fri, Aug 11, 2017 at 04:26:19PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > perf stat -e cpu/uops_executed.core,cmask=1/
> > 
> > would be detected as a BPF source event because the .c matches the .c
> > source BPF pattern.
> > 
> > v2:
> > 
> > Originally I tried to use lex lookahead, but it doesn't seem to work.
> > 
> > This now extends the BPF pattern to match longer events, but then
> > does an extra check in the C code to reject BPF matches that
> > do not end with .c/.o/.obj
> > 
> > This uses REJECT, which makes the flex scanner slower, but
> > that shouldn't be a big problem for the perf events.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> seems ok, Arnaldo, could you run it with BPF/clang test machinery? ;-)

Seems to work now:

[root@jouet bpf]# perf trace -e write -e /home/acme/bpf/tracepoint.c cat /etc/passwd > /dev/null
     0.000 ( 0.006 ms): cat/18485 write(fd: 1, buf: 0x7f59eebe1000, count: 3494                         ) ...
     0.006 (         ): raw_syscalls:sys_enter:NR 1 (1, 7f59eebe1000, da6, 22, 7f59eebe0010, 0))
     0.008 (         ): perf_bpf_probe:_write:(ffffffff9626b2c0))
     0.000 ( 0.010 ms): cat/18485  ... [continued]: write()) = 3494
[root@jouet bpf]# 

I'll try and add this as a "shell test", looking if all the required
components are in place, etc.

- Arnaldo

Re: [PATCH v2 01/19] perf, tools: Save max_x, max_y in xyarray

[Arnaldo Carvalho de Melo]

Em Tue, Aug 22, 2017 at 04:44:16PM +0200, Jiri Olsa escreveu:
> On Tue, Aug 22, 2017 at 11:40:50AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Aug 22, 2017 at 10:20:28AM +0200, Jiri Olsa escreveu:
> > > On Fri, Aug 11, 2017 at 04:26:16PM -0700, Andi Kleen wrote:
> > > > From: Andi Kleen <ak@linux.intel.com>
> > > > +static inline int xy_max_y(struct xyarray *xy)
> > > > +{
> > > > +	return xy->max_x;
> > > > +}
> > > > +
> > > > +static inline int xy_max_x(struct xyarray *xy)
> > > > +{
> > > > +	return xy->max_y;
> > > > +}
> > > 
> > > I think you should follow the standard naming
> > > and use xyarray__max_x and xyarray__max_y
> > 
> > Yeah, will take just a bit longer to process, I'll fix that up...
> 
> fyi it's used in following patch

Yeah, sure, I expected it to be :-P And fixed it up accordingly :-)

- Arnaldo

Re: [PATCH v2 07/19] perf, tools: Add utility function to detect SMT status

[Arnaldo Carvalho de Melo]

Em Tue, Aug 22, 2017 at 10:45:00AM +0200, Jiri Olsa escreveu:
> On Fri, Aug 11, 2017 at 04:26:22PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Add an smt_on() function to return if SMT is enabled or disabled.
> > Used in the next patch.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.

- Arnaldo

Re: [PATCH v2 08/19] perf, tools: Expression parser enhancements for metrics

[Arnaldo Carvalho de Melo]

Em Tue, Aug 22, 2017 at 10:45:18AM +0200, Jiri Olsa escreveu:
> On Fri, Aug 11, 2017 at 04:26:23PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Enhance the expression parser for more complex metric formulas.
> > 
> > - Support python style IF ELSE operators
> > - Add an #SMT_On magic variable for formulas that depend on the SMT
> > status.
> > 
> > Example: 4 *( CPU_CLK_UNHALTED.THREAD_ANY / 2 ) if #SMT_on else cycles
> > 
> > - Support MIN/MAX operations
> > 
> > Example: min(1 , IDQ.MITE_UOPS / ( UPI * 16 * ( ICACHE.HIT + ICACHE.MISSES ) / 4.0 ) )
> > 
> > This is useful to fix up problems caused by multiplexing.
> > 
> > - Support | & ^ operators
> > - Minor cleanups and fixes
> > - Support an \ escape for operators. This allows to specify event names
> > like c2-residency
> > - Support @ as an alternative for / to be able to specify pmus without
> > conflicts with operators (like msr/tsc/ as msr@tsc@)
> > 
> > Example: (cstate_core@c3\\-residency@ / msr@tsc@) * 100
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.

- Arnaldo

Re: [PATCH v2 09/19] perf, tools: Increase maximum number of events in expressions

[Arnaldo Carvalho de Melo]

Em Tue, Aug 22, 2017 at 10:45:33AM +0200, Jiri Olsa escreveu:
> On Fri, Aug 11, 2017 at 04:26:24PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Some of the upcoming metrics need more than 8 events. Increase the maximum
> > number the parser supports.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.

- Arnaldo

Re: [PATCH v2 05/19] perf, tools: Support weak groups

[Andi Kleen]

On Tue, Aug 22, 2017 at 10:34:15AM +0200, Jiri Olsa wrote:
> On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:
> 
> SNIP
> 
> > diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> > index f709de54707b..d432965d728d 100644
> > --- a/tools/perf/Documentation/perf-list.txt
> > +++ b/tools/perf/Documentation/perf-list.txt
> > @@ -47,6 +47,7 @@ counted. The following modifiers exist:
> >   P - use maximum detected precise level
> >   S - read sample value (PERF_SAMPLE_READ)
> >   D - pin the event to the PMU
> > + W - group is weak and will fallback to non-group if not schedulable
> >  
> >  The 'p' modifier can be used for specifying how precise the instruction
> >  address should be. The 'p' modifier can be specified multiple times:
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 866da7aa54bf..ac605caf1a01 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
> >  	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
> >  }
> >  
> > +static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
> > +{
> > +	struct perf_evsel *c2, *leader;
> > +	bool is_open = true;
> > +
> > +	leader = counter->leader;
> > +	pr_debug("Weak group for %s/%d failed\n",
> > +			leader->name, counter->nr_members);
> 
> I'm getting 'branches/0' in here for you example:
> 
> [root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1
> Using CPUID GenuineIntel-6-3D
> l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
> l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
> l2_rqsts.all_code_rd -> cpu/umask=0xe4,period=200003,event=0x24/
> Weak group for branches/0 failed
> 
> I'd expect nr_members == 5 ... any idea?

Looks like nr_members is only correct in the leader. This works

@@ -599,7 +599,7 @@ static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
 
        leader = counter->leader;
        pr_debug("Weak group for %s/%d failed\n",
-                       leader->name, counter->nr_members);
+                       leader->name, leader->nr_members);


-Andi

Re: [PATCH v2 05/19] perf, tools: Support weak groups

[Andi Kleen]

> also if I put 'cycles' instead of the l2_rqsts.all_code_rd,
> I get clean open but 'not counted' as result.. I wonder
> there's some counter scheduling issue
> 
> [root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,cycles}:W' -a sleep 1
> Using CPUID GenuineIntel-6-3D
> l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
> l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
> branches: 0 4004293853 0
> branch-misses: 0 4004293853 0
> l1d.replacement: 0 4004293853 0
> l2_lines_in.all: 0 4004293853 0
> cycles: 0 4004293853 0
> 
>  Performance counter stats for 'system wide':
> 
>      <not counted>      branches                                                      (0.00%)
>      <not counted>      branch-misses                                                 (0.00%)
>      <not counted>      l1d.replacement                                               (0.00%)
>      <not counted>      l2_lines_in.all                                               (0.00%)
>      <not counted>      cycles                                                        (0.00%)
> 
>        1.001088589 seconds time elapsed

Cannot reproduce on my system

Does it work without group or a normal group?

./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,cycles}:W'
Using CPUID GenuineIntel-6-3E
l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
^Cbranches: 105004611 27393959413 27393959413
branch-misses: 5854172 27393959413 27393959413
l1d.replacement: 13935607 27393959413 27393959413
l2_lines_in.all: 24987238 27393959413 27393959413
cycles: 1259015810 27393959413 27393959413

 Performance counter stats for 'system wide':

       105,004,611      branches                                                    
         5,854,172      branch-misses             #    5.58% of all branches        
        13,935,607      l1d.replacement                                             
        24,987,238      l2_lines_in.all                                             
     1,259,015,810      cycles                                                      

       2.282966609 seconds time elapsed

Re: [PATCH v2 05/19] perf, tools: Support weak groups

[Jiri Olsa]

On Tue, Aug 22, 2017 at 01:58:12PM -0700, Andi Kleen wrote:
> On Tue, Aug 22, 2017 at 10:34:15AM +0200, Jiri Olsa wrote:
> > On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:
> > 
> > SNIP
> > 
> > > diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> > > index f709de54707b..d432965d728d 100644
> > > --- a/tools/perf/Documentation/perf-list.txt
> > > +++ b/tools/perf/Documentation/perf-list.txt
> > > @@ -47,6 +47,7 @@ counted. The following modifiers exist:
> > >   P - use maximum detected precise level
> > >   S - read sample value (PERF_SAMPLE_READ)
> > >   D - pin the event to the PMU
> > > + W - group is weak and will fallback to non-group if not schedulable
> > >  
> > >  The 'p' modifier can be used for specifying how precise the instruction
> > >  address should be. The 'p' modifier can be specified multiple times:
> > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > index 866da7aa54bf..ac605caf1a01 100644
> > > --- a/tools/perf/builtin-stat.c
> > > +++ b/tools/perf/builtin-stat.c
> > > @@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
> > >  	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
> > >  }
> > >  
> > > +static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
> > > +{
> > > +	struct perf_evsel *c2, *leader;
> > > +	bool is_open = true;
> > > +
> > > +	leader = counter->leader;
> > > +	pr_debug("Weak group for %s/%d failed\n",
> > > +			leader->name, counter->nr_members);
> > 
> > I'm getting 'branches/0' in here for you example:
> > 
> > [root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1
> > Using CPUID GenuineIntel-6-3D
> > l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
> > l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
> > l2_rqsts.all_code_rd -> cpu/umask=0xe4,period=200003,event=0x24/
> > Weak group for branches/0 failed
> > 
> > I'd expect nr_members == 5 ... any idea?
> 
> Looks like nr_members is only correct in the leader. This works
> 
> @@ -599,7 +599,7 @@ static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
>  
>         leader = counter->leader;
>         pr_debug("Weak group for %s/%d failed\n",
> -                       leader->name, counter->nr_members);
> +                       leader->name, leader->nr_members);
> 

right, that's it

jirka

Re: [PATCH v2 08/19] perf, tools: Expression parser enhancements for metrics

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:23PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Enhance the expression parser for more complex metric formulas.
> 
> - Support python style IF ELSE operators
> - Add an #SMT_On magic variable for formulas that depend on the SMT
> status.
> 
> Example: 4 *( CPU_CLK_UNHALTED.THREAD_ANY / 2 ) if #SMT_on else cycles
> 
> - Support MIN/MAX operations
> 
> Example: min(1 , IDQ.MITE_UOPS / ( UPI * 16 * ( ICACHE.HIT + ICACHE.MISSES ) / 4.0 ) )
> 
> This is useful to fix up problems caused by multiplexing.
> 
> - Support | & ^ operators
> - Minor cleanups and fixes
> - Support an \ escape for operators. This allows to specify event names
> like c2-residency
> - Support @ as an alternative for / to be able to specify pmus without
> conflicts with operators (like msr/tsc/ as msr@tsc@)
> 
> Example: (cstate_core@c3\\-residency@ / msr@tsc@) * 100
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/tests/expr.c |  5 ++++
>  tools/perf/util/expr.y  | 61 ++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 60 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
> index 6c6a3749aaf6..e75365098f23 100644
> --- a/tools/perf/tests/expr.c
> +++ b/tools/perf/tests/expr.c
> @@ -31,6 +31,11 @@ int test__expr(int subtest __maybe_unused)
>  	ret |= test(&ctx, "(BAR/2)%2", 1);
>  	ret |= test(&ctx, "1 - -4",  5);
>  	ret |= test(&ctx, "(FOO-1)*2 + (BAR/2)%2 - -4",  5);
> +	ret |= test(&ctx, "1-1 | 1", 1);
> +	ret |= test(&ctx, "1-1 & 1", 0);
> +	ret |= test(&ctx, "min(1,2) + 1", 2);
> +	ret |= test(&ctx, "max(1,2) + 1", 3);
> +	ret |= test(&ctx, "1+1 if 3*4 else 0", 2);

fyi I'm getting on Arnaldo's tree 'syntax error' but test is passing

[jolsa@krava perf]$ ./perf test expr -v
 6: Simple expression parser                              :
--- start ---
test child forked, pid 1520
syntax error
test child finished with 0
---- end ----
Simple expression parser: Ok

jirka

[tip:perf/core] perf xyarray: Save max_x, max_y

[tip-bot for Andi Kleen]

Commit-ID:  d74be47673676eded2f0bb8274e752bed32c42d8
Gitweb:     http://git.kernel.org/tip/d74be47673676eded2f0bb8274e752bed32c42d8
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:16 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 11:51:28 -0300

perf xyarray: Save max_x, max_y

Save the original array dimensions in xyarrays, so that users can
retrieve them later. Add some inline functions to access these fields.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170811232634.30465-1-andi@firstfloor.org
[ As noticed by Jiri, fix up namespacing: xy__method() -> xyarray__method() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/xyarray.c |  2 ++
 tools/perf/util/xyarray.h | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/tools/perf/util/xyarray.c b/tools/perf/util/xyarray.c
index 7251fdb..c8f415d 100644
--- a/tools/perf/util/xyarray.c
+++ b/tools/perf/util/xyarray.c
@@ -12,6 +12,8 @@ struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size)
 		xy->entry_size = entry_size;
 		xy->row_size   = row_size;
 		xy->entries    = xlen * ylen;
+		xy->max_x      = xlen;
+		xy->max_y      = ylen;
 	}
 
 	return xy;
diff --git a/tools/perf/util/xyarray.h b/tools/perf/util/xyarray.h
index 7f30af3..4ba726c 100644
--- a/tools/perf/util/xyarray.h
+++ b/tools/perf/util/xyarray.h
@@ -7,6 +7,8 @@ struct xyarray {
 	size_t row_size;
 	size_t entry_size;
 	size_t entries;
+	size_t max_x;
+	size_t max_y;
 	char contents[];
 };
 
@@ -19,4 +21,14 @@ static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
 	return &xy->contents[x * xy->row_size + y * xy->entry_size];
 }
 
+static inline int xyarray__max_y(struct xyarray *xy)
+{
+	return xy->max_x;
+}
+
+static inline int xyarray__max_x(struct xyarray *xy)
+{
+	return xy->max_y;
+}
+
 #endif /* _PERF_XYARRAY_H_ */

[tip:perf/core] perf evsel: Fix buffer overflow while freeing events

[tip-bot for Andi Kleen]

Commit-ID:  475fb533fb7d3dcf009a434f9b9ea238b93f4cb8
Gitweb:     http://git.kernel.org/tip/475fb533fb7d3dcf009a434f9b9ea238b93f4cb8
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:17 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 11:51:31 -0300

perf evsel: Fix buffer overflow while freeing events

Fix buffer overflow for:

  % perf stat -e msr/tsc/,cstate_core/c7-residency/ true

that causes glibc free list corruption. For some reason it doesn't
trigger in valgrind, but it is visible in AS:

  =================================================================
  ==32681==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000003f5c at pc 0x0000005671ef bp 0x7ffdaaac9ac0 sp 0x7ffdaaac9ab0
  READ of size 4 at 0x603000003f5c thread T0
    #0 0x5671ee in perf_evsel__close_fd util/evsel.c:1196
    #1 0x56c57a in perf_evsel__close util/evsel.c:1717
    #2 0x55ed5f in perf_evlist__close util/evlist.c:1631
    #3 0x4647e1 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:749
    #4 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767
    #5 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785
    #6 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296
    #7 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348
    #8 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392
    #9 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530
    #10 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    #11 0x428419 in _start (/home/ak/hle/obj-perf/perf+0x428419)

  0x603000003f5c is located 0 bytes to the right of 28-byte region [0x603000003f40,0x603000003f5c)
  allocated by thread T0 here:
    #0 0x7f0675139020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x648a2d in zalloc util/util.h:23
    #2 0x648a88 in xyarray__new util/xyarray.c:9
    #3 0x566419 in perf_evsel__alloc_fd util/evsel.c:1039
    #4 0x56b427 in perf_evsel__open util/evsel.c:1529
    #5 0x56c620 in perf_evsel__open_per_thread util/evsel.c:1730
    #6 0x461dea in create_perf_stat_counter /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:263
    #7 0x4637d7 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:600
    #8 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767
    #9 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785
    #10 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296
    #11 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348
    #12 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392
    #13 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530
    #14 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400)

The event is allocated with cpus == 1, but freed with cpus == real number
When the evsel close function walks the file descriptors it exceeds the
fd xyarray boundaries and reads random memory.

v2:

Now that xyarrays save their original dimensions we can use these to
iterate the two dimensional fd arrays. Fix some users (close, ioctl) in
evsel.c to use these fields directly. This allows simplifying the code
and dropping quite a few function arguments. Adjust all callers by
removing the unneeded arguments.

The actual perf event reading still uses the original values from the
evsel list.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170811232634.30465-2-andi@firstfloor.org
[ Fix up xy_max_[xy]() -> xyarray__max_[xy]() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/openat-syscall-all-cpus.c |  2 +-
 tools/perf/tests/openat-syscall.c          |  2 +-
 tools/perf/util/evlist.c                   | 12 +++-------
 tools/perf/util/evsel.c                    | 37 ++++++++++--------------------
 tools/perf/util/evsel.h                    |  7 +++---
 5 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/tools/perf/tests/openat-syscall-all-cpus.c b/tools/perf/tests/openat-syscall-all-cpus.c
index 8726511..9cf1c35 100644
--- a/tools/perf/tests/openat-syscall-all-cpus.c
+++ b/tools/perf/tests/openat-syscall-all-cpus.c
@@ -115,7 +115,7 @@ int test__openat_syscall_event_on_all_cpus(struct test *test __maybe_unused, int
 
 	perf_evsel__free_counts(evsel);
 out_close_fd:
-	perf_evsel__close_fd(evsel, 1, threads->nr);
+	perf_evsel__close_fd(evsel);
 out_evsel_delete:
 	perf_evsel__delete(evsel);
 out_thread_map_delete:
diff --git a/tools/perf/tests/openat-syscall.c b/tools/perf/tests/openat-syscall.c
index 85bb672..9dc5c5d 100644
--- a/tools/perf/tests/openat-syscall.c
+++ b/tools/perf/tests/openat-syscall.c
@@ -56,7 +56,7 @@ int test__openat_syscall_event(struct test *test __maybe_unused, int subtest __m
 
 	err = 0;
 out_close_fd:
-	perf_evsel__close_fd(evsel, 1, threads->nr);
+	perf_evsel__close_fd(evsel);
 out_evsel_delete:
 	perf_evsel__delete(evsel);
 out_thread_map_delete:
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 078b585..6a0d7ff 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1419,8 +1419,6 @@ int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **e
 {
 	struct perf_evsel *evsel;
 	int err = 0;
-	const int ncpus = cpu_map__nr(evlist->cpus),
-		  nthreads = thread_map__nr(evlist->threads);
 
 	evlist__for_each_entry(evlist, evsel) {
 		if (evsel->filter == NULL)
@@ -1430,7 +1428,7 @@ int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **e
 		 * filters only work for tracepoint event, which doesn't have cpu limit.
 		 * So evlist and evsel should always be same.
 		 */
-		err = perf_evsel__apply_filter(evsel, ncpus, nthreads, evsel->filter);
+		err = perf_evsel__apply_filter(evsel, evsel->filter);
 		if (err) {
 			*err_evsel = evsel;
 			break;
@@ -1623,13 +1621,9 @@ void perf_evlist__set_selected(struct perf_evlist *evlist,
 void perf_evlist__close(struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel;
-	int ncpus = cpu_map__nr(evlist->cpus);
-	int nthreads = thread_map__nr(evlist->threads);
 
-	evlist__for_each_entry_reverse(evlist, evsel) {
-		int n = evsel->cpus ? evsel->cpus->nr : ncpus;
-		perf_evsel__close(evsel, n, nthreads);
-	}
+	evlist__for_each_entry_reverse(evlist, evsel)
+		perf_evsel__close(evsel);
 }
 
 static int perf_evlist__create_syswide_maps(struct perf_evlist *evlist)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3735c9e..5dfb8bc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1051,16 +1051,13 @@ static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthread
 	return evsel->fd != NULL ? 0 : -ENOMEM;
 }
 
-static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthreads,
+static int perf_evsel__run_ioctl(struct perf_evsel *evsel,
 			  int ioc,  void *arg)
 {
 	int cpu, thread;
 
-	if (evsel->system_wide)
-		nthreads = 1;
-
-	for (cpu = 0; cpu < ncpus; cpu++) {
-		for (thread = 0; thread < nthreads; thread++) {
+	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) {
+		for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
 			int fd = FD(evsel, cpu, thread),
 			    err = ioctl(fd, ioc, arg);
 
@@ -1072,10 +1069,9 @@ static int perf_evsel__run_ioctl(struct perf_evsel *evsel, int ncpus, int nthrea
 	return 0;
 }
 
-int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
-			     const char *filter)
+int perf_evsel__apply_filter(struct perf_evsel *evsel, const char *filter)
 {
-	return perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+	return perf_evsel__run_ioctl(evsel,
 				     PERF_EVENT_IOC_SET_FILTER,
 				     (void *)filter);
 }
@@ -1122,20 +1118,14 @@ int perf_evsel__append_addr_filter(struct perf_evsel *evsel, const char *filter)
 
 int perf_evsel__enable(struct perf_evsel *evsel)
 {
-	int nthreads = thread_map__nr(evsel->threads);
-	int ncpus = cpu_map__nr(evsel->cpus);
-
-	return perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+	return perf_evsel__run_ioctl(evsel,
 				     PERF_EVENT_IOC_ENABLE,
 				     0);
 }
 
 int perf_evsel__disable(struct perf_evsel *evsel)
 {
-	int nthreads = thread_map__nr(evsel->threads);
-	int ncpus = cpu_map__nr(evsel->cpus);
-
-	return perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+	return perf_evsel__run_ioctl(evsel,
 				     PERF_EVENT_IOC_DISABLE,
 				     0);
 }
@@ -1185,15 +1175,12 @@ static void perf_evsel__free_config_terms(struct perf_evsel *evsel)
 	}
 }
 
-void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
+void perf_evsel__close_fd(struct perf_evsel *evsel)
 {
 	int cpu, thread;
 
-	if (evsel->system_wide)
-		nthreads = 1;
-
-	for (cpu = 0; cpu < ncpus; cpu++)
-		for (thread = 0; thread < nthreads; ++thread) {
+	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++)
+		for (thread = 0; thread < xyarray__max_y(evsel->fd); ++thread) {
 			close(FD(evsel, cpu, thread));
 			FD(evsel, cpu, thread) = -1;
 		}
@@ -1854,12 +1841,12 @@ out_close:
 	return err;
 }
 
-void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads)
+void perf_evsel__close(struct perf_evsel *evsel)
 {
 	if (evsel->fd == NULL)
 		return;
 
-	perf_evsel__close_fd(evsel, ncpus, nthreads);
+	perf_evsel__close_fd(evsel);
 	perf_evsel__free_fd(evsel);
 }
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index de03c18..351d3b2 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -226,7 +226,7 @@ const char *perf_evsel__group_name(struct perf_evsel *evsel);
 int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size);
 
 int perf_evsel__alloc_id(struct perf_evsel *evsel, int ncpus, int nthreads);
-void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
+void perf_evsel__close_fd(struct perf_evsel *evsel);
 
 void __perf_evsel__set_sample_bit(struct perf_evsel *evsel,
 				  enum perf_event_sample_format bit);
@@ -246,8 +246,7 @@ int perf_evsel__set_filter(struct perf_evsel *evsel, const char *filter);
 int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter);
 int perf_evsel__append_addr_filter(struct perf_evsel *evsel,
 				   const char *filter);
-int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
-			     const char *filter);
+int perf_evsel__apply_filter(struct perf_evsel *evsel, const char *filter);
 int perf_evsel__enable(struct perf_evsel *evsel);
 int perf_evsel__disable(struct perf_evsel *evsel);
 
@@ -257,7 +256,7 @@ int perf_evsel__open_per_thread(struct perf_evsel *evsel,
 				struct thread_map *threads);
 int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 		     struct thread_map *threads);
-void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads);
+void perf_evsel__close(struct perf_evsel *evsel);
 
 struct perf_sample;
 

[tip:perf/core] perf bpf: Tighten detection of BPF events

[tip-bot for Andi Kleen]

Commit-ID:  77d0871c76bad1093a3d86870fe76dd1ad0ca397
Gitweb:     http://git.kernel.org/tip/77d0871c76bad1093a3d86870fe76dd1ad0ca397
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:19 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 11:56:22 -0300

perf bpf: Tighten detection of BPF events

  perf stat -e cpu/uops_executed.core,cmask=1/

would be detected as a BPF source event because the .c matches the .c
source BPF pattern.

v2:

Originally I tried to use lex lookahead, but it doesn't seem to work.

This now extends the BPF pattern to match longer events, but then does
an extra check in the C code to reject BPF matches that do not end with
.c/.o/.obj

This uses REJECT, which makes the flex scanner slower, but that
shouldn't be a big problem for the perf events.

Committer testing:

  # perf trace -e write -e /home/acme/bpf/tracepoint.c cat /etc/passwd > /dev/null
     0.000 ( 0.006 ms): cat/18485 write(fd: 1, buf: 0x7f59eebe1000, count: 3494                         ) ...
     0.006 (         ): raw_syscalls:sys_enter:NR 1 (1, 7f59eebe1000, da6, 22, 7f59eebe0010, 0))
     0.008 (         ): perf_bpf_probe:_write:(ffffffff9626b2c0))
     0.000 ( 0.010 ms): cat/18485  ... [continued]: write()) = 3494
  #

It continues doing what was expected, i.e. identifying
/home/acme/bpf/tracepoint.c as a BPF event and activates the clang
machinery to build an eBPF object and then uses sys_bpf() to hook it up
to the raw_syscalls:sys_enter tracepoint, etc.

Andi forgot to add Wang to the CC list, fix it.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170811232634.30465-4-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.l | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 660fca0..c42edea 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -53,6 +53,21 @@ static int str(yyscan_t scanner, int token)
 	return token;
 }
 
+static bool isbpf(yyscan_t scanner)
+{
+	char *text = parse_events_get_text(scanner);
+	int len = strlen(text);
+
+	if (len < 2)
+		return false;
+	if ((text[len - 1] == 'c' || text[len - 1] == 'o') &&
+	    text[len - 2] == '.')
+		return true;
+	if (len > 4 && !strcmp(text + len - 4, ".obj"))
+		return true;
+	return false;
+}
+
 /*
  * This function is called when the parser gets two kind of input:
  *
@@ -136,8 +151,8 @@ do {							\
 group		[^,{}/]*[{][^}]*[}][^,{}/]*
 event_pmu	[^,{}/]+[/][^/]*[/][^,{}/]*
 event		[^,{}/]+
-bpf_object	[^,{}]+\.(o|bpf)
-bpf_source	[^,{}]+\.c
+bpf_object	[^,{}]+\.(o|bpf)[a-zA-Z0-9._]*
+bpf_source	[^,{}]+\.c[a-zA-Z0-9._]*
 
 num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
@@ -307,8 +322,8 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 {num_hex}		{ return value(yyscanner, 16); }
 
 {modifier_event}	{ return str(yyscanner, PE_MODIFIER_EVENT); }
-{bpf_object}		{ return str(yyscanner, PE_BPF_OBJECT); }
-{bpf_source}		{ return str(yyscanner, PE_BPF_SOURCE); }
+{bpf_object}		{ if (!isbpf(yyscanner)) REJECT; return str(yyscanner, PE_BPF_OBJECT); }
+{bpf_source}		{ if (!isbpf(yyscanner)) REJECT; return str(yyscanner, PE_BPF_SOURCE); }
 {name}			{ return pmu_str_check(yyscanner); }
 "/"			{ BEGIN(config); return '/'; }
 -			{ return '-'; }

[tip:perf/core] perf tools: Add utility function to detect SMT status

[tip-bot for Andi Kleen]

Commit-ID:  de5077c4e38f2a51f50d28bdd5e4a0f14b3d16ff
Gitweb:     http://git.kernel.org/tip/de5077c4e38f2a51f50d28bdd5e4a0f14b3d16ff
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:22 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 12:09:04 -0300

perf tools: Add utility function to detect SMT status

Add an smt_on() function to return if SMT is enabled or disabled.  Used
in the next patch.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170811232634.30465-7-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/Build |  1 +
 tools/perf/util/smt.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/smt.h |  6 ++++++
 3 files changed, 51 insertions(+)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 8d49a98..94518c1 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -22,6 +22,7 @@ libperf-y += rbtree.o
 libperf-y += libstring.o
 libperf-y += bitmap.o
 libperf-y += hweight.o
+libperf-y += smt.o
 libperf-y += quote.o
 libperf-y += strbuf.o
 libperf-y += string.o
diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
new file mode 100644
index 0000000..453f6f6
--- /dev/null
+++ b/tools/perf/util/smt.c
@@ -0,0 +1,44 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <linux/bitops.h>
+#include "api/fs/fs.h"
+#include "smt.h"
+
+int smt_on(void)
+{
+	static bool cached;
+	static int cached_result;
+	int cpu;
+	int ncpu;
+
+	if (cached)
+		return cached_result;
+
+	ncpu = sysconf(_SC_NPROCESSORS_CONF);
+	for (cpu = 0; cpu < ncpu; cpu++) {
+		unsigned long long siblings;
+		char *str;
+		size_t strlen;
+		char fn[256];
+
+		snprintf(fn, sizeof fn,
+			"devices/system/cpu/cpu%d/topology/thread_siblings",
+			cpu);
+		if (sysfs__read_str(fn, &str, &strlen) < 0)
+			continue;
+		/* Entry is hex, but does not have 0x, so need custom parser */
+		siblings = strtoull(str, NULL, 16);
+		free(str);
+		if (hweight64(siblings) > 1) {
+			cached_result = 1;
+			cached = true;
+			break;
+		}
+	}
+	if (!cached) {
+		cached_result = 0;
+		cached = true;
+	}
+	return cached_result;
+}
diff --git a/tools/perf/util/smt.h b/tools/perf/util/smt.h
new file mode 100644
index 0000000..b8414b7
--- /dev/null
+++ b/tools/perf/util/smt.h
@@ -0,0 +1,6 @@
+#ifndef SMT_H
+#define SMT_H 1
+
+int smt_on(void);
+
+#endif

[tip:perf/core] perf tools: Expression parser enhancements for metrics

[tip-bot for Andi Kleen]

Commit-ID:  d73bad06851b8d5745c11dd4ab789464f6c71b39
Gitweb:     http://git.kernel.org/tip/d73bad06851b8d5745c11dd4ab789464f6c71b39
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:23 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 12:15:53 -0300

perf tools: Expression parser enhancements for metrics

Enhance the expression parser for more complex metric formulas.

- Support python style IF ELSE operators
- Add an #SMT_On magic variable for formulas that depend on the SMT
status.

Example: 4 *( CPU_CLK_UNHALTED.THREAD_ANY / 2 ) if #SMT_on else cycles

- Support MIN/MAX operations

Example: min(1 , IDQ.MITE_UOPS / ( UPI * 16 * ( ICACHE.HIT + ICACHE.MISSES ) / 4.0 ) )

This is useful to fix up problems caused by multiplexing.

- Support | & ^ operators
- Minor cleanups and fixes
- Support an \ escape for operators. This allows to specify event names
like c2-residency
- Support @ as an alternative for / to be able to specify pmus without
conflicts with operators (like msr/tsc/ as msr@tsc@)

Example: (cstate_core@c3\\-residency@ / msr@tsc@) * 100

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170811232634.30465-8-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/expr.c |  5 ++++
 tools/perf/util/expr.y  | 61 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index e939032..cb251bf 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -31,6 +31,11 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
 	ret |= test(&ctx, "(BAR/2)%2", 1);
 	ret |= test(&ctx, "1 - -4",  5);
 	ret |= test(&ctx, "(FOO-1)*2 + (BAR/2)%2 - -4",  5);
+	ret |= test(&ctx, "1-1 | 1", 1);
+	ret |= test(&ctx, "1-1 & 1", 0);
+	ret |= test(&ctx, "min(1,2) + 1", 2);
+	ret |= test(&ctx, "max(1,2) + 1", 3);
+	ret |= test(&ctx, "1+1 if 3*4 else 0", 2);
 
 	if (ret)
 		return ret;
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 953e65b..5753c4f 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -4,6 +4,7 @@
 #include "util/debug.h"
 #define IN_EXPR_Y 1
 #include "expr.h"
+#include "smt.h"
 #include <string.h>
 
 #define MAXIDLEN 256
@@ -22,13 +23,15 @@
 
 %token <num> NUMBER
 %token <id> ID
+%token MIN MAX IF ELSE SMT_ON
+%left MIN MAX IF
 %left '|'
 %left '^'
 %left '&'
 %left '-' '+'
 %left '*' '/' '%'
 %left NEG NOT
-%type <num> expr
+%type <num> expr if_expr
 
 %{
 static int expr__lex(YYSTYPE *res, const char **pp);
@@ -57,7 +60,12 @@ static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
 %}
 %%
 
-all_expr: expr			{ *final_val = $1; }
+all_expr: if_expr			{ *final_val = $1; }
+	;
+
+if_expr:
+	expr IF expr ELSE expr { $$ = $3 ? $1 : $5; }
+	| expr
 	;
 
 expr:	  NUMBER
@@ -66,13 +74,19 @@ expr:	  NUMBER
 					YYABORT;
 				  }
 				}
+	| expr '|' expr		{ $$ = (long)$1 | (long)$3; }
+	| expr '&' expr		{ $$ = (long)$1 & (long)$3; }
+	| expr '^' expr		{ $$ = (long)$1 ^ (long)$3; }
 	| expr '+' expr		{ $$ = $1 + $3; }
 	| expr '-' expr		{ $$ = $1 - $3; }
 	| expr '*' expr		{ $$ = $1 * $3; }
 	| expr '/' expr		{ if ($3 == 0) YYABORT; $$ = $1 / $3; }
 	| expr '%' expr		{ if ((long)$3 == 0) YYABORT; $$ = (long)$1 % (long)$3; }
 	| '-' expr %prec NEG	{ $$ = -$2; }
-	| '(' expr ')'		{ $$ = $2; }
+	| '(' if_expr ')'	{ $$ = $2; }
+	| MIN '(' expr ',' expr ')' { $$ = $3 < $5 ? $3 : $5; }
+	| MAX '(' expr ',' expr ')' { $$ = $3 > $5 ? $3 : $5; }
+	| SMT_ON		 { $$ = smt_on() > 0; }
 	;
 
 %%
@@ -82,13 +96,47 @@ static int expr__symbol(YYSTYPE *res, const char *p, const char **pp)
 	char *dst = res->id;
 	const char *s = p;
 
-	while (isalnum(*p) || *p == '_' || *p == '.') {
+	if (*p == '#')
+		*dst++ = *p++;
+
+	while (isalnum(*p) || *p == '_' || *p == '.' || *p == ':' || *p == '@' || *p == '\\') {
 		if (p - s >= MAXIDLEN)
 			return -1;
-		*dst++ = *p++;
+		/*
+		 * Allow @ instead of / to be able to specify pmu/event/ without
+		 * conflicts with normal division.
+		 */
+		if (*p == '@')
+			*dst++ = '/';
+		else if (*p == '\\')
+			*dst++ = *++p;
+		else
+			*dst++ = *p;
+		p++;
 	}
 	*dst = 0;
 	*pp = p;
+	dst = res->id;
+	switch (dst[0]) {
+	case 'm':
+		if (!strcmp(dst, "min"))
+			return MIN;
+		if (!strcmp(dst, "max"))
+			return MAX;
+		break;
+	case 'i':
+		if (!strcmp(dst, "if"))
+			return IF;
+		break;
+	case 'e':
+		if (!strcmp(dst, "else"))
+			return ELSE;
+		break;
+	case '#':
+		if (!strcasecmp(dst, "#smt_on"))
+			return SMT_ON;
+		break;
+	}
 	return ID;
 }
 
@@ -102,6 +150,7 @@ static int expr__lex(YYSTYPE *res, const char **pp)
 		p++;
 	s = p;
 	switch (*p++) {
+	case '#':
 	case 'a' ... 'z':
 	case 'A' ... 'Z':
 		return expr__symbol(res, p - 1, pp);
@@ -151,7 +200,7 @@ int expr__find_other(const char *p, const char *one, const char ***other,
 			err = 0;
 			break;
 		}
-		if (tok == ID && strcasecmp(one, val.id)) {
+		if (tok == ID && (!one || strcasecmp(one, val.id))) {
 			if (num_other >= EXPR_MAX_OTHER - 1) {
 				pr_debug("Too many extra events in %s\n", orig);
 				break;

[tip:perf/core] perf tools: Increase maximum number of events in expressions

[tip-bot for Andi Kleen]

Commit-ID:  8d3db2b97f5cb22cc589ba1e8e5232a26bfeb5d6
Gitweb:     http://git.kernel.org/tip/8d3db2b97f5cb22cc589ba1e8e5232a26bfeb5d6
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:24 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 12:19:05 -0300

perf tools: Increase maximum number of events in expressions

Some of the upcoming metrics need more than 8 events. Increase the maximum
number the parser supports.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170811232634.30465-9-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/expr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9c2760a..400ef9e 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -1,7 +1,7 @@
 #ifndef PARSE_CTX_H
 #define PARSE_CTX_H 1
 
-#define EXPR_MAX_OTHER 8
+#define EXPR_MAX_OTHER 15
 #define MAX_PARSE_ID EXPR_MAX_OTHER
 
 struct parse_id {

[tip:perf/core] perf tools: Dedup events in expression parsing

[tip-bot for Andi Kleen]

Commit-ID:  d66dccdb13c9748f27d8401b45c027953fc833e8
Gitweb:     http://git.kernel.org/tip/d66dccdb13c9748f27d8401b45c027953fc833e8
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 11 Aug 2017 16:26:25 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 Aug 2017 12:19:08 -0300

perf tools: Dedup events in expression parsing

Avoid adding redundant events while parsing an expression.  When we add
an "other" event check first if it already exists.

v2: Fix perf test failure.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170811232634.30465-10-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/expr.y | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 5753c4f..432b856 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -181,6 +181,19 @@ void expr__ctx_init(struct parse_ctx *ctx)
 	ctx->num_ids = 0;
 }
 
+static bool already_seen(const char *val, const char *one, const char **other,
+			 int num_other)
+{
+	int i;
+
+	if (one && !strcasecmp(one, val))
+		return true;
+	for (i = 0; i < num_other; i++)
+		if (!strcasecmp(other[i], val))
+			return true;
+	return false;
+}
+
 int expr__find_other(const char *p, const char *one, const char ***other,
 		     int *num_otherp)
 {
@@ -200,7 +213,7 @@ int expr__find_other(const char *p, const char *one, const char ***other,
 			err = 0;
 			break;
 		}
-		if (tok == ID && (!one || strcasecmp(one, val.id))) {
+		if (tok == ID && !already_seen(val.id, one, *other, num_other)) {
 			if (num_other >= EXPR_MAX_OTHER - 1) {
 				pr_debug("Too many extra events in %s\n", orig);
 				break;

Re: [PATCH v2 14/19] perf, tools, stat: Support JSON metrics in perf stat

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:29PM -0700, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index ac16a9db1fb5..4f049285e3ac 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -516,16 +516,8 @@ char * __weak get_cpuid_str(void)
>  	return NULL;
>  }
>  
> -/*
> - * From the pmu_events_map, find the table of PMU events that corresponds
> - * to the current running CPU. Then, add all PMU events from that table
> - * as aliases.
> - */
> -static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
> +char *perf_pmu__getcpuid(void)

please add this function in separate patch

>  {
> -	int i;
> -	struct pmu_events_map *map;
> -	struct pmu_event *pe;
>  	char *cpuid;

also you could make cpuid static and not rerun the check
every time it's called

jirka

>  	static bool printed;
>  
> @@ -535,12 +527,26 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
>  	if (!cpuid)
>  		cpuid = get_cpuid_str();
>  	if (!cpuid)
> -		return;
> +		return NULL;
>  
>  	if (!printed) {
>  		pr_debug("Using CPUID %s\n", cpuid);
>  		printed = true;
>  	}
> +	return cpuid;
> +}

Re: [PATCH v2 14/19] perf, tools, stat: Support JSON metrics in perf stat

[Jiri Olsa]

On Mon, Aug 28, 2017 at 10:13:35AM +0200, Jiri Olsa wrote:
> On Fri, Aug 11, 2017 at 04:26:29PM -0700, Andi Kleen wrote:
> 
> SNIP
> 
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index ac16a9db1fb5..4f049285e3ac 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -516,16 +516,8 @@ char * __weak get_cpuid_str(void)
> >  	return NULL;
> >  }
> >  
> > -/*
> > - * From the pmu_events_map, find the table of PMU events that corresponds
> > - * to the current running CPU. Then, add all PMU events from that table
> > - * as aliases.
> > - */
> > -static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
> > +char *perf_pmu__getcpuid(void)
> 
> please add this function in separate patch
> 
> >  {
> > -	int i;
> > -	struct pmu_events_map *map;
> > -	struct pmu_event *pe;
> >  	char *cpuid;
> 
> also you could make cpuid static and not rerun the check
> every time it's called
> 
> jirka
> 
> >  	static bool printed;
> >  
> > @@ -535,12 +527,26 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)

also pmu_add_cpu_aliases could use the find_map,
which duplicates the beginning of this function

jirka

Re: [PATCH v2 14/19] perf, tools, stat: Support JSON metrics in perf stat

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:29PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add generic support for standalone metrics specified in JSON files
> to perf stat. A metric is a formula that uses multiple events
> to compute a higher level result (e.g. IPC).
> 
> Previously metrics were always tied to an event and automatically
> enabled with that event. But now change it that we can have
> standalone metrics. They are in the same JSON data structure
> as events, but don't have an event name.
> 
> We also allow to organize the metrics in metric groups, which
> allows a short cut to select several related metrics at once.
> 
> Add a new -M / --metrics option to perf stat that adds the metrics
> or metric groups specified.
> 
> Add the core code to manage and parse the metric groups. They
> are collected from the JSON data structures into a separate rblist.
> When computing shadow values look for metrics in that list.
> Then they are computed using the existing saved values infrastructure
> in stat-shadow.c
> 
> The actual JSON metrics are in a separate pull request.

would need this for testing.. could you please put it somewhere?

thanks,
jirka

Re: [PATCH v2 12/19] perf, tools, stat: Factor out generic metric printing

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:27PM -0700, Andi Kleen wrote:

SNIP

> +static void generic_metric(const char *metric_expr,
> +			   struct perf_evsel **metric_events,
> +			   char *name,
> +			   const char *metric_name,
> +			   double avg,
> +			   int cpu,
> +			   int ctx,
> +			   struct perf_stat_output_ctx *out)
> +{
> +	print_metric_t print_metric = out->print_metric;
> +	struct parse_ctx pctx;
> +	double ratio;
> +	int i;
> +	void *ctxp = out->ctx;
> +
> +	expr__ctx_init(&pctx);
> +	expr__add_id(&pctx, name, avg);
> +	for (i = 0; metric_events[i]; i++) {
> +		struct saved_value *v;
> +
> +		v = saved_value_lookup(metric_events[i], cpu, ctx, false);
> +		if (!v)
> +			break;
> +		expr__add_id(&pctx, metric_events[i]->name, avg_stats(&v->stats));
> +	}
> +	if (!metric_events[i]) {
> +		const char *p = metric_expr;
> +
> +		if (expr__parse(&ratio, &pctx, &p) == 0)
> +			print_metric(ctxp, NULL, "%8.1f",
> +				metric_name ?
> +				metric_name :
> +			out->force_header ?  name : "",

nit, missing tab ^^^

jirka

Re: [PATCH v2 17/19] perf, tools, stat: Support duration_time for metrics

[Jiri Olsa]

On Fri, Aug 11, 2017 at 04:26:32PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Some of the metrics formulas (like GFLOPs) need to know how long
> the measurement period is. Support an internal event called duration_time, which
> reports time in second. It maps to the dummy event, but is
> special cased for statistics to report the walltime duration.
> 
> So far it is not printed, but only used internally for metrics.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/parse-events.l |  1 +
>  tools/perf/util/stat-shadow.c  | 17 +++++++++++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index fdb5bb52f01f..ea2426daf7e8 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -288,6 +288,7 @@ cpu-migrations|migrations			{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COU
>  alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
>  emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
>  dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
> +duration_time					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }

could you please use '-' (current style) instead of '_'

thanks,
jirka

Re: [PATCH v2 14/19] perf, tools, stat: Support JSON metrics in perf stat

[Andi Kleen]

On Mon, Aug 28, 2017 at 11:19:21AM +0200, Jiri Olsa wrote:
> On Fri, Aug 11, 2017 at 04:26:29PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Add generic support for standalone metrics specified in JSON files
> > to perf stat. A metric is a formula that uses multiple events
> > to compute a higher level result (e.g. IPC).
> > 
> > Previously metrics were always tied to an event and automatically
> > enabled with that event. But now change it that we can have
> > standalone metrics. They are in the same JSON data structure
> > as events, but don't have an event name.
> > 
> > We also allow to organize the metrics in metric groups, which
> > allows a short cut to select several related metrics at once.
> > 
> > Add a new -M / --metrics option to perf stat that adds the metrics
> > or metric groups specified.
> > 
> > Add the core code to manage and parse the metric groups. They
> > are collected from the JSON data structures into a separate rblist.
> > When computing shadow values look for metrics in that list.
> > Then they are computed using the existing saved values infrastructure
> > in stat-shadow.c
> > 
> > The actual JSON metrics are in a separate pull request.
> 
> would need this for testing.. could you please put it somewhere?

It was in the 0/0 (which I forgot to post for the second rev)

---

Support standalone metrics and metric groups for perf v2
  
Add generic support for standalone metrics specified in JSON files
to perf stat. A metric is a formula that uses multiple events
to compute a higher level result (e.g. IPC). 

For more complex metrics we need to have micro architecture
specific knowledge, so it makes sense to tie metrics to
JSON event lists.
    
Previously metrics were always tied to an event and automatically
enabled with that event. But now change it that we can have
standalone metrics. They are in the same JSON data structure
as events, but don't have an event name, only a metric name.
    
We also allow to organize the metrics in metric groups, which
allows a short cut to select several related metrics at once.

This patch kit adds the code to perf to manage metric groups

The first few patches are generic bug fixes and can be applied
directly. Then there is a 'weak group' feature that is useful
independently from metrics. After there are metrics specific
patches.

The patches are available in

   git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git perf/metric-group-5

The actual Intel JSON metrics are available in git as a separate pull
request in 

   git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git perf/intel-json-metrics-2

Some example output:
   % perf list metricgroup
    ..
    Metric Groups:
    
    DSB:
      DSB_Coverage
            [Fraction of Uops delivered by the DSB (aka Decoded Icache; or Uop Cache)]
    FLOPS:
      GFLOPs
            [Giga Floating Point Operations Per Second]
    Frontend:
      IFetch_Line_Utilization
            [Rough Estimation of fraction of fetched lines bytes that were likely consumed by program instructions]
    Frontend_Bandwidth:
      DSB_Coverage
            [Fraction of Uops delivered by the DSB (aka Decoded Icache; or Uop Cache)]
    Memory_BW:
      MLP
            [Memory-Level-Parallelism (average number of L1 miss demand load when there is at least 1 such miss)]

   % perf stat -M Summary --metric-only -a sleep 1
    
     Performance counter stats for 'system wide':
    
    Instructions                              CLKS                 CPU_Utilization      GFLOPs               SMT_2T_Utilization   Kernel_Utilization
    317614222.0                              1392930775.0             0.0                 0.0                 0.2                 0.1
    
           1.001497549 seconds time elapsed
    
   % perf stat -M GFLOPs flops
    
     Performance counter stats for 'flops':
    
         3,999,541,471      fp_comp_ops_exe.sse_scalar_single #      1.2 GFLOPs                   (66.65%)
                    14      fp_comp_ops_exe.sse_scalar_double                                     (66.65%)
                     0      fp_comp_ops_exe.sse_packed_double                                     (66.67%)
                     0      fp_comp_ops_exe.sse_packed_single                                     (66.70%)
                     0      simd_fp_256.packed_double                                     (66.70%)
                     0      simd_fp_256.packed_single                                     (66.67%)
    
           3.238372845 seconds time elapsed

Re: [PATCH v2 17/19] perf, tools, stat: Support duration_time for metrics

[Andi Kleen]

On Mon, Aug 28, 2017 at 11:19:31AM +0200, Jiri Olsa wrote:
> On Fri, Aug 11, 2017 at 04:26:32PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Some of the metrics formulas (like GFLOPs) need to know how long
> > the measurement period is. Support an internal event called duration_time, which
> > reports time in second. It maps to the dummy event, but is
> > special cased for statistics to report the walltime duration.
> > 
> > So far it is not printed, but only used internally for metrics.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > ---
> >  tools/perf/util/parse-events.l |  1 +
> >  tools/perf/util/stat-shadow.c  | 17 +++++++++++++----
> >  2 files changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index fdb5bb52f01f..ea2426daf7e8 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -288,6 +288,7 @@ cpu-migrations|migrations			{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COU
> >  alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
> >  emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
> >  dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
> > +duration_time					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
> 
> could you please use '-' (current style) instead of '_'

This would require escaping in the expressions because - is a vaid
operator. It looks nicer with the _ and it's only used internally
anyways.

-Andi