Support standalone metrics and metric groups for perf

Support standalone metrics and metric groups for perf

[Andi Kleen]

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

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%)
                     0      duration_time
    
           3.238372845 seconds time elapsed

v1: Initial post

[PATCH v1 01/15] perf, tools, stat: 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.

Just make sure to always use the same dummy cpu map following
the same logic as the open call.

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 48ac53b199fc..97d6b6c42014 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -715,6 +715,8 @@ static int __run_perf_stat(int argc, const char **argv)
 	 * group leaders.
 	 */
 	read_counters();
+	if (!target__has_cpu(&target))
+		evsel_list->cpus = cpu_map__dummy_new();
 	perf_evlist__close(evsel_list);
 
 	return WEXITSTATUS(status);
-- 
2.9.4

Re: [PATCH v1 01/15] perf, tools, stat: Fix buffer overflow while freeing events

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:01PM -0700, Andi Kleen wrote:

SNIP

> 
> 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.
> 
> Just make sure to always use the same dummy cpu map following
> the same logic as the open call.
> 
> 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 48ac53b199fc..97d6b6c42014 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -715,6 +715,8 @@ static int __run_perf_stat(int argc, const char **argv)
>  	 * group leaders.
>  	 */
>  	read_counters();
> +	if (!target__has_cpu(&target))
> +		evsel_list->cpus = cpu_map__dummy_new();

you're leaking evsel_list->cpus right here..

I can see there's the issue when we mix system_wide event
(with cpumask defined) and normal event:

  - we open such group as per_thread events (not system_wide),
    forcing both evsel->fd xyarray to be allocated from dummy
    cpus (with ncpus == 1)

  - but when we call perf_evlist__close we take ncpus from
      int n = evsel->cpus ? evsel->cpus->nr : ncpus;

    which is wrong for system_wide event and will
    cause the xyarray out of bounds access 

I can see the solution either in:
  1) keeping the bounds for xyarray in it and use it for iterations
  2) or forcing system_wide target if there's single system_wide event
     specified (patch below)

but not sure there's any sense in meassuting system_wide
event in non system_wide mode (ad 1).. thoughts?

thanks,
jirka


---
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 866da7aa54bf..9ccb4d671568 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2540,8 +2540,8 @@ static void setup_system_wide(int forks)
 	 * conditions is met:
 	 *
 	 *   - there's no workload specified
-	 *   - there is workload specified but all requested
-	 *     events are system wide events
+	 *   - there is workload specified and one
+	 *     of the events is system wide
 	 */
 	if (!target__none(&target))
 		return;
@@ -2552,12 +2552,11 @@ static void setup_system_wide(int forks)
 		struct perf_evsel *counter;
 
 		evlist__for_each_entry(evsel_list, counter) {
-			if (!counter->system_wide)
+			if (counter->system_wide) {
+				target.system_wide = true;
 				return;
+			}
 		}
-
-		if (evsel_list->nr_entries)
-			target.system_wide = true;
 	}
 }
 

[PATCH v1 02/15] 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.

Add lookahead to the BPF patterns and reject them if they are followed
by more letters.

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

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 660fca05bc93..7fa3f2e851b0 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -182,9 +182,13 @@ modifier_bp	[rwx]{1,3}
 			REWIND(0);
 		}
 
+({bpf_object}|{bpf_source})/[^a-z] {
+			BEGIN(INITIAL);
+			REWIND(1);
+			return PE_EVENT_NAME;
+		}
+
 {event_pmu}	|
-{bpf_object}	|
-{bpf_source}	|
 {event}		{
 			BEGIN(INITIAL);
 			REWIND(1);
@@ -307,8 +311,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}/[^a-z]	{ return str(yyscanner, PE_BPF_OBJECT); }
+{bpf_source}/[^a-z]	{ return str(yyscanner, PE_BPF_SOURCE); }
 {name}			{ return pmu_str_check(yyscanner); }
 "/"			{ BEGIN(config); return '/'; }
 -			{ return '-'; }
-- 
2.9.4

Re: [PATCH v1 02/15] perf, tools: Tighten detection of BPF events

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:02PM -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.
> 
> Add lookahead to the BPF patterns and reject them if they are followed
> by more letters.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

