Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1500566 > unrolled thread

Support Intel uncore event lists

Back to article view | Back to linux.kernel

Contents

  Support Intel uncore event lists Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    [PATCH 07/10] perf, tools: Collapse identically named events in perf stat Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:00 +0200
      Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:30 +0200
        Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Andi Kleen <andi@firstfloor.org> - 2016-10-17 18:40 +0200
          Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:30 +0200
            Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Andi Kleen <ak@linux.intel.com> - 2016-10-17 20:20 +0200
      Re: [PATCH 07/10] perf, tools: Collapse identically named events in  perf stat Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:30 +0200
    [PATCH 08/10] perf, tools: Expand PMU events by prefix match Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:40 +0200
      Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:50 +0200
        Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Andi Kleen <andi@firstfloor.org> - 2016-10-17 19:20 +0200
          Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:30 +0200
    [PATCH 02/10] perf, tools: Only print Using CPUID message once Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 02/10] perf, tools: Only print Using CPUID message once Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 17:50 +0200
      [tip:perf/core] perf pmu: Only print Using CPUID message once tip-bot for Andi Kleen <tipbot@zytor.com> - 2016-10-24 21:10 +0200
    [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:50 +0200
        Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Andi Kleen <andi@firstfloor.org> - 2016-10-17 18:30 +0200
          Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:50 +0200
          Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Jiri Olsa <jolsa@redhat.com> - 2016-10-17 19:50 +0200
            Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON  event list Andi Kleen <andi@firstfloor.org> - 2016-10-17 20:30 +0200
    [PATCH 04/10] perf, tools: Support per pmu json aliases Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 04/10] perf, tools: Support per pmu json aliases Jiri Olsa <jolsa@redhat.com> - 2016-10-14 14:40 +0200
    [PATCH 05/10] perf, tools: Support event aliases for non cpu// pmus Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 05/10] perf, tools: Support event aliases for non cpu//  pmus Jiri Olsa <jolsa@redhat.com> - 2016-10-17 11:40 +0200
      Re: [PATCH 05/10] perf, tools: Support event aliases for non cpu//  pmus Jiri Olsa <jolsa@redhat.com> - 2016-10-17 12:30 +0200
    [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
      Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 17:40 +0200
        Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <andi@firstfloor.org> - 2016-10-14 17:50 +0200
          Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 18:20 +0200
            Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <ak@linux.intel.com> - 2016-10-14 18:20 +0200
              Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 18:30 +0200
                Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-14 18:40 +0200
                Re: [PATCH 01/10] perf, tools: Factor out scale conversion code Andi Kleen <andi@firstfloor.org> - 2016-10-14 18:40 +0200
    [PATCH 10/10] perf, tools, stat: Output generic dividedby metric Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    [PATCH 06/10] perf, tools: Add debug support for outputing alias string Andi Kleen <andi@firstfloor.org> - 2016-10-13 23:20 +0200
    Re: Support Intel uncore event lists Jiri Olsa <jolsa@redhat.com> - 2016-10-17 13:10 +0200

Page 1 of 2  [1] 2  Next page →

#1500566 — Support Intel uncore event lists

This adds uncore support on top of the recently merged JSON event list
infrastructure for core events. Uncore is everything outside the core,
including memory controllers, PCI, interconnect etc.

Uncore is more complicated to handle than core events because it uses
many duplicated PMUs, which leads to long event lists and verbose duplicated
outputs. 

In fact previously it was nearly unusable for many cases without special 
tools to generate event list and aggregate data (such as 
https://github.com/andikleen/pmu-tools/tree/master/ucevent)

With this patchkit we add:
- Basic support for uncore events in JSON events
- Support aliases that get duplicated over many PMUs transparently
- Support summing up duplicated PMUs per socket
- Support extending the perf stat builtin metrics with simple ratios
specified in the event list. This covers the vast majority of useful
metrics.

So far mainly servers are supported. Also this is not using full event lists
(which are full of very obscure events) but only for a smaller subset of
curated useful and understandable metrics.

The actual event lists are not posted, but available at
git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/intel-uncore-json-files-1

The code is available here
git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/builtin-json-15

v1: Initial post

-Andi

[toc] | [next] | [standalone]

#1500567 — [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

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

The uncore PMU has a lot of duplicated PMUs for different subsystems.
When expanding an uncore alias we usually end up with a large
number of identically named aliases, which makes perf stat
output difficult to read.

Automatically sum them up in perf stat, unless --no-merge is specified.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-stat.txt |   3 +
 tools/perf/builtin-stat.c              | 125 +++++++++++++++++++++++++++------
 tools/perf/util/evsel.h                |   1 +
 3 files changed, 106 insertions(+), 23 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index d96ccd4844df..320d8020bc5b 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -237,6 +237,9 @@ To interpret the results it is usually needed to know on which
 CPUs the workload runs on. If needed the CPUs can be forced using
 taskset.
 
+--no-merge::
+Do not merge results from same PMUs.
+
 EXAMPLES
 --------
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 688dea7cb08f..76304f27c090 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -140,6 +140,7 @@ static unsigned int		unit_width			= 4; /* strlen("unit") */
 static bool			forever				= false;
 static bool			metric_only			= false;
 static bool			force_metric_only		= false;
+static bool			no_merge			= false;
 static struct timespec		ref_time;
 static struct cpu_map		*aggr_map;
 static aggr_get_id_t		aggr_get_id;
@@ -1178,11 +1179,59 @@ static void aggr_update_shadow(void)
 	}
 }
 