I don't have the llvm/bpf toolchain, so can't test that side,
but Arnaldo will run it for sure ;-) anyway looks ok to me:

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

thanks,
jirka

> ---
>  tools/perf/util/parse-events.l | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 660fca05bc93..7fa3f2e851b0 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -182,9 +182,13 @@ modifier_bp	[rwx]{1,3}
>  			REWIND(0);
>  		}
>  
> +({bpf_object}|{bpf_source})/[^a-z] {
> +			BEGIN(INITIAL);
> +			REWIND(1);
> +			return PE_EVENT_NAME;
> +		}
> +
>  {event_pmu}	|
> -{bpf_object}	|
> -{bpf_source}	|
>  {event}		{
>  			BEGIN(INITIAL);
>  			REWIND(1);
> @@ -307,8 +311,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}/[^a-z]	{ return str(yyscanner, PE_BPF_OBJECT); }
> +{bpf_source}/[^a-z]	{ return str(yyscanner, PE_BPF_SOURCE); }
>  {name}			{ return pmu_str_check(yyscanner); }
>  "/"			{ BEGIN(config); return '/'; }
>  -			{ return '-'; }
> -- 
> 2.9.4
> 

Re: [PATCH v1 02/15] perf, tools: Tighten detection of BPF events

[Arnaldo Carvalho de Melo]

CCing eBPF guys and linux-perf-users, as it shows an example of
attaching a eBPF proggie to a tracepoint and reading its result with the
default BPF tracepoint beautifier in a simple, compact 'perf trace'
command line, together with asking for the 'write' syscall to be as well
beautified.x

Em Wed, Aug 02, 2017 at 09:35:18AM +0200, Jiri Olsa escreveu:
> On Mon, Jul 24, 2017 at 04:40:02PM -0700, Andi Kleen wrote:
> > 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.

> > Add lookahead to the BPF patterns and reject them if they are followed
> > by more letters.
 
> I don't have the llvm/bpf toolchain, so can't test that side,
> but Arnaldo will run it for sure ;-) anyway looks ok to me:
 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Sure, it regresses:

Before the patch:

  # perf trace -e write -e /home/acme/bpf/tracepoint.c cat /etc/passwd
  <SNIP>
     0.000 ( 0.010 ms): cat/6676 write(fd: 1, buf: 0x7f5fe3065000, count: 3494) ...
     0.010 (         ): raw_syscalls:sys_enter:NR 1 (1, 7f5fe3065000, da6, 7f5fe3064010, ffffffffffffffff, 0))
     0.013 (         ): perf_bpf_probe:_write:(ffffffffa625ea60))
     0.000 ( 0.208 ms): cat/6676  ... [continued]: write()) = 3494
  #

And after:

  # perf trace -e write -e /home/acme/bpf/tracepoint.c cat /etc/passwd
  invalid or unsupported event: '/home/acme/bpf/tracepoint.c'
  Run 'perf list' for a list of valid events

   Usage: perf trace [<options>] [<command>]
      or: perf trace [<options>] -- <command> [<options>]
      or: perf trace record [<options>] [<command>]
      or: perf trace record [<options>] -- <command> [<options>]

      -e, --event <event>   event/syscall selector. use 'perf list' to list available events
  #

For testing, please install clang and then use that proggie:

# cat /home/acme/bpf/tracepoint.c 
#include <uapi/linux/bpf.h>
#define SEC(NAME) __attribute__((section(NAME), used))
SEC("raw_syscalls:sys_enter")
int func(void *ctx)
{
      /*
       * /sys/kernel/debug/tracing/events/raw_syscalls/sys_enter/format:
       * ...
       * field:long id;	offset:8;	size:8;	signed:1;
       * ...
       * ctx + 8 select 'id'
       */
      u64 id = *((u64 *)(ctx + 8));
      if (id == 1)
      	return 1;
      return 0;
}
SEC("_write=sys_write")
int _write(void *ctx)
{
      return 1;
}
char _license[] SEC("license") = "GPL";
int _version SEC("version") = LINUX_VERSION_CODE;
#