+static void collect_aliases(struct perf_evsel *counter,
+			    void (*cb)(struct perf_evsel *counter, void *data,
+				       bool first),
+			    void *data)
+{
+	struct perf_evsel *alias;
+
+	alias = list_prepare_entry(counter, &(evsel_list->entries), node);
+	cb(counter, data, true);
+	if (no_merge)
+		return;
+	list_for_each_entry_continue (alias, &evsel_list->entries, node) {
+		if (strcmp(perf_evsel__name(alias), perf_evsel__name(counter)) ||
+		    alias->scale != counter->scale ||
+		    alias->cgrp != counter->cgrp ||
+		    strcmp(alias->unit, counter->unit) ||
+		    nsec_counter(alias) != nsec_counter(counter))
+			break;
+		alias->alias = true;
+		cb(alias, data, false);
+	}
+}
+
+struct aggr_data {
+	u64 ena, run, val;
+	int id;
+	int nr;
+	int cpu;
+};
+
+static void aggr_cb(struct perf_evsel *counter, void *data, bool first)
+{
+	struct aggr_data *ad = data;
+	int cpu, cpu2, s2;
+
+	for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
+		cpu2 = perf_evsel__cpus(counter)->map[cpu];
+		s2 = aggr_get_id(evsel_list->cpus, cpu2);
+		if (s2 != ad->id)
+			continue;
+		ad->val += perf_counts(counter->counts, cpu, 0)->val;
+		ad->ena += perf_counts(counter->counts, cpu, 0)->ena;
+		ad->run += perf_counts(counter->counts, cpu, 0)->run;
+		if (first)
+			ad->nr++;
+	}
+}
+
 static void print_aggr(char *prefix)
 {
 	FILE *output = stat_config.output;
 	struct perf_evsel *counter;
-	int cpu, s, s2, id, nr;
+	int s, id, nr;
 	double uval;
 	u64 ena, run, val;
 	bool first;
@@ -1197,23 +1246,22 @@ static void print_aggr(char *prefix)
 	 * Without each counter has its own line.
 	 */
 	for (s = 0; s < aggr_map->nr; s++) {
+		struct aggr_data ad;
 		if (prefix && metric_only)
 			fprintf(output, "%s", prefix);
 
-		id = aggr_map->map[s];
+		ad.id = id = aggr_map->map[s];
 		first = true;
 		evlist__for_each_entry(evsel_list, counter) {
-			val = ena = run = 0;
-			nr = 0;
-			for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
-				s2 = aggr_get_id(perf_evsel__cpus(counter), cpu);
-				if (s2 != id)
-					continue;
-				val += perf_counts(counter->counts, cpu, 0)->val;
-				ena += perf_counts(counter->counts, cpu, 0)->ena;
-				run += perf_counts(counter->counts, cpu, 0)->run;
-				nr++;
-			}
+			if (counter->alias)
+				continue;
+			ad.val = ad.ena = ad.run = 0;
+			ad.nr = 0;
+			collect_aliases(counter, aggr_cb, &ad);
+			nr = ad.nr;
+			ena = ad.ena;
+			run = ad.run;
+			val = ad.val;
 			if (first && metric_only) {
 				first = false;
 				aggr_printout(counter, id, nr);
@@ -1257,6 +1305,21 @@ static void print_aggr_thread(struct perf_evsel *counter, char *prefix)
 	}
 }
 
+struct caggr_data {
+	double avg, avg_enabled, avg_running;
+};
+
+static void counter_aggr_cb(struct perf_evsel *counter, void *data,
+			    bool first __maybe_unused)
+{
+	struct caggr_data *cd = data;
+	struct perf_stat_evsel *ps = counter->priv;
+
+	cd->avg += avg_stats(&ps->res_stats[0]);
+	cd->avg_enabled += avg_stats(&ps->res_stats[1]);
+	cd->avg_running += avg_stats(&ps->res_stats[2]);
+}
+
 /*
  * Print out the results of a single counter:
  * aggregated counts in system-wide mode
@@ -1264,23 +1327,32 @@ static void print_aggr_thread(struct perf_evsel *counter, char *prefix)
 static void print_counter_aggr(struct perf_evsel *counter, char *prefix)
 {
 	FILE *output = stat_config.output;
-	struct perf_stat_evsel *ps = counter->priv;
-	double avg = avg_stats(&ps->res_stats[0]);
 	double uval;
-	double avg_enabled, avg_running;
+	struct caggr_data cd = { .avg = 0.0 };
 
-	avg_enabled = avg_stats(&ps->res_stats[1]);
-	avg_running = avg_stats(&ps->res_stats[2]);
+	if (counter->alias)
+		return;
+	collect_aliases(counter, counter_aggr_cb, &cd);
 
 	if (prefix && !metric_only)
 		fprintf(output, "%s", prefix);
 
-	uval = avg * counter->scale;
-	printout(-1, 0, counter, uval, prefix, avg_running, avg_enabled, avg);
+	uval = cd.avg * counter->scale;
+	printout(-1, 0, counter, uval, prefix, cd.avg_running, cd.avg_enabled, cd.avg);
 	if (!metric_only)
 		fprintf(output, "\n");
 }
 
+static void counter_cb(struct perf_evsel *counter, void *data,
+		       bool first __maybe_unused)
+{
+	struct aggr_data *ad = data;
+
+	ad->val += perf_counts(counter->counts, ad->cpu, 0)->val;
+	ad->ena += perf_counts(counter->counts, ad->cpu, 0)->ena;
+	ad->run += perf_counts(counter->counts, ad->cpu, 0)->run;
+}
+
 /*
  * Print out the results of a single counter:
  * does not use aggregated count in system-wide
@@ -1292,10 +1364,16 @@ static void print_counter(struct perf_evsel *counter, char *prefix)
 	double uval;
 	int cpu;
 
+	if (counter->alias)
+		return;
+
 	for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
-		val = perf_counts(counter->counts, cpu, 0)->val;
-		ena = perf_counts(counter->counts, cpu, 0)->ena;
-		run = perf_counts(counter->counts, cpu, 0)->run;
+		struct aggr_data ad = { .cpu = cpu };
+
+		collect_aliases(counter, counter_cb, &ad);
+		val = ad.val;
+		ena = ad.ena;
+		run = ad.run;
 
 		if (prefix)
 			fprintf(output, "%s", prefix);
@@ -1633,6 +1711,7 @@ static const struct option stat_options[] = {
 		    "list of cpus to monitor in system-wide"),
 	OPT_SET_UINT('A', "no-aggr", &stat_config.aggr_mode,
 		    "disable CPU count aggregation", AGGR_NONE),
+	OPT_BOOLEAN(0, "no-merge", &no_merge, "Do not merge identical named events"),
 	OPT_STRING('x', "field-separator", &csv_sep, "separator",
 		   "print counts with custom separator"),
 	OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index b1503b0ecdff..4e3158fe79c2 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -128,6 +128,7 @@ struct perf_evsel {
 	bool			cmdline_group_boundary;
 	struct list_head	config_terms;
 	int			bpf_fd;
+	bool			alias;
 };
 
 union u64_swap {
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1501869 — Re: [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

On Thu, Oct 13, 2016 at 02:15:29PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> The uncore PMU has a lot of duplicated PMUs for different subsystems.
> When expanding an uncore alias we usually end up with a large
> number of identically named aliases, which makes perf stat
> output difficult to read.

please provide output examples

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1501892 — Re: [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

On Thu, Oct 13, 2016 at 02:15:29PM -0700, Andi Kleen wrote:

SNIP

>  --------
>  
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 688dea7cb08f..76304f27c090 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -140,6 +140,7 @@ static unsigned int		unit_width			= 4; /* strlen("unit") */
>  static bool			forever				= false;
>  static bool			metric_only			= false;
>  static bool			force_metric_only		= false;
> +static bool			no_merge			= false;
>  static struct timespec		ref_time;
>  static struct cpu_map		*aggr_map;
>  static aggr_get_id_t		aggr_get_id;
> @@ -1178,11 +1179,59 @@ static void aggr_update_shadow(void)
>  	}
>  }
>  
> +static void collect_aliases(struct perf_evsel *counter,
> +			    void (*cb)(struct perf_evsel *counter, void *data,
> +				       bool first),
> +			    void *data)

merges_stats might be better name

> +{
> +	struct perf_evsel *alias;
> +
> +	alias = list_prepare_entry(counter, &(evsel_list->entries), node);
> +	cb(counter, data, true);
> +	if (no_merge)
> +		return;

please put this decision (no_merge) outside this function,
so the normal path is straight

this leads to my next question: why this merging should be default?

it seems to make sense just for uncore events

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1502188 — Re: [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

> this leads to my next question: why this merging should be default?

It's the right default for uncore, and it doesn't do anything for
non uncore because these usually don't have duplicated event aliases
over different PMUs.

-Andi

[toc] | [prev] | [next] | [standalone]

#1502298 — Re: [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

On Mon, Oct 17, 2016 at 09:30:17AM -0700, Andi Kleen wrote:
> > this leads to my next question: why this merging should be default?
> 
> It's the right default for uncore, and it doesn't do anything for
> non uncore because these usually don't have duplicated event aliases
> over different PMUs.

I don't like the part when we depends on 'usually'

jirka

[toc] | [prev] | [next] | [standalone]

#1502326 — Re: [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

On Mon, Oct 17, 2016 at 07:28:36PM +0200, Jiri Olsa wrote:
> On Mon, Oct 17, 2016 at 09:30:17AM -0700, Andi Kleen wrote:
> > > this leads to my next question: why this merging should be default?
> > 
> > It's the right default for uncore, and it doesn't do anything for
> > non uncore because these usually don't have duplicated event aliases
> > over different PMUs.
> 
> I don't like the part when we depends on 'usually'

I'm not aware of any counter example in today's perf.

-Andi

[toc] | [prev] | [next] | [standalone]

#1501894 — Re: [PATCH 07/10] perf, tools: Collapse identically named events in perf stat

On Thu, Oct 13, 2016 at 02:15:29PM -0700, Andi Kleen wrote:

SNIP

> +	OPT_BOOLEAN(0, "no-merge", &no_merge, "Do not merge identical named events"),
>  	OPT_STRING('x', "field-separator", &csv_sep, "separator",
>  		   "print counts with custom separator"),
>  	OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index b1503b0ecdff..4e3158fe79c2 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -128,6 +128,7 @@ struct perf_evsel {
>  	bool			cmdline_group_boundary;
>  	struct list_head	config_terms;
>  	int			bpf_fd;
> +	bool			alias;

I think we should call this some other name,
we already use alias for something else..
also alias->alias looks bad ;-)

how about 'merged_stats'

jirka

[toc] | [prev] | [next] | [standalone]

#1500568 — [PATCH 08/10] perf, tools: Expand PMU events by prefix match

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

When the user specifies a pmu directly, expand it automatically
with a prefix match, similar as we do for the normal aliases now.

This allows to specify attributes for duplicated boxes quickly.
For example uncore_cbox_{0,8}/.../ can be now specified as cbox/.../
and it gets automatically expanded.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/parse-events.c | 25 +++++++++++++++++++++++++
 tools/perf/util/parse-events.h |  3 +++
 tools/perf/util/parse-events.y | 27 +++++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a2bbd17a0dc3..8b2333278988 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2399,6 +2399,31 @@ int parse_events_term__clone(struct parse_events_term **new,
 			term->err_term, term->err_val);
 }
 
+int parse_events_copy_term_list(struct list_head *old,
+				 struct list_head **new)
+{
+	struct parse_events_term *term, *n;
+	int ret;
+
+	if (!old) {
+		*new = NULL;
+		return 0;
+	}
+
+	*new = malloc(sizeof(struct list_head));
+	if (!*new)
+		return -ENOMEM;
+	INIT_LIST_HEAD(*new);
+
+	list_for_each_entry (term, old, list) {
+		ret = parse_events_term__clone(&n, term);
+		if (ret)
+			return ret;
+		list_add_tail(&n->list, *new);
+	}
+	return 0;
+}
+
 void parse_events_terms__purge(struct list_head *terms)
 {
 	struct parse_events_term *term, *h;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index da246a3ddb69..7ea95c35095c 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -164,6 +164,9 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 int parse_events_add_pmu(struct parse_events_evlist *data,
 			 struct list_head *list, char *name,
 			 struct list_head *head_config);
+int parse_events_copy_term_list(struct list_head *old,
+				 struct list_head **new);
+
 enum perf_pmu_event_symbol_type
 perf_pmu__parse_check(const char *name);
 void parse_events__set_leader(char *name, struct list_head *list);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 3a5196380609..790f0dd598b9 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -224,11 +224,34 @@ event_pmu:
 PE_NAME opt_event_config
 {
 	struct parse_events_evlist *data = _data;
-	struct list_head *list;
+	struct list_head *list, *orig_terms, *terms;
+
+	if (parse_events_copy_term_list($2, &orig_terms))
+		YYABORT;
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(data, list, $1, $2));
+	if (parse_events_add_pmu(data, list, $1, $2)) {
+		struct perf_pmu *pmu = NULL;
+		int ok = 0;
+
+		while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+			char *name = pmu->name;
+
+			if (!strncmp(name, "uncore_", 7))
+				name += 7;
+			if (!strncmp($1, name, strlen($1))) {
+				if (parse_events_copy_term_list(orig_terms, &terms))
+					YYABORT;
+				if (!parse_events_add_pmu(data, list, pmu->name, terms))
+					ok++;
+				parse_events_terms__delete(terms);
+			}
+		}
+		if (!ok)
+			YYABORT;
+	}
 	parse_events_terms__delete($2);
+	parse_events_terms__delete(orig_terms);
 	$$ = list;
 }
 |
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1501897 — Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match