BTW, count: 3494 = 0xda6 :-)

Cheers,

- Arnaldo

[PATCH v1 03/15] 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.

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

Re: [PATCH v1 03/15] perf, tools, stat: Fix saved values rbtree lookup

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:03PM -0700, Andi Kleen wrote:
> 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.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

thanks,
jirka

Re: [PATCH v1 03/15] perf, tools, stat: Fix saved values rbtree lookup

[Arnaldo Carvalho de Melo]

Em Wed, Aug 02, 2017 at 09:35:13AM +0200, Jiri Olsa escreveu:
> On Mon, Jul 24, 2017 at 04:40:03PM -0700, Andi Kleen wrote:
> > 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.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Applied.

- Arnaldo

[tip:perf/core] perf stat: Fix saved values rbtree lookup

[tip-bot for Andi Kleen]

Commit-ID:  5e97665f91d343b5bcf1f92249e45e18987ffd00
Gitweb:     http://git.kernel.org/tip/5e97665f91d343b5bcf1f92249e45e18987ffd00
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Mon, 24 Jul 2017 16:40:03 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 11 Aug 2017 10:42:52 -0300

perf stat: Fix saved values rbtree lookup

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.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170724234015.5165-4-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 719d6cb..a04cf56 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,

[PATCH v1 04/15] 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

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-list.txt |  1 +
 tools/perf/builtin-stat.c              | 28 +++++++++++++++++++++++++++-
 tools/perf/util/evsel.h                |  1 +
 tools/perf/util/parse-events.c         |  8 +++++++-
 tools/perf/util/parse-events.l         |  2 +-
 5 files changed, 37 insertions(+), 3 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 97d6b6c42014..551ed938e05c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -564,7 +564,7 @@ static int __run_perf_stat(int argc, const char **argv)
 	int interval = stat_config.interval;
 	char msg[BUFSIZ];
 	unsigned long long t0, t1;
-	struct perf_evsel *counter;
+	struct perf_evsel *counter, *c2, *leader;
 	struct timespec ts;
 	size_t l;
 	int status = 0;
@@ -595,6 +595,32 @@ 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) {
+				bool is_open = true;
+
+				pr_debug("Weak group for %s/%d failed\n",
+						counter->leader->name, counter->nr_members);
+				leader = counter->leader;
+				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->cpus ? c2->cpus->nr :
+								cpu_map__nr(evsel_list->cpus),
+								thread_map__nr(evsel_list->threads));
+						c2->leader = c2;
+						c2->nr_members = 0;
+					}
+				}
+				counter = leader;
+				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 fb40ca3c6519..4a8456d0501c 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 7fa3f2e851b0..07ee5bb3a8b0 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -146,7 +146,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

Re: [PATCH v1 04/15] perf, tools: Support weak groups

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:04PM -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
> 
> 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%)

looks handy, few comments below

thanks,
jirka

> 
> The extra event scheduled with some extra multiplexing
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---

SNIP

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 97d6b6c42014..551ed938e05c 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -564,7 +564,7 @@ static int __run_perf_stat(int argc, const char **argv)
>  	int interval = stat_config.interval;
>  	char msg[BUFSIZ];
>  	unsigned long long t0, t1;
> -	struct perf_evsel *counter;
> +	struct perf_evsel *counter, *c2, *leader;
>  	struct timespec ts;
>  	size_t l;
>  	int status = 0;
> @@ -595,6 +595,32 @@ 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) {

could you please put this de-grouping code into a function?

> +				bool is_open = true;
> +
> +				pr_debug("Weak group for %s/%d failed\n",
> +						counter->leader->name, counter->nr_members);
> +				leader = counter->leader;
> +				evlist__for_each_entry(evsel_list, c2) {

we have for_each_group_member

> +					if (c2 == counter)
> +						is_open = false;
> +					if (c2->leader == leader) {
> +						if (is_open)
> +							perf_evsel__close(c2,
> +								c2->cpus ? c2->cpus->nr :
> +								cpu_map__nr(evsel_list->cpus),
> +								thread_map__nr(evsel_list->threads));
> +						c2->leader = c2;
> +						c2->nr_members = 0;
> +					}
> +				}
> +				counter = leader;
> +				goto try_again;
> +			}
> +
>  			/*
>  			 * PPC returns ENXIO for HW counters until 2.6.37
>  			 * (behavior changed with commit b0a873e).

SNIP

[PATCH v1 05/15] perf, tools: Add missing newline to expr parser error messages

[Andi Kleen]

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

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

Re: [PATCH v1 05/15] perf, tools: Add missing newline to expr parser error messages

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:05PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

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

thanks,
jirka

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

[tip:perf/core] perf tools: Add missing newline to expr parser error messages

[tip-bot for Andi Kleen]

Commit-ID:  c295036b6a2cf968ceac76d16d5fac43e22369f7
Gitweb:     http://git.kernel.org/tip/c295036b6a2cf968ceac76d16d5fac43e22369f7
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Mon, 24 Jul 2017 16:40:05 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 11 Aug 2017 10:42:53 -0300

perf tools: Add missing newline to expr parser error messages

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170724234015.5165-6-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 954556b..953e65b 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;
 				  }
 				}

[PATCH v1 06/15] 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 v1 07/15] 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
- Support MIN/MAX operations
- Support | & ^ operators
- Minor cleanups and fixes
- Add an #SMT_On magic variable for formulas that depend on the SMT
status.
- 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@)

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

Re: [PATCH v1 07/15] perf, tools: Expression parser enhancements for metrics

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:07PM -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
> - Support MIN/MAX operations
> - Support | & ^ operators
> - Minor cleanups and fixes
> - Add an #SMT_On magic variable for formulas that depend on the SMT
> status.
> - Support an \ escape for operators. This allows to specify event names
> like c2-residency

please state some examples of this

thanks,
jirka

> - Support @ as an alternative for / to be able to specify pmus without
> conflicts with operators (like msr/tsc/ as msr@tsc@)
> 
> 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 v1 08/15] 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 v1 09/15] 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.

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..f54928f00893 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 false;
+	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 v1 09/15] perf, tools: Dedup events in expression parsing

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:09PM -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.

hi,
got this:

[jolsa@krava perf]$ ./perf test expr -v
 6: Simple expression parser                   :
--- start ---
test child forked, pid 13589
syntax error
FAILED tests/expr.c:53 find other
test child finished with -1
---- end ----
Simple expression parser: FAILED!

jirka

[PATCH v1 10/15] 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 bd0aabb2bd0f..70cbd5bc4819 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 v1 11/15] 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 v1 12/15] 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

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/pmu.c                  |  31 +--
 tools/perf/util/pmu.h                  |   2 +
 tools/perf/util/stat-shadow.c          |  22 ++-
 tools/perf/util/stat.h                 |   4 +-
 8 files changed, 403 insertions(+), 16 deletions(-)
 create mode 100644 tools/perf/util/metricgroup.c

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 551ed938e05c..0dccae290b88 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,
 };
@@ -1203,7 +1206,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);
@@ -1534,7 +1537,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);
 }
@@ -1758,6 +1762,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"),
@@ -1823,6 +1834,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/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 b6cbb6e67167..1fe6b0db25a6 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,
@@ -669,13 +670,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]);
@@ -878,6 +882,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 7522bf10b03e..f0656c5eca57 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -90,9 +90,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 v1 13/15] 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)]

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          | 172 +++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.c         |   3 +
 4 files changed, 188 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..c6492faf1a01 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -210,6 +210,178 @@ 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);
+
+	return strcmp(a->name, entry);
+}
+
+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, name);
+	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
+					asprintf(&s, "%s\n\t[%s]",
+						 pe->metric_name, pe->desc);
+				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 v1 14/15] 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 1fe6b0db25a6..122f37b38a65 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));
@@ -864,7 +858,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];
@@ -893,7 +887,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 v1 15/15] perf, tools: Support duration_time

[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 07ee5bb3a8b0..f7bc74f8a615 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -277,6 +277,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 122f37b38a65..34ae2960730b 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

Re: [PATCH v1 15/15] perf, tools: Support duration_time

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:15PM -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 07ee5bb3a8b0..f7bc74f8a615 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -277,6 +277,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); }

not ideal to actualy create an kernel event for something we
need only access in userspace, but I could live with that..


>  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 122f37b38a65..34ae2960730b 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;

however this will enable the event value only for metric processing
and following stat command will return 0:

[jolsa@krava perf]$ ./perf stat -e duration_time kill
kill: not enough arguments

 Performance counter stats for 'kill':

                 0      duration_time:u                                             

       0.006556409 seconds time elapsed

perhaps we could substitute the value when we read the event

also walltime_nsecs_stats is only updated at the end of the
workload, so this won't work for interval.. but not sure this
is an issue though

jirka

Re: Support standalone metrics and metric groups for perf

[Jiri Olsa]

On Mon, Jul 24, 2017 at 04:40:00PM -0700, Andi Kleen wrote:
> 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-4
> 
> 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

hi,
looks like it's not pushed out?

[jolsa@krava perf]$ git remote -v | grep andi
andi    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git (fetch)
andi    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git (push)
[jolsa@krava perf]$ git remote update andi
Fetching andi
[jolsa@krava perf]$ git branch -r | grep metric-group
[jolsa@krava perf]$ git branch -r | grep intel-json-metrics-2
[jolsa@krava perf]$ 


thanks,
jirka

Re: Support standalone metrics and metric groups for perf

[Andi Kleen]

> [jolsa@krava perf]$ git remote -v | grep andi
> andi    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git (fetch)
> andi    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git (push)
> [jolsa@krava perf]$ git remote update andi
> Fetching andi
> [jolsa@krava perf]$ git branch -r | grep metric-group
> [jolsa@krava perf]$ git branch -r | grep intel-json-metrics-2
> [jolsa@krava perf]$ 

Pushed now.

-Andi

Re: Support standalone metrics and metric groups for perf

[Jiri Olsa]

On Wed, Jul 26, 2017 at 08:38:41AM -0700, Andi Kleen wrote:
> > [jolsa@krava perf]$ git remote -v | grep andi
> > andi    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git (fetch)
> > andi    git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git (push)
> > [jolsa@krava perf]$ git remote update andi
> > Fetching andi
> > [jolsa@krava perf]$ git branch -r | grep metric-group
> > [jolsa@krava perf]$ git branch -r | grep intel-json-metrics-2
> > [jolsa@krava perf]$ 
> 
> Pushed now.

  CC       util/metricgroup.o
util/metricgroup.c: In function ‘metricgroup__print’:
util/metricgroup.c:347:6: error: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Werror=unused-result]
      asprintf(&s, "%s\n\t[%s]",
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
        pe->metric_name, pe->desc);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


jirka

Support standalone metrics and metric groups for perf

[Andi Kleen]

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

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

v1: Initial post
v2: Address all review feedback (see individual patches)
BPF now works again.
Fix some bugs in perf list printing that I added last minute last time.
v3: Address all review feedback. Some patches are split. Rebased.
Not caching cpuids because it's too complicated.

Re: Support standalone metrics and metric groups for perf

[Jiri Olsa]

On Thu, Aug 31, 2017 at 12:40:25PM -0700, Andi Kleen wrote:

SNIP

> 
>    % 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%)

looks like some events are probably crossing some
output boundaries we have:

[jolsa@krava perf]$ sudo ./perf stat -M SMT -I 1000
#           time             counts unit events
     1.000565706        408,879,985      inst_retired.any          #      0.7 CoreIPC                  (66.68%)
     1.000565706      1,120,999,114      cpu_clk_unhalted.thread_any                                     (66.68%)
     1.000565706        701,285,312      cycles                                                        (66.68%)
     1.000565706      1,148,325,740      cpu_clk_unhalted.thread_any # 574162870.0 CORE_CLKS             (66.67%)
     1.000565706        711,565,247      cpu_clk_unhalted.thread                                       (66.66%)
     1.000565706         24,057,590      cpu_clk_thread_unhalted.one_thread_active #      0.3 SMT_2T_Utilization       (66.67%)
     1.000565706         65,753,475      cpu_clk_thread_unhalted.ref_xclk_any                                     (66.67%)
^C     1.349436822         21,198,385      inst_retired.any          #      0.1 CoreIPC                  (66.70%)
     1.349436822        112,740,282      cpu_clk_unhalted.thread_any                                     (66.70%)
     1.349436822         84,509,414      cycles                                                        (66.70%)
     1.349436822        108,181,315      cpu_clk_unhalted.thread_any # 54090657.5 CORE_CLKS              (66.62%)
     1.349436822         79,700,353      cpu_clk_unhalted.thread                                       (66.61%)
     1.349436822          3,911,698      cpu_clk_thread_unhalted.one_thread_active #      0.8 SMT_2T_Utilization       (66.69%)
     1.349436822         14,739,671      cpu_clk_thread_unhalted.ref_xclk_any                                     (66.69%)


could you please check on that and maybe shift the alignment for the longest name?

thanks,
jirka

Re: Support standalone metrics and metric groups for perf

[Jiri Olsa]

On Fri, Sep 01, 2017 at 07:26:18PM +0200, Jiri Olsa wrote:
> On Thu, Aug 31, 2017 at 12:40:25PM -0700, Andi Kleen wrote:
> 
> SNIP
> 
> > 
> >    % 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%)
> 
> looks like some events are probably crossing some
> output boundaries we have:
> 
> [jolsa@krava perf]$ sudo ./perf stat -M SMT -I 1000
> #           time             counts unit events
>      1.000565706        408,879,985      inst_retired.any          #      0.7 CoreIPC                  (66.68%)
>      1.000565706      1,120,999,114      cpu_clk_unhalted.thread_any                                     (66.68%)
>      1.000565706        701,285,312      cycles                                                        (66.68%)
>      1.000565706      1,148,325,740      cpu_clk_unhalted.thread_any # 574162870.0 CORE_CLKS             (66.67%)
>      1.000565706        711,565,247      cpu_clk_unhalted.thread                                       (66.66%)
>      1.000565706         24,057,590      cpu_clk_thread_unhalted.one_thread_active #      0.3 SMT_2T_Utilization       (66.67%)
>      1.000565706         65,753,475      cpu_clk_thread_unhalted.ref_xclk_any                                     (66.67%)
> ^C     1.349436822         21,198,385      inst_retired.any          #      0.1 CoreIPC                  (66.70%)
>      1.349436822        112,740,282      cpu_clk_unhalted.thread_any                                     (66.70%)
>      1.349436822         84,509,414      cycles                                                        (66.70%)
>      1.349436822        108,181,315      cpu_clk_unhalted.thread_any # 54090657.5 CORE_CLKS              (66.62%)
>      1.349436822         79,700,353      cpu_clk_unhalted.thread                                       (66.61%)
>      1.349436822          3,911,698      cpu_clk_thread_unhalted.one_thread_active #      0.8 SMT_2T_Utilization       (66.69%)
>      1.349436822         14,739,671      cpu_clk_thread_unhalted.ref_xclk_any                                     (66.69%)
> 
> 
> could you please check on that and maybe shift the alignment for the longest name?

other than this the rest of the patchset looks ok to me

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

thanks,
jirka

Re: Support standalone metrics and metric groups for perf

[Andi Kleen]

> could you please check on that and maybe shift the alignment for the longest name?

To make everything align would require shifting everything. So most
usages which don't have that wide events wouldn't fit into 80
character columns anymore. That would be worse.

Or do a two pass output that computes lengths first. But that's
fairly complicated and would prefer to not tackle this right now.

Again it's only a few cases where it is visible at all.

-Andi

Re: Support standalone metrics and metric groups for perf

[Jiri Olsa]

On Fri, Sep 01, 2017 at 10:42:04AM -0700, Andi Kleen wrote:
> > could you please check on that and maybe shift the alignment for the longest name?
> 
> To make everything align would require shifting everything. So most
> usages which don't have that wide events wouldn't fit into 80
> character columns anymore. That would be worse.
> 
> Or do a two pass output that computes lengths first. But that's
> fairly complicated and would prefer to not tackle this right now.

that's what I had in mind.. but could be dealt with later

jirka