On Thu, Oct 13, 2016 at 02:15:30PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> When the user specifies a pmu directly, expand it automatically
> with a prefix match, similar as we do for the normal aliases now.
> 
> This allows to specify attributes for duplicated boxes quickly.
> For example uncore_cbox_{0,8}/.../ can be now specified as cbox/.../
> and it gets automatically expanded.

so expand here means adding all the events, right?

could you please state and example and make clear what happens as outcome

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1501902 — Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match

On Thu, Oct 13, 2016 at 02:15:30PM -0700, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 3a5196380609..790f0dd598b9 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -224,11 +224,34 @@ event_pmu:
>  PE_NAME opt_event_config
>  {
>  	struct parse_events_evlist *data = _data;
> -	struct list_head *list;
> +	struct list_head *list, *orig_terms, *terms;
> +
> +	if (parse_events_copy_term_list($2, &orig_terms))
> +		YYABORT;
>  
>  	ALLOC_LIST(list);
> -	ABORT_ON(parse_events_add_pmu(data, list, $1, $2));
> +	if (parse_events_add_pmu(data, list, $1, $2)) {
> +		struct perf_pmu *pmu = NULL;
> +		int ok = 0;
> +
> +		while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> +			char *name = pmu->name;
> +
> +			if (!strncmp(name, "uncore_", 7))
> +				name += 7;

so there's a special treatment for uncore events,
what if user says 'uncore_box/..' then?


> +			if (!strncmp($1, name, strlen($1))) {
> +				if (parse_events_copy_term_list(orig_terms, &terms))
> +					YYABORT;
> +				if (!parse_events_add_pmu(data, list, pmu->name, terms))
> +					ok++;

so we're ok if some of the events is not added?
do we warn at least?

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1502280 — Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match

> so there's a special treatment for uncore events,
> what if user says 'uncore_box/..' then?

It should work. There's nothing special for uncore later, this
is just for convenience so that I have less to type.

> > +			if (!strncmp($1, name, strlen($1))) {
> > +				if (parse_events_copy_term_list(orig_terms, &terms))
> > +					YYABORT;
> > +				if (!parse_events_add_pmu(data, list, pmu->name, terms))
> > +					ok++;
> 
> so we're ok if some of the events is not added?
> do we warn at least?

It would warn a lot because most PMUs don't have a given aliases.
So you would get an warning for every extra PMU.

Trying to warn only for those that have the alias would need a lot
of extra tracking, and it didn't seem worth the complexity.

-Andi

[toc] | [prev] | [next] | [standalone]

#1502290 — Re: [PATCH 08/10] perf, tools: Expand PMU events by prefix match

On Mon, Oct 17, 2016 at 09:56:42AM -0700, Andi Kleen wrote:
> > so there's a special treatment for uncore events,
> > what if user says 'uncore_box/..' then?
> 
> It should work. There's nothing special for uncore later, this
> is just for convenience so that I have less to type.

really..


'uncore_cbox_0/clockticks/'

	[jolsa@krava perf]$ sudo ./perf stat -e 'uncore_cbox_0/clockticks/' -a
	^C
	 Performance counter stats for 'system wide':

			 0      uncore_cbox_0/clockticks/                                   

	       0.676237018 seconds time elapsed


'cbox_0/clockticks/'

	[jolsa@krava perf]$ sudo ./perf stat -e 'cbox_0/clockticks/' -a
	^C
	 Performance counter stats for 'system wide':

			 0      cbox_0/clockticks/                                          

	       0.991623038 seconds time elapsed


'cbox/clockticks'

	[jolsa@krava perf]$ sudo ./perf stat -e 'cbox/clockticks/' -a
	^C
	 Performance counter stats for 'system wide':

			 0      cbox/clockticks/                                            

	       0.708006656 seconds time elapsed


'uncore_cbox/clockticks/'

	[jolsa@krava perf]$ sudo ./perf stat -e 'uncore_cbox/clockticks/' -a
	invalid or unsupported event: 'uncore_cbox/clockticks/'
	Run 'perf list' for a list of valid events

	 Usage: perf stat [<options>] [<command>]

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


jirka

[toc] | [prev] | [next] | [standalone]

#1500570 — [PATCH 02/10] perf, tools: Only print Using CPUID message once

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

With uncore event aliases which are duplicated over multiple PMUs
the "Using CPUID" message with -v could be printed many times.
Only print it once.

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

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9adae7e7477c..b36bf9e77799 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -508,6 +508,7 @@ static void pmu_add_cpu_aliases(struct list_head *head)
 	struct pmu_events_map *map;
 	struct pmu_event *pe;
 	char *cpuid;
+	static bool printed;
 
 	cpuid = getenv("PERF_CPUID");
 	if (cpuid)
@@ -517,7 +518,10 @@ static void pmu_add_cpu_aliases(struct list_head *head)
 	if (!cpuid)
 		return;
 
-	pr_debug("Using CPUID %s\n", cpuid);
+	if (!printed) {
+		pr_debug("Using CPUID %s\n", cpuid);
+		printed = true;
+	}
 
 	i = 0;
 	while (1) {
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1501066 — Re: [PATCH 02/10] perf, tools: Only print Using CPUID message once

Em Thu, Oct 13, 2016 at 02:15:24PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> With uncore event aliases which are duplicated over multiple PMUs
> the "Using CPUID" message with -v could be printed many times.
> Only print it once.

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/pmu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 9adae7e7477c..b36bf9e77799 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -508,6 +508,7 @@ static void pmu_add_cpu_aliases(struct list_head *head)
>  	struct pmu_events_map *map;
>  	struct pmu_event *pe;
>  	char *cpuid;
> +	static bool printed;
>  
>  	cpuid = getenv("PERF_CPUID");
>  	if (cpuid)
> @@ -517,7 +518,10 @@ static void pmu_add_cpu_aliases(struct list_head *head)
>  	if (!cpuid)
>  		return;
>  
> -	pr_debug("Using CPUID %s\n", cpuid);
> +	if (!printed) {
> +		pr_debug("Using CPUID %s\n", cpuid);
> +		printed = true;
> +	}
>  
>  	i = 0;
>  	while (1) {
> -- 
> 2.5.5

[toc] | [prev] | [next] | [standalone]

#1507611 — [tip:perf/core] perf pmu: Only print Using CPUID message once

Commit-ID:  fb967063699e25ae73f0991672f99bd7104f70c8
Gitweb:     http://git.kernel.org/tip/fb967063699e25ae73f0991672f99bd7104f70c8
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Thu, 13 Oct 2016 14:15:24 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 24 Oct 2016 11:07:41 -0300

perf pmu: Only print Using CPUID message once

With uncore event aliases which are duplicated over multiple PMUs the
"Using CPUID" message with -v could be printed many times.  Only print
it once.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1476393332-20732-3-git-send-email-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index b1474dc..d7174f3 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -504,6 +504,7 @@ static void pmu_add_cpu_aliases(struct list_head *head)
 	struct pmu_events_map *map;
 	struct pmu_event *pe;
 	char *cpuid;
+	static bool printed;
 
 	cpuid = getenv("PERF_CPUID");
 	if (cpuid)
@@ -513,7 +514,10 @@ static void pmu_add_cpu_aliases(struct list_head *head)
 	if (!cpuid)
 		return;
 
-	pr_debug("Using CPUID %s\n", cpuid);
+	if (!printed) {
+		pr_debug("Using CPUID %s\n", cpuid);
+		printed = true;
+	}
 
 	i = 0;
 	while (1) {

[toc] | [prev] | [next] | [standalone]

#1500571 — [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

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

Add support for parsing the DividedBy header in the JSON event lists and
storing them in the alias structure.

Used in the next patch.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/pmu-events/jevents.c    | 18 ++++++++++++++----
 tools/perf/pmu-events/jevents.h    |  2 +-
 tools/perf/pmu-events/pmu-events.h |  1 +
 tools/perf/util/pmu.c              |  9 ++++++---
 tools/perf/util/pmu.h              |  1 +
 5 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 23517db584e6..a99106060658 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -294,7 +294,8 @@ static void print_events_table_prefix(FILE *fp, const char *tblname)
 
 static int print_events_table_entry(void *data, char *name, char *event,
 				    char *desc, char *long_desc,
-				    char *pmu, char *unit, char *perpkg)
+				    char *pmu, char *unit, char *perpkg,
+				    char *dividedby)
 {
 	struct perf_entry_data *pd = data;
 	FILE *outfp = pd->outfp;
@@ -318,6 +319,8 @@ static int print_events_table_entry(void *data, char *name, char *event,
 		fprintf(outfp, "\t.unit = \"%s\",\n", unit);
 	if (perpkg)
 		fprintf(outfp, "\t.perpkg = \"%s\",\n", perpkg);
+	if (dividedby)
+		fprintf(outfp, "\t.dividedby = \"%s\",\n", dividedby);
 	fprintf(outfp, "},\n");
 
 	return 0;
@@ -365,7 +368,8 @@ static char *real_event(const char *name, char *event)
 int json_events(const char *fn,
 	  int (*func)(void *data, char *name, char *event, char *desc,
 		      char *long_desc,
-		      char *pmu, char *unit, char *perpkg),
+		      char *pmu, char *unit, char *perpkg,
+		      char *dividedby),
 	  void *data)
 {
 	int err = -EIO;
@@ -391,6 +395,7 @@ int json_events(const char *fn,
 		char *filter = NULL;
 		char *perpkg = NULL;
 		char *unit = NULL;
+		char *dividedby = NULL;
 		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
@@ -401,6 +406,7 @@ int json_events(const char *fn,
 		for (j = 0; j < obj->size; j += 2) {
 			jsmntok_t *field, *val;
 			int nz;
+			char *s;
 
 			field = tok + j;
 			EXPECT(field->type == JSMN_STRING, tok + j,
@@ -447,7 +453,6 @@ int json_events(const char *fn,
 					NULL);
 			} else if (json_streq(map, field, "Unit")) {
 				const char *ppmu;
-				char *s;
 
 				ppmu = field_to_perf(unit_to_pmu, map, val);
 				if (ppmu) {
@@ -465,6 +470,10 @@ int json_events(const char *fn,
 				addfield(map, &unit, "", "", val);
 			} else if (json_streq(map, field, "PerPkg")) {
 				addfield(map, &perpkg, "", "", val);
+			} else if (json_streq(map, field, "DividedBy")) {
+				addfield(map, &dividedby, "", "", val);
+				for (s = dividedby; *s; s++)
+					*s = tolower(*s);
 			}
 			/* ignore unknown fields */
 		}
@@ -489,7 +498,7 @@ int json_events(const char *fn,
 		fixname(name);
 
 		err = func(data, name, real_event(name, event), desc, long_desc,
-				pmu, unit, perpkg);
+				pmu, unit, perpkg, dividedby);
 		free(event);
 		free(desc);
 		free(name);
@@ -499,6 +508,7 @@ int json_events(const char *fn,
 		free(filter);
 		free(perpkg);
 		free(unit);
+		free(dividedby);
 		if (err)
 			break;
 		tok += j;
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index 71e13de31092..9488369a9467 100644
--- a/tools/perf/pmu-events/jevents.h
+++ b/tools/perf/pmu-events/jevents.h
@@ -5,7 +5,7 @@ int json_events(const char *fn,
 		int (*func)(void *data, char *name, char *event, char *desc,
 				char *long_desc,
 				char *pmu,
-				char *unit, char *perpkg),
+				char *unit, char *perpkg, char *dividedby),
 		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 c669a3cdb9f0..90603afddb77 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -13,6 +13,7 @@ struct pmu_event {
 	const char *pmu;
 	const char *unit;
 	const char *perpkg;
+	const char *dividedby;
 };
 
 /*
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index dc93c7d4a799..d298e7413a80 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -229,7 +229,8 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias *alias,
 static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 				 char *desc __maybe_unused, char *val,
 				 char *long_desc, char *topic,
-				 char *unit, char *perpkg)
+				 char *unit, char *perpkg,
+				 char *dividedby)
 {
 	struct perf_pmu_alias *alias;
 	int ret;
@@ -263,6 +264,7 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 		perf_pmu__parse_snapshot(alias, dir, name);
 	}
 
+	alias->dividedby = dividedby ? strdup(dividedby) : NULL;
 	alias->desc = desc ? strdup(desc) : NULL;
 	alias->long_desc = long_desc ? strdup(long_desc) :
 				desc ? strdup(desc) : NULL;
@@ -291,7 +293,7 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
 	buf[ret] = 0;
 
 	return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL, NULL,
-				     NULL);
+				     NULL, NULL);
 }
 
 static inline bool pmu_alias_info_file(char *name)
@@ -558,7 +560,8 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 		__perf_pmu__new_alias(head, NULL, (char *)pe->name,
 				(char *)pe->desc, (char *)pe->event,
 				(char *)pe->long_desc, (char *)pe->topic,
-				(char *)pe->unit, (char *)pe->perpkg);
+				(char *)pe->unit, (char *)pe->perpkg,
+				(char *)pe->dividedby);
 	}
 
 out:
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 00852ddc7741..faf8a7f97d03 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -50,6 +50,7 @@ struct perf_pmu_alias {
 	double scale;
 	bool per_pkg;
 	bool snapshot;
+	char *dividedby;
 };
 
 struct perf_pmu *perf_pmu__find(const char *name);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1501907 — Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add support for parsing the DividedBy header in the JSON event lists and
> storing them in the alias structure.

I wish you'd add JSON tags always one by one as you did in here ;-)

however Ithink we'll need more info here:
  - what's the value?
  - what's it going to be used for?
  - looks like formula stuff, why post processing via python/perl can't be used in this case?

jirka

[toc] | [prev] | [next] | [standalone]

#1502174 — Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

On Mon, Oct 17, 2016 at 01:44:43PM +0200, Jiri Olsa wrote:
> On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Add support for parsing the DividedBy header in the JSON event lists and
> > storing them in the alias structure.
> 
> I wish you'd add JSON tags always one by one as you did in here ;-)
> 
> however Ithink we'll need more info here:
>   - what's the value?
>   - what's it going to be used for?

That's all described in the next patch. But I can copy the description.

>   - looks like formula stuff, why post processing via python/perl can't be used in this case?

It would be fairly complicated to interface that with event lists, and
also still wouldn't work with standard perf stat. 

DividedBy already covers the majority of interesting cases and fits
nicely with the existing frame work. If we wanted more complex
formulas something with python would be probably needed, but I don't see
the need yet.

-Andi

[toc] | [prev] | [next] | [standalone]

#1502313 — Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

On Mon, Oct 17, 2016 at 07:43:12PM +0200, Jiri Olsa wrote:
> On Mon, Oct 17, 2016 at 09:27:54AM -0700, Andi Kleen wrote:
> > On Mon, Oct 17, 2016 at 01:44:43PM +0200, Jiri Olsa wrote:
> > > On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> > > > From: Andi Kleen <ak@linux.intel.com>
> > > > 
> > > > Add support for parsing the DividedBy header in the JSON event lists and
> > > > storing them in the alias structure.
> > > 
> > > I wish you'd add JSON tags always one by one as you did in here ;-)
> > > 
> > > however Ithink we'll need more info here:
> > >   - what's the value?
> > >   - what's it going to be used for?
> > 
> > That's all described in the next patch. But I can copy the description.
> > 
> > >   - looks like formula stuff, why post processing via python/perl can't be used in this case?
> > 
> > It would be fairly complicated to interface that with event lists, and
> > also still wouldn't work with standard perf stat. 
> > 
> > DividedBy already covers the majority of interesting cases and fits
> > nicely with the existing frame work. If we wanted more complex
> > formulas something with python would be probably needed, but I don't see
> > the need yet.
> 
> so..
> 
> - you put 'DividedBy' into JSON event's defition any further
                                                  ^ without ;-)

>   explanation how or why the format we use for event defs will
>   be used now used to describe ratios
> 
> - then you force perf stat to merge together all 'same' uncore events
>   to get just one number..
> 
> - then you display that ratio (just the number) in perf stat metrics output
>   without any explanation or description
> 
> I dont see that as a nicely fit, more like hack
> 
> please let's go first to discuss the DividedBy being included
> in JSON defs, which is fragile topic to begin with
> 
> thanks,
> jirka

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web