All of lore.kernel.org
 help / color / mirror / Atom feed
* Support Intel uncore event lists
@ 2017-01-28  2:03 Andi Kleen
  2017-01-28  2:03 ` [PATCH 01/10] perf, tools: Parse eventcode as number in jevents Andi Kleen
                   ` (9 more replies)
  0 siblings, 10 replies; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel

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 expressions
specified in the event list.

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

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

v1: Initial post
v2: Address review feedback. See changelog in commits.
v3: Repost. Rebase to latest tree.
v4: Rebase. Change DividedBy to generic simple expression parser.
Fix refactoring problem that broke git bisect.
v5: Address review feedback. See changelog in commits. Add acks.

-Andi

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 01/10] perf, tools: Parse eventcode as number in jevents
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-10  7:41   ` [tip:perf/core] perf jevents: Parse eventcode as number tip-bot for Andi Kleen
  2017-01-28  2:03 ` [PATCH 02/10] perf, tools: Add support for parsing uncore json files Andi Kleen
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

The next patch needs to modify event code. Previously eventcode was just
passed through as a string. Now parse it as a number.

v2: Don't special case 0
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/pmu-events/jevents.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 41611d7f9873..551377bf8428 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -135,7 +135,6 @@ static struct field {
 	const char *field;
 	const char *kernel;
 } fields[] = {
-	{ "EventCode",	"event=" },
 	{ "UMask",	"umask=" },
 	{ "CounterMask", "cmask=" },
 	{ "Invert",	"inv=" },
@@ -343,6 +342,7 @@ int json_events(const char *fn,
 	jsmntok_t *tokens, *tok;
 	int i, j, len;
 	char *map;
+	char buf[128];
 
 	if (!fn)
 		return -ENOENT;
@@ -356,6 +356,7 @@ int json_events(const char *fn,
 		char *event = NULL, *desc = NULL, *name = NULL;
 		char *long_desc = NULL;
 		char *extra_desc = NULL;
+		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
 		jsmntok_t *precise = NULL;
@@ -376,6 +377,11 @@ int json_events(const char *fn,
 			nz = !json_streq(map, val, "0");
 			if (match_field(map, field, nz, &event, val)) {
 				/* ok */
+			} else if (json_streq(map, field, "EventCode")) {
+				char *code = NULL;
+				addfield(map, &code, "", "", val);
+				eventcode |= strtoul(code, NULL, 0);
+				free(code);
 			} else if (json_streq(map, field, "EventName")) {
 				addfield(map, &name, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
@@ -410,6 +416,8 @@ int json_events(const char *fn,
 				addfield(map, &extra_desc, " ",
 						"(Precise event)", NULL);
 		}
+		snprintf(buf, sizeof buf, "event=%#llx", eventcode);
+		addfield(map, &event, ",", buf, NULL);
 		if (desc && extra_desc)
 			addfield(map, &desc, " ", extra_desc, NULL);
 		if (long_desc && extra_desc)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 02/10] perf, tools: Add support for parsing uncore json files
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
  2017-01-28  2:03 ` [PATCH 01/10] perf, tools: Parse eventcode as number in jevents Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-10  7:41   ` [tip:perf/core] perf jevents: " tip-bot for Andi Kleen
  2017-01-28  2:03 ` [PATCH 03/10] perf, tools: Support per pmu json aliases Andi Kleen
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

Handle the Unit field, which is needed to find the right PMU for
an event. We call it "pmu" and convert it to the perf pmu name
with an uncore prefix.

Handle the ExtSel field, which just extends the event mask with
an additional bit.

Handle the Filter field which adds parameters to the main event
to configure filtering.

Handle the Unit field which declares the unit the values
should be scaled too (similar to what the kernel exports)

Set up the perpkg field for uncore events so that perf
knows they are per package (similar to what the kernel exports)

Then output the fields into the pmu-events data structures which
are compiled into perf.

Filter out zero fields, except for the event itself.

v2: Fix compilation. Add uncore_ prefix at pre-processing time.
Move eventcode change to separate patch.
v3: Remove extra __maybe_unused
v4: dont duplicate aliases for cpu pmu events
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/pmu-events/jevents.c    | 74 +++++++++++++++++++++++++++++++++++---
 tools/perf/pmu-events/jevents.h    |  4 ++-
 tools/perf/pmu-events/pmu-events.h |  3 ++
 tools/perf/util/pmu.c              | 30 +++++++++++-----
 4 files changed, 98 insertions(+), 13 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 551377bf8428..eed09346a72a 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -188,6 +188,27 @@ static struct msrmap *lookup_msr(char *map, jsmntok_t *val)
 	return NULL;
 }
 
+static struct map {
+	const char *json;
+	const char *perf;
+} unit_to_pmu[] = {
+	{ "CBO", "uncore_cbox" },
+	{ "QPI LL", "uncore_qpi" },
+	{ "SBO", "uncore_sbox" },
+	{}
+};
+
+static const char *field_to_perf(struct map *table, char *map, jsmntok_t *val)
+{
+	int i;
+
+	for (i = 0; table[i].json; i++) {
+		if (json_streq(map, val, table[i].json))
+			return table[i].perf;
+	}
+	return NULL;
+}
+
 #define EXPECT(e, t, m) do { if (!(e)) {			\
 	jsmntok_t *loc = (t);					\
 	if (!(t)->start && (t) > tokens)			\
@@ -269,7 +290,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 *desc, char *long_desc,
+				    char *pmu, char *unit, char *perpkg)
 {
 	struct perf_entry_data *pd = data;
 	FILE *outfp = pd->outfp;
@@ -287,7 +309,12 @@ static int print_events_table_entry(void *data, char *name, char *event,
 	fprintf(outfp, "\t.topic = \"%s\",\n", topic);
 	if (long_desc && long_desc[0])
 		fprintf(outfp, "\t.long_desc = \"%s\",\n", long_desc);
-
+	if (pmu)
+		fprintf(outfp, "\t.pmu = \"%s\",\n", pmu);
+	if (unit)
+		fprintf(outfp, "\t.unit = \"%s\",\n", unit);
+	if (perpkg)
+		fprintf(outfp, "\t.perpkg = \"%s\",\n", perpkg);
 	fprintf(outfp, "},\n");
 
 	return 0;
@@ -334,7 +361,8 @@ static char *real_event(const char *name, char *event)
 /* Call func with each event in the json file */
 int json_events(const char *fn,
 	  int (*func)(void *data, char *name, char *event, char *desc,
-		      char *long_desc),
+		      char *long_desc,
+		      char *pmu, char *unit, char *perpkg),
 	  void *data)
 {
 	int err = -EIO;
@@ -356,6 +384,10 @@ int json_events(const char *fn,
 		char *event = NULL, *desc = NULL, *name = NULL;
 		char *long_desc = NULL;
 		char *extra_desc = NULL;
+		char *pmu = NULL;
+		char *filter = NULL;
+		char *perpkg = NULL;
+		char *unit = NULL;
 		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
@@ -382,6 +414,11 @@ int json_events(const char *fn,
 				addfield(map, &code, "", "", val);
 				eventcode |= strtoul(code, NULL, 0);
 				free(code);
+			} else if (json_streq(map, field, "ExtSel")) {
+				char *code = NULL;
+				addfield(map, &code, "", "", val);
+				eventcode |= strtoul(code, NULL, 0) << 21;
+				free(code);
 			} else if (json_streq(map, field, "EventName")) {
 				addfield(map, &name, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
@@ -405,6 +442,28 @@ int json_events(const char *fn,
 				addfield(map, &extra_desc, ". ",
 					" Supports address when precise",
 					NULL);
+			} else if (json_streq(map, field, "Unit")) {
+				const char *ppmu;
+				char *s;
+
+				ppmu = field_to_perf(unit_to_pmu, map, val);
+				if (ppmu) {
+					pmu = strdup(ppmu);
+				} else {
+					if (!pmu)
+						pmu = strdup("uncore_");
+					addfield(map, &pmu, "", "", val);
+					for (s = pmu; *s; s++)
+						*s = tolower(*s);
+				}
+				addfield(map, &desc, ". ", "Unit: ", NULL);
+				addfield(map, &desc, "", pmu, NULL);
+			} else if (json_streq(map, field, "Filter")) {
+				addfield(map, &filter, "", "", val);
+			} else if (json_streq(map, field, "ScaleUnit")) {
+				addfield(map, &unit, "", "", val);
+			} else if (json_streq(map, field, "PerPkg")) {
+				addfield(map, &perpkg, "", "", val);
 			}
 			/* ignore unknown fields */
 		}
@@ -422,16 +481,23 @@ int json_events(const char *fn,
 			addfield(map, &desc, " ", extra_desc, NULL);
 		if (long_desc && extra_desc)
 			addfield(map, &long_desc, " ", extra_desc, NULL);
+		if (filter)
+			addfield(map, &event, ",", filter, NULL);
 		if (msr != NULL)
 			addfield(map, &event, ",", msr->pname, msrval);
 		fixname(name);
 
-		err = func(data, name, real_event(name, event), desc, long_desc);
+		err = func(data, name, real_event(name, event), desc, long_desc,
+				pmu, unit, perpkg);
 		free(event);
 		free(desc);
 		free(name);
 		free(long_desc);
 		free(extra_desc);
+		free(pmu);
+		free(filter);
+		free(perpkg);
+		free(unit);
 		if (err)
 			break;
 		tok += j;
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index b0eb2744b498..71e13de31092 100644
--- a/tools/perf/pmu-events/jevents.h
+++ b/tools/perf/pmu-events/jevents.h
@@ -3,7 +3,9 @@
 
 int json_events(const char *fn,
 		int (*func)(void *data, char *name, char *event, char *desc,
-				char *long_desc),
+				char *long_desc,
+				char *pmu,
+				char *unit, char *perpkg),
 		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 2eaef595d8a0..c669a3cdb9f0 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -10,6 +10,9 @@ struct pmu_event {
 	const char *desc;
 	const char *topic;
 	const char *long_desc;
+	const char *pmu;
+	const char *unit;
+	const char *perpkg;
 };
 
 /*
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 78b16100567d..6dc3cc050105 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -229,11 +229,13 @@ 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, char *val, char *long_desc,
-				 char *topic)
+				 char *desc, char *val,
+				 char *long_desc, char *topic,
+				 char *unit, char *perpkg)
 {
 	struct perf_pmu_alias *alias;
 	int ret;
+	int num;
 
 	alias = malloc(sizeof(*alias));
 	if (!alias)
@@ -267,7 +269,12 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 	alias->long_desc = long_desc ? strdup(long_desc) :
 				desc ? strdup(desc) : NULL;
 	alias->topic = topic ? strdup(topic) : NULL;
-
+	if (unit) {
+		if (convert_scale(unit, &unit, &alias->scale) < 0)
+			return -1;
+		snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
+	}
+	alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
 	list_add_tail(&alias->list, list);
 
 	return 0;
@@ -284,7 +291,8 @@ 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);
+	return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL, NULL,
+				     NULL);
 }
 
 static inline bool pmu_alias_info_file(char *name)
@@ -504,7 +512,7 @@ char * __weak get_cpuid_str(void)
  * 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)
+static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 {
 	int i;
 	struct pmu_events_map *map;
@@ -540,14 +548,21 @@ static void pmu_add_cpu_aliases(struct list_head *head)
 	 */
 	i = 0;
 	while (1) {
+		const char *pname;
+
 		pe = &map->table[i++];
 		if (!pe->name)
 			break;
 
+		pname = pe->pmu ? pe->pmu : "cpu";
+		if (strncmp(pname, name, strlen(pname)))
+			continue;
+
 		/* need type casts to override 'const' */
 		__perf_pmu__new_alias(head, NULL, (char *)pe->name,
 				(char *)pe->desc, (char *)pe->event,
-				(char *)pe->long_desc, (char *)pe->topic);
+				(char *)pe->long_desc, (char *)pe->topic,
+				(char *)pe->unit, (char *)pe->perpkg);
 	}
 
 out:
@@ -578,8 +593,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_aliases(name, &aliases))
 		return NULL;
 
-	if (!strcmp(name, "cpu"))
-		pmu_add_cpu_aliases(&aliases);
+	pmu_add_cpu_aliases(&aliases, name);
 
 	if (pmu_type(name, &type))
 		return NULL;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 03/10] perf, tools: Support per pmu json aliases
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
  2017-01-28  2:03 ` [PATCH 01/10] perf, tools: Parse eventcode as number in jevents Andi Kleen
  2017-01-28  2:03 ` [PATCH 02/10] perf, tools: Add support for parsing uncore json files Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-10  7:42   ` [tip:perf/core] perf pmu: " tip-bot for Andi Kleen
  2017-01-28  2:03 ` [PATCH 04/10] perf, tools: Support event aliases for non cpu// pmus Andi Kleen
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

Add support for registering json aliases per PMU. Any alias
with an unit matching the prefix is registered to the PMU.
Uncore has multiple instances of most units, so all
these aliases get registered for each individual PMU
(this is important later to run the event on every instance
of the PMU).

To avoid printing the events multiple times in perf list
filter out duplicated events during printing.

v2: Rely on uncore_ prefix already in unit
v3: Document why calls were reordered
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/pmu.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6dc3cc050105..8e9d00fd418e 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -590,14 +590,16 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_format(name, &format))
 		return NULL;
 
-	if (pmu_aliases(name, &aliases))
+	/*
+	 * Check the type first to avoid unnecessary work.
+	 */
+	if (pmu_type(name, &type))
 		return NULL;
 
-	pmu_add_cpu_aliases(&aliases, name);
-
-	if (pmu_type(name, &type))
+	if (pmu_aliases(name, &aliases))
 		return NULL;
 
+	pmu_add_cpu_aliases(&aliases, name);
 	pmu = zalloc(sizeof(*pmu));
 	if (!pmu)
 		return NULL;
@@ -1195,6 +1197,9 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 	len = j;
 	qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
 	for (j = 0; j < len; j++) {
+		/* Skip duplicates */
+		if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name))
+			continue;
 		if (name_only) {
 			printf("%s ", aliases[j].name);
 			continue;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 04/10] perf, tools: Support event aliases for non cpu// pmus
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (2 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 03/10] perf, tools: Support per pmu json aliases Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-10  7:42   ` [tip:perf/core] perf pmu: " tip-bot for Andi Kleen
  2017-01-28  2:03 ` [PATCH 05/10] perf, tools: Add debug support for outputing alias string Andi Kleen
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

The code for handling pmu aliases without specifying
the PMU hardcoded only supported the cpu PMU.

This patch extends it to work for all PMUs. We always
duplicate the event for all PMUs that have an matching alias.
This allows to automatically expand an alias for all instances
of a PMU (so for example you can monitor all cache boxes with
a single event)

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/parse-events.c | 46 ++++++++++++++++++++++++------------------
 tools/perf/util/parse-events.y | 32 ++++++++++++++++++++++-------
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3c876b8ba4de..6dbcba7f0969 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1504,35 +1504,41 @@ static void perf_pmu__parse_init(void)
 	struct perf_pmu_alias *alias;
 	int len = 0;
 
-	pmu = perf_pmu__find("cpu");
-	if ((pmu == NULL) || list_empty(&pmu->aliases)) {
+	pmu = NULL;
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (strchr(alias->name, '-'))
+				len++;
+			len++;
+		}
+	}
+
+	if (len == 0) {
 		perf_pmu_events_list_num = -1;
 		return;
 	}
-	list_for_each_entry(alias, &pmu->aliases, list) {
-		if (strchr(alias->name, '-'))
-			len++;
-		len++;
-	}
 	perf_pmu_events_list = malloc(sizeof(struct perf_pmu_event_symbol) * len);
 	if (!perf_pmu_events_list)
 		return;
 	perf_pmu_events_list_num = len;
 
 	len = 0;
-	list_for_each_entry(alias, &pmu->aliases, list) {
-		struct perf_pmu_event_symbol *p = perf_pmu_events_list + len;
-		char *tmp = strchr(alias->name, '-');
-
-		if (tmp != NULL) {
-			SET_SYMBOL(strndup(alias->name, tmp - alias->name),
-					PMU_EVENT_SYMBOL_PREFIX);
-			p++;
-			SET_SYMBOL(strdup(++tmp), PMU_EVENT_SYMBOL_SUFFIX);
-			len += 2;
-		} else {
-			SET_SYMBOL(strdup(alias->name), PMU_EVENT_SYMBOL);
-			len++;
+	pmu = NULL;
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			struct perf_pmu_event_symbol *p = perf_pmu_events_list + len;
+			char *tmp = strchr(alias->name, '-');
+
+			if (tmp != NULL) {
+				SET_SYMBOL(strndup(alias->name, tmp - alias->name),
+						PMU_EVENT_SYMBOL_PREFIX);
+				p++;
+				SET_SYMBOL(strdup(++tmp), PMU_EVENT_SYMBOL_SUFFIX);
+				len += 2;
+			} else {
+				SET_SYMBOL(strdup(alias->name), PMU_EVENT_SYMBOL);
+				len++;
+			}
 		}
 	}
 	qsort(perf_pmu_events_list, len,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 879115f93edc..f3b5ec901600 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -12,6 +12,7 @@
 #include <linux/list.h>
 #include <linux/types.h>
 #include "util.h"
+#include "pmu.h"
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
@@ -236,15 +237,32 @@ PE_KERNEL_PMU_EVENT sep_dc
 	struct list_head *head;
 	struct parse_events_term *term;
 	struct list_head *list;
+	struct perf_pmu *pmu = NULL;
+	int ok = 0;
 
-	ALLOC_LIST(head);
-	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1, &@1, NULL));
-	list_add_tail(&term->list, head);
-
+	/* Add it for all PMUs that support the alias */
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(data, list, "cpu", head));
-	parse_events_terms__delete(head);
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		struct perf_pmu_alias *alias;
+
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (!strcasecmp(alias->name, $1)) {
+				ALLOC_LIST(head);
+				ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, 1, &@1, NULL));
+				list_add_tail(&term->list, head);
+
+				if (!parse_events_add_pmu(data, list,
+						  pmu->name, head)) {
+					ok++;
+				}
+
+				parse_events_terms__delete(head);
+			}
+		}
+	}
+	if (!ok)
+		YYABORT;
 	$$ = list;
 }
 |
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 05/10] perf, tools: Add debug support for outputing alias string
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (3 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 04/10] perf, tools: Support event aliases for non cpu// pmus Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-10  7:43   ` [tip:perf/core] perf list: " tip-bot for Andi Kleen
  2017-01-28  2:03 ` [PATCH 06/10] perf, tools: Collapse identically named events in perf stat Andi Kleen
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

For debugging and testing it is useful to see the converted
alias string. Add support to perf stat/record and perf list to print
the alias conversion. The text string is saved in the alias structure.
For perf stat/record it is folded into the normal -v. For perf list
-v was taken, so we use --debug.

Before:

% perf list
...
cache:
  l1d.replacement
       [L1D data line replacements]
  l1d_pend_miss.fb_full
       [Cycles a demand request was blocked due to Fill Buffers inavailability]

After

% perf list --debug
...
cache:
  l1d.replacement
       [L1D data line replacements]
        cpu/umask=0x1,period=2000003,event=0x51/
  l1d_pend_miss.fb_full
       [Cycles a demand request was blocked due to Fill Buffers inavailability]
        cpu/umask=0x2,period=2000003,cmask=1,event=0x48/

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/builtin-list.c      | 3 +++
 tools/perf/util/parse-events.y | 3 +++
 tools/perf/util/pmu.c          | 8 ++++++++
 tools/perf/util/pmu.h          | 1 +
 4 files changed, 15 insertions(+)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index ba9322ff858b..3b9d98b5feef 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -14,6 +14,7 @@
 #include "util/parse-events.h"
 #include "util/cache.h"
 #include "util/pmu.h"
+#include "util/debug.h"
 #include <subcmd/parse-options.h>
 
 static bool desc_flag = true;
@@ -29,6 +30,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 			    "Print extra event descriptions. --no-desc to not print."),
 		OPT_BOOLEAN('v', "long-desc", &long_desc_flag,
 			    "Print longer event descriptions."),
+		OPT_INCR(0, "debug", &verbose,
+			     "Enable debugging output"),
 		OPT_END()
 	};
 	const char * const list_usage[] = {
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index f3b5ec901600..3a5196380609 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -13,6 +13,7 @@
 #include <linux/types.h>
 #include "util.h"
 #include "pmu.h"
+#include "debug.h"
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
@@ -254,6 +255,8 @@ PE_KERNEL_PMU_EVENT sep_dc
 
 				if (!parse_events_add_pmu(data, list,
 						  pmu->name, head)) {
+					pr_debug("%s -> %s/%s/\n", $1,
+						 pmu->name, alias->str);
 					ok++;
 				}
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8e9d00fd418e..82a654dec666 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -275,6 +275,8 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 		snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
 	}
 	alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
+	alias->str = strdup(val);
+
 	list_add_tail(&alias->list, list);
 
 	return 0;
@@ -1087,6 +1089,8 @@ struct sevent {
 	char *name;
 	char *desc;
 	char *topic;
+	char *str;
+	char *pmu;
 };
 
 static int cmp_sevent(const void *a, const void *b)
@@ -1183,6 +1187,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 			aliases[j].desc = long_desc ? alias->long_desc :
 						alias->desc;
 			aliases[j].topic = alias->topic;
+			aliases[j].str = alias->str;
+			aliases[j].pmu = pmu->name;
 			j++;
 		}
 		if (pmu->selectable &&
@@ -1217,6 +1223,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 			printf("%*s", 8, "[");
 			wordwrap(aliases[j].desc, 8, columns, 0);
 			printf("]\n");
+			if (verbose)
+				printf("%*s%s/%s/\n", 8, "", aliases[j].pmu, aliases[j].str);
 		} else
 			printf("  %-50s [Kernel PMU event]\n", aliases[j].name);
 		printed++;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 25712034c815..00852ddc7741 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -43,6 +43,7 @@ struct perf_pmu_alias {
 	char *desc;
 	char *long_desc;
 	char *topic;
+	char *str;
 	struct list_head terms; /* HEAD struct parse_events_term -> list */
 	struct list_head list;  /* ELEM */
 	char unit[UNIT_MAX_LEN+1];
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 06/10] perf, tools: Collapse identically named events in perf stat
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (4 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 05/10] perf, tools: Add debug support for outputing alias string Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-08 11:31   ` Jiri Olsa
  2017-01-28  2:03 ` [PATCH 07/10] perf, tools: Expand PMU events by prefix match Andi Kleen
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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.

This can be default because only the uncores generally have duplicated
aliases. Other PMUs have unique names.

Before:

% perf stat --no-merge -a  -e unc_c_llc_lookup.any sleep 1

 Performance counter stats for 'system wide':

           694,976 Bytes unc_c_llc_lookup.any
           706,304 Bytes unc_c_llc_lookup.any
           956,608 Bytes unc_c_llc_lookup.any
           782,720 Bytes unc_c_llc_lookup.any
           605,696 Bytes unc_c_llc_lookup.any
           442,816 Bytes unc_c_llc_lookup.any
           659,328 Bytes unc_c_llc_lookup.any
           509,312 Bytes unc_c_llc_lookup.any
           263,936 Bytes unc_c_llc_lookup.any
           592,448 Bytes unc_c_llc_lookup.any
           672,448 Bytes unc_c_llc_lookup.any
           608,640 Bytes unc_c_llc_lookup.any
           641,024 Bytes unc_c_llc_lookup.any
           856,896 Bytes unc_c_llc_lookup.any
           808,832 Bytes unc_c_llc_lookup.any
           684,864 Bytes unc_c_llc_lookup.any
           710,464 Bytes unc_c_llc_lookup.any
           538,304 Bytes unc_c_llc_lookup.any

       1.002577660 seconds time elapsed

After:

% perf stat  -a  -e unc_c_llc_lookup.any sleep 1

 Performance counter stats for 'system wide':

         2,685,120 Bytes unc_c_llc_lookup.any

       1.002648032 seconds time elapsed

v2: Split collect_aliases. Rename alias flag.
v3: Make sure unsupported/not counted is always printed.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-stat.txt |   3 +
 tools/perf/builtin-stat.c              | 145 +++++++++++++++++++++++++++------
 tools/perf/util/evsel.h                |   1 +
 3 files changed, 126 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 a02f2e965628..765346a581b3 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,79 @@ static void aggr_update_shadow(void)
 	}
 }
 
+static void collect_all_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);
+	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->merged_stat = true;
+		cb(alias, data, false);
+	}
+}
+
+static void collect_aliases(struct perf_evsel *counter,
+			    void (*cb)(struct perf_evsel *counter, void *data,
+				       bool first),
+			    void *data)
+{
+	cb(counter, data, true);
+	if (!no_merge)
+		collect_all_aliases(counter, cb, data);
+}
+
+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++) {
+		struct perf_counts_values *counts;
+
+		cpu2 = perf_evsel__cpus(counter)->map[cpu];
+		s2 = aggr_get_id(evsel_list->cpus, cpu2);
+		if (s2 != ad->id)
+			continue;
+		if (first)
+			ad->nr++;
+		counts = perf_counts(counter->counts, cpu, 0);
+		/*
+		 * When any result is bad, make them all to give
+		 * consistent output in interval mode.
+		 */
+		if (counts->ena == 0 || counts->run == 0 ||
+		    counter->counts->scaled == -1) {
+			ad->ena = 0;
+			ad->run = 0;
+			break;
+		}
+		ad->val += counts->val;
+		ad->ena += counts->ena;
+		ad->run += counts->run;
+	}
+}
+
 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 +1266,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->merged_stat)
+				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 +1325,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 +1347,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->merged_stat)
+		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 +1384,16 @@ static void print_counter(struct perf_evsel *counter, char *prefix)
 	double uval;
 	int cpu;
 
+	if (counter->merged_stat)
+		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 +1731,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 06ef6f29efa1..bd2e9b112d49 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -131,6 +131,7 @@ struct perf_evsel {
 	bool			cmdline_group_boundary;
 	struct list_head	config_terms;
 	int			bpf_fd;
+	bool			merged_stat;
 };
 
 union u64_swap {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 07/10] perf, tools: Expand PMU events by prefix match
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (5 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 06/10] perf, tools: Collapse identically named events in perf stat Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-08 11:30   ` Jiri Olsa
  2017-01-28  2:03 ` [PATCH 08/10] perf, tools: Add a simple expression parser for JSON Andi Kleen
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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,6}/.../ can be now specified as cbox/.../
and it gets automatically expanded.

Before

% perf stat -a -e uncore_cbox_0/event=0x35,umask=0x1,filter_opc=0x19C/,\
uncore_cbox_1/event=0x35,umask=0x1,filter_opc=0x19C/,\
uncore_cbox_2/event=0x35,umask=0x1,filter_opc=0x19C/,\
uncore_cbox_3/event=0x35,umask=0x1,filter_opc=0x19C/,\
uncore_cbox_4/event=0x35,umask=0x1,filter_opc=0x19C/,\
uncore_cbox_5/event=0x35,umask=0x1,filter_opc=0x19C/ sleep 1

After

perf stat -a -e cbox/event=0x35,umask=0x1,filter_opc=0x19C/ sleep 1

v2: Handle all bison rules. Move multi add code to separate function.
Handle uncore_ prefix correctly.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/parse-events.c | 71 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h |  8 +++++
 tools/perf/util/parse-events.y | 73 +++++++++++++++++-------------------------
 3 files changed, 109 insertions(+), 43 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6dbcba7f0969..fba53ba22431 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1257,6 +1257,52 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 	return evsel ? 0 : -ENOMEM;
 }
 
+int parse_events_multi_pmu_add(struct parse_events_evlist *data,
+			       char *str, struct list_head **listp)
+{
+	struct list_head *head;
+	struct parse_events_term *term;
+	struct list_head *list;
+	struct perf_pmu *pmu = NULL;
+	int ok = 0;
+
+	*listp = NULL;
+	/* Add it for all PMUs that support the alias */
+	list = malloc(sizeof(struct list_head));
+	if (!list)
+		return -1;
+	INIT_LIST_HEAD(list);
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		struct perf_pmu_alias *alias;
+
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (!strcasecmp(alias->name, str)) {
+				head = malloc(sizeof(struct list_head));
+				if (!head)
+					return -1;
+				INIT_LIST_HEAD(head);
+				if (parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+							   str, 1, &str, NULL) < 0)
+					return -1;
+				list_add_tail(&term->list, head);
+
+				if (!parse_events_add_pmu(data, list,
+						  pmu->name, head)) {
+					pr_debug("%s -> %s/%s/\n", str,
+						 pmu->name, alias->str);
+					ok++;
+				}
+
+				parse_events_terms__delete(head);
+			}
+		}
+	}
+	if (!ok)
+		return -1;
+	*listp = list;
+	return 0;
+}
+
 int parse_events__modifier_group(struct list_head *list,
 				 char *event_mod)
 {
@@ -2406,6 +2452,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..33b3e52cd9b8 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -164,6 +164,14 @@ 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_multi_pmu_add(struct parse_events_evlist *data,
+			       char *str,
+			       struct list_head **listp);
+
+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..1c2788f69bf1 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -224,68 +224,55 @@ 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) &&
+			    strncmp($1, "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;
 }
 |
 PE_KERNEL_PMU_EVENT sep_dc
 {
-	struct parse_events_evlist *data = _data;
-	struct list_head *head;
-	struct parse_events_term *term;
 	struct list_head *list;
-	struct perf_pmu *pmu = NULL;
-	int ok = 0;
-
-	/* Add it for all PMUs that support the alias */
-	ALLOC_LIST(list);
-	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
-		struct perf_pmu_alias *alias;
 
-		list_for_each_entry(alias, &pmu->aliases, list) {
-			if (!strcasecmp(alias->name, $1)) {
-				ALLOC_LIST(head);
-				ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1, &@1, NULL));
-				list_add_tail(&term->list, head);
-
-				if (!parse_events_add_pmu(data, list,
-						  pmu->name, head)) {
-					pr_debug("%s -> %s/%s/\n", $1,
-						 pmu->name, alias->str);
-					ok++;
-				}
-
-				parse_events_terms__delete(head);
-			}
-		}
-	}
-	if (!ok)
+	if (parse_events_multi_pmu_add(_data, $1, &list) < 0)
 		YYABORT;
 	$$ = list;
 }
 |
 PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
 {
-	struct parse_events_evlist *data = _data;
-	struct list_head *head;
-	struct parse_events_term *term;
 	struct list_head *list;
 	char pmu_name[128];
-	snprintf(&pmu_name, 128, "%s-%s", $1, $3);
-
-	ALLOC_LIST(head);
-	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					&pmu_name, 1, &@1, NULL));
-	list_add_tail(&term->list, head);
 
-	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(data, list, "cpu", head));
-	parse_events_terms__delete(head);
+	snprintf(&pmu_name, 128, "%s-%s", $1, $3);
+	if (parse_events_multi_pmu_add(_data, pmu_name, &list) < 0)
+		YYABORT;
 	$$ = list;
 }
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 08/10] perf, tools: Add a simple expression parser for JSON
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (6 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 07/10] perf, tools: Expand PMU events by prefix match Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-08 11:31   ` Jiri Olsa
  2017-01-28  2:03 ` [PATCH 09/10] perf, tools: Support MetricExpr header in JSON event list Andi Kleen
  2017-01-28  2:03 ` [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric Andi Kleen
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

Add a simple expression parser good enough to parse JSON relation
expressions. The parser is implemented using bison.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   4 ++
 tools/perf/tests/expr.c         |  48 +++++++++++++
 tools/perf/tests/tests.h        |   1 +
 tools/perf/util/Build           |   5 ++
 tools/perf/util/expr.h          |  23 ++++++
 tools/perf/util/expr.y          | 156 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 238 insertions(+)
 create mode 100644 tools/perf/tests/expr.c
 create mode 100644 tools/perf/util/expr.h
 create mode 100644 tools/perf/util/expr.y

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 1cb3d9b540e9..af58ebc243ef 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -38,6 +38,7 @@ perf-y += cpumap.o
 perf-y += stat.o
 perf-y += event_update.o
 perf-y += event-times.o
+perf-y += expr.o
 perf-y += backward-ring-buffer.o
 perf-y += sdt.o
 perf-y += is_printable_array.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 37e326bfd2dc..db03e853ba7f 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -44,6 +44,10 @@ static struct test generic_tests[] = {
 		.func = test__parse_events,
 	},
 	{
+		.desc = "Simple expression parser",
+		.func = test__expr,
+	},
+	{
 		.desc = "PERF_RECORD_* events & perf_sample fields",
 		.func = test__PERF_RECORD,
 	},
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
new file mode 100644
index 000000000000..38e4bb31692c
--- /dev/null
+++ b/tools/perf/tests/expr.c
@@ -0,0 +1,48 @@
+#include "util/debug.h"
+#include "util/expr.h"
+#include "tests.h"
+
+static int test(struct parse_ctx *ctx, const char *e, double val2)
+{
+	double val;
+
+	if (expr_parse(&val, ctx, &e))
+		TEST_ASSERT_VAL("parse test failed", 0);
+	TEST_ASSERT_VAL("unexpected value", val == val2);
+	return 0;
+}
+
+int test__expr(int subtest __maybe_unused)
+{
+	const char *p;
+	double val;
+	int ret;
+	struct parse_ctx ctx;
+
+	expr_ctx_init(&ctx);
+	expr_add_id(&ctx, "FOO", 1);
+	expr_add_id(&ctx, "BAR", 2);
+
+	ret = test(&ctx, "1+1", 2);
+	ret |= test(&ctx, "FOO+BAR", 3);
+	ret |= test(&ctx, "(BAR/2)%2", 1);
+	ret |= test(&ctx, "1 - -4",  5);
+	ret |= test(&ctx, "(FOO-1)*2 + (BAR/2)%2 - -4",  5);
+
+	if (ret)
+		return ret;
+
+	p = "FOO/0";
+	ret = expr_parse(&val, &ctx, &p);
+	TEST_ASSERT_VAL("division by zero", ret == 1);
+
+	p = "BAR/";
+	ret = expr_parse(&val, &ctx, &p);
+	TEST_ASSERT_VAL("missing operand", ret == 1);
+
+	TEST_ASSERT_VAL("find other", expr_find_other("FOO + BAR", "FOO", &p) == 0);
+	TEST_ASSERT_VAL("find other", !strcmp(p, "BAR"));
+	free((void *)p);
+
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 1fa9b9d83aa5..631859629403 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -62,6 +62,7 @@ int test__sample_parsing(int subtest);
 int test__keep_tracking(int subtest);
 int test__parse_no_sample_id_all(int subtest);
 int test__dwarf_unwind(int subtest);
+int test__expr(int subtest);
 int test__hists_filter(int subtest);
 int test__mmap_thread_lookup(int subtest);
 int test__thread_mg_share(int subtest);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 5da376bc1afc..887642ca609c 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -88,6 +88,7 @@ libperf-y += mem-events.o
 libperf-y += vsprintf.o
 libperf-y += drv_configs.o
 libperf-y += time-utils.o
+libperf-y += expr-bison.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
@@ -140,6 +141,10 @@ $(OUTPUT)util/parse-events-bison.c: util/parse-events.y
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,bison)$(BISON) -v util/parse-events.y -d $(PARSER_DEBUG_BISON) -o $@ -p parse_events_
 
+$(OUTPUT)util/expr-bison.c: util/expr.y
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,bison)$(BISON) -v util/expr.y -d $(PARSER_DEBUG_BISON) -o $@ -p expr_
+
 $(OUTPUT)util/pmu-flex.c: util/pmu.l $(OUTPUT)util/pmu-bison.c
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,flex)$(FLEX) -o $@ --header-file=$(OUTPUT)util/pmu-flex.h util/pmu.l
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
new file mode 100644
index 000000000000..3b99fa282059
--- /dev/null
+++ b/tools/perf/util/expr.h
@@ -0,0 +1,23 @@
+#ifndef PARSE_CTX_H
+#define PARSE_CTX_H 1
+
+#define MAX_PARSE_ID 2
+
+struct parse_id {
+	const char *name;
+	double val;
+};
+
+struct parse_ctx {
+	int num_ids;
+	struct parse_id ids[MAX_PARSE_ID];
+};
+
+void expr_ctx_init(struct parse_ctx *ctx);
+void expr_add_id(struct parse_ctx *ctx, const char *id, double val);
+#ifndef IN_EXPR_Y
+int expr_parse(double *final_val, struct parse_ctx *ctx, const char **pp);
+#endif
+int expr_find_other(const char *p, const char *one, const char **other);
+
+#endif
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
new file mode 100644
index 000000000000..9c2c39512b8d
--- /dev/null
+++ b/tools/perf/util/expr.y
@@ -0,0 +1,156 @@
+/* Simple expression parser */
+%{
+#include "util.h"
+#include "util/debug.h"
+#define IN_EXPR_Y 1
+#include "expr.h"
+#include <string.h>
+
+#define MAXIDLEN 256
+%}
+
+%define api.pure full
+%parse-param { double *final_val }
+%parse-param { struct parse_ctx *ctx }
+%parse-param { const char **pp }
+%lex-param { const char **pp }
+
+%union {
+	double num;
+	char id[MAXIDLEN+1];
+}
+
+%token <num> NUMBER
+%token <id> ID
+%left '|'
+%left '^'
+%left '&'
+%left '-' '+'
+%left '*' '/' '%'
+%left NEG NOT
+%type <num> expr
+
+%{
+static int expr_lex(YYSTYPE *res, const char **pp);
+
+static void expr_error(double *final_val __maybe_unused,
+		       struct parse_ctx *ctx __maybe_unused,
+		       const char **pp __maybe_unused,
+		       const char *s)
+{
+	pr_debug("%s", s);
+}
+
+static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
+{
+	int i;
+
+	for (i = 0; i < ctx->num_ids; i++) {
+		if (!strcasecmp(ctx->ids[i].name, id)) {
+			*val = ctx->ids[i].val;
+			return 0;
+		}
+	}
+	return -1;
+}
+
+%}
+%%
+
+all_expr: expr			{ *final_val = $1; }
+	;
+
+expr:	  NUMBER
+	| ID			{ if (lookup_id(ctx, $1, &$$) < 0) {
+					pr_debug("%s not found", $1);
+					YYABORT;
+				  }
+				}
+	| 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; }
+	;
+
+%%
+
+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 - s >= MAXIDLEN)
+			return -1;
+		*dst++ = *p++;
+	}
+	*dst = 0;
+	*pp = p;
+	return ID;
+}
+
+static int expr_lex(YYSTYPE *res, const char **pp)
+{
+	int tok;
+	const char *s;
+	const char *p = *pp;
+
+	while (isspace(*p))
+		p++;
+	s = p;
+	switch (*p++) {
+	case 'a' ... 'z':
+	case 'A' ... 'Z':
+		return expr_symbol(res, p - 1, pp);
+	case '0' ... '9': case '.':
+		res->num = strtod(s, (char **)&p);
+		tok = NUMBER;
+		break;
+	default:
+		tok = *s;
+		break;
+	}
+	*pp = p;
+	return tok;
+}
+
+/* Caller must make sure id is allocated */
+void expr_add_id(struct parse_ctx *ctx, const char *name, double val)
+{
+	int idx;
+	assert(ctx->num_ids < MAX_PARSE_ID);
+	idx = ctx->num_ids++;
+	ctx->ids[idx].name = name;
+	ctx->ids[idx].val = val;
+}
+
+void expr_ctx_init(struct parse_ctx *ctx)
+{
+	ctx->num_ids = 0;
+}
+
+int expr_find_other(const char *p, const char *one, const char **other)
+{
+	const char *orig = p;
+
+	*other = NULL;
+	for (;;) {
+		YYSTYPE val;
+		int tok = expr_lex(&val, &p);
+		if (tok == 0)
+			break;
+		if (tok == ID && strcasecmp(one, val.id)) {
+			if (*other) {
+				pr_debug("More than one extra identifier in %s\n", orig);
+				return -1;
+			}
+			*other = strdup(val.id);
+			if (!*other)
+				return -1;
+		}
+	}
+	return 0;
+}
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 09/10] perf, tools: Support MetricExpr header in JSON event list
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (7 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 08/10] perf, tools: Add a simple expression parser for JSON Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-08 11:31   ` Jiri Olsa
  2017-01-28  2:03 ` [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric Andi Kleen
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

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

Used in the next patch.

v2: Change DividedBy to MetricExpr
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 eed09346a72a..0735dc2a167a 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -291,7 +291,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 *metric_expr)
 {
 	struct perf_entry_data *pd = data;
 	FILE *outfp = pd->outfp;
@@ -315,6 +316,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 (metric_expr)
+		fprintf(outfp, "\t.metric_expr = \"%s\",\n", metric_expr);
 	fprintf(outfp, "},\n");
 
 	return 0;
@@ -362,7 +365,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 *metric_expr),
 	  void *data)
 {
 	int err = -EIO;
@@ -388,6 +392,7 @@ int json_events(const char *fn,
 		char *filter = NULL;
 		char *perpkg = NULL;
 		char *unit = NULL;
+		char *metric_expr = NULL;
 		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
@@ -398,6 +403,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,
@@ -444,7 +450,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) {
@@ -464,6 +469,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, "MetricExpr")) {
+				addfield(map, &metric_expr, "", "", val);
+				for (s = metric_expr; *s; s++)
+					*s = tolower(*s);
 			}
 			/* ignore unknown fields */
 		}
@@ -488,7 +497,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, metric_expr);
 		free(event);
 		free(desc);
 		free(name);
@@ -498,6 +507,7 @@ int json_events(const char *fn,
 		free(filter);
 		free(perpkg);
 		free(unit);
+		free(metric_expr);
 		if (err)
 			break;
 		tok += j;
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index 71e13de31092..57e111bf2168 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 *metric_expr),
 		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..d046e3a4ce46 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 *metric_expr;
 };
 
 /*
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 82a654dec666..b78b348068d7 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -231,7 +231,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, char *val,
 				 char *long_desc, char *topic,
-				 char *unit, char *perpkg)
+				 char *unit, char *perpkg,
+				 char *dividedby)
 {
 	struct perf_pmu_alias *alias;
 	int ret;
@@ -265,6 +266,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;
@@ -294,7 +296,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)
@@ -564,7 +566,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.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
                   ` (8 preceding siblings ...)
  2017-01-28  2:03 ` [PATCH 09/10] perf, tools: Support MetricExpr header in JSON event list Andi Kleen
@ 2017-01-28  2:03 ` Andi Kleen
  2017-02-08 11:31   ` Jiri Olsa
  9 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-01-28  2:03 UTC (permalink / raw)
  To: acme; +Cc: jolsa, linux-kernel, Andi Kleen

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

Add generic infrastructure to perf stat to output ratios for "MetricExpr"
entries in the event lists. Many events are more useful as ratios
than in raw form, typically some count in relation to total ticks.

Transfer the MetricExpr information from the alias to the evsel.

We mark the events that need to be collected for MetricExpr, and also
link the events using them with a pointer. The code is careful
to always prefer the right event in the same group to minimize
multiplexing errors. At the moment only a single relation is supported.

Then add a rblist to the stat shadow code that remembers stats based
on the cpu and context.

Then finally update and retrieve and print these values similarly to the
existing hardcoded perf metrics. We use the simple expression parser
added earlier to evaluate the expression.

Normally we just output the result without further commentary,
but for --metric-only this would lead to empty columns. So for this
case use the original event as description.

So far there is no attempt to automatically add the MetricExpr event,
if it is missing, however we suggest it to the user.

$ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
     1.000228813        800,139,950      unc_p_clockticks
     1.000228813        789,833,783      unc_p_freq_max_os_cycles  #     98.7
     2.000654229        800,308,990      unc_p_clockticks
     2.000654229        396,214,238      unc_p_freq_max_os_cycles  #     49.5

$ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}' --metric-only
     1.000206740     48.0
     2.000451543     48.1

v2: Change from DivideBy to MetricExpr
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/builtin-stat.c      |   3 +
 tools/perf/util/evsel.c        |   3 +
 tools/perf/util/evsel.h        |   3 +
 tools/perf/util/parse-events.c |   1 +
 tools/perf/util/pmu.c          |   8 ++-
 tools/perf/util/pmu.h          |   3 +-
 tools/perf/util/stat-shadow.c  | 152 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/stat.h         |   2 +
 8 files changed, 171 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 765346a581b3..806d957bbe6a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1141,6 +1141,7 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval,
 	out.print_metric = pm;
 	out.new_line = nl;
 	out.ctx = &os;
+	out.force_header = false;
 
 	if (csv_output && !metric_only) {
 		print_noise(counter, noise);
@@ -1478,6 +1479,7 @@ static void print_metric_headers(const char *prefix, bool no_indent)
 		out.ctx = &os;
 		out.print_metric = print_metric_header;
 		out.new_line = new_line_metric;
+		out.force_header = true;
 		os.evsel = counter;
 		perf_stat__print_shadow_stats(counter, 0,
 					      0,
@@ -2460,6 +2462,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 	argc = parse_options_subcommand(argc, argv, stat_options, stat_subcommands,
 					(const char **) stat_usage,
 					PARSE_OPT_STOP_AT_NON_OPTION);
+	perf_stat__collect_metric_expr(evsel_list);
 	perf_stat__init_shadow_stats();
 
 	if (csv_sep) {
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 04e536ae4d88..8db6c375487f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -236,6 +236,9 @@ void perf_evsel__init(struct perf_evsel *evsel,
 	evsel->sample_size = __perf_evsel__sample_size(attr->sample_type);
 	perf_evsel__calc_id_pos(evsel);
 	evsel->cmdline_group_boundary = false;
+	evsel->metric_expr   = NULL;
+	evsel->metric_event   = NULL;
+	evsel->collect_stat = false;
 }
 
 struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index bd2e9b112d49..92b0a9a3535d 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -132,6 +132,9 @@ struct perf_evsel {
 	struct list_head	config_terms;
 	int			bpf_fd;
 	bool			merged_stat;
+	const char *		metric_expr;
+	struct perf_evsel	*metric_event;
+	bool			collect_stat;
 };
 
 union u64_swap {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index fba53ba22431..6adc284ff434 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1252,6 +1252,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data,
 		evsel->scale = info.scale;
 		evsel->per_pkg = info.per_pkg;
 		evsel->snapshot = info.snapshot;
+		evsel->metric_expr = info.metric_expr;
 	}
 
 	return evsel ? 0 : -ENOMEM;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index b78b348068d7..28b9c5f0b547 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -232,7 +232,7 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 				 char *desc, char *val,
 				 char *long_desc, char *topic,
 				 char *unit, char *perpkg,
-				 char *dividedby)
+				 char *metric_expr)
 {
 	struct perf_pmu_alias *alias;
 	int ret;
@@ -266,7 +266,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->metric_expr = metric_expr ? strdup(metric_expr) : NULL;
 	alias->desc = desc ? strdup(desc) : NULL;
 	alias->long_desc = long_desc ? strdup(long_desc) :
 				desc ? strdup(desc) : NULL;
@@ -567,7 +567,7 @@ static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 				(char *)pe->desc, (char *)pe->event,
 				(char *)pe->long_desc, (char *)pe->topic,
 				(char *)pe->unit, (char *)pe->perpkg,
-				(char *)pe->dividedby);
+				(char *)pe->metric_expr);
 	}
 
 out:
@@ -985,6 +985,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
 	info->unit     = NULL;
 	info->scale    = 0.0;
 	info->snapshot = false;
+	info->metric_expr = NULL;
 
 	list_for_each_entry_safe(term, h, head_terms, list) {
 		alias = pmu_find_alias(pmu, term);
@@ -1000,6 +1001,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
 
 		if (alias->per_pkg)
 			info->per_pkg = true;
+		info->metric_expr = alias->metric_expr;
 
 		list_del(&term->list);
 		free(term);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index faf8a7f97d03..27f078ccc594 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -31,6 +31,7 @@ struct perf_pmu {
 
 struct perf_pmu_info {
 	const char *unit;
+	const char *metric_expr;
 	double scale;
 	bool per_pkg;
 	bool snapshot;
@@ -50,7 +51,7 @@ struct perf_pmu_alias {
 	double scale;
 	bool per_pkg;
 	bool snapshot;
-	char *dividedby;
+	char *metric_expr;
 };
 
 struct perf_pmu *perf_pmu__find(const char *name);
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 8a2bbd2a4d82..f22178ddc0ef 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -3,6 +3,9 @@
 #include "stat.h"
 #include "color.h"
 #include "pmu.h"
+#include "rblist.h"
+#include "evlist.h"
+#include "expr.h"
 
 enum {
 	CTX_BIT_USER	= 1 << 0,
@@ -41,13 +44,73 @@ static struct stats runtime_topdown_slots_issued[NUM_CTX][MAX_NR_CPUS];
 static struct stats runtime_topdown_slots_retired[NUM_CTX][MAX_NR_CPUS];
 static struct stats runtime_topdown_fetch_bubbles[NUM_CTX][MAX_NR_CPUS];
 static struct stats runtime_topdown_recovery_bubbles[NUM_CTX][MAX_NR_CPUS];
+static struct rblist runtime_saved_values;
 static bool have_frontend_stalled;
 
 struct stats walltime_nsecs_stats;
 
+struct saved_value {
+	struct rb_node rb_node;
+	struct perf_evsel *evsel;
+	int cpu;
+	int ctx;
+	struct stats stats;
+};
+
+static int saved_value_cmp(struct rb_node *rb_node, const void *entry)
+{
+	struct saved_value *a = container_of(rb_node,
+					     struct saved_value,
+					     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;
+	return a->evsel - b->evsel;
+}
+
+static struct rb_node *saved_value_new(struct rblist *rblist __maybe_unused,
+				     const void *entry)
+{
+	struct saved_value *nd = malloc(sizeof(struct saved_value));
+
+	if (!nd)
+		return NULL;
+	memcpy(nd, entry, sizeof(struct saved_value));
+	return &nd->rb_node;
+}
+
+static struct saved_value *saved_value_lookup(struct perf_evsel *evsel,
+					      int cpu, int ctx,
+					      bool create)
+{
+	struct rb_node *nd;
+	struct saved_value dm = {
+		.cpu = cpu,
+		.ctx = ctx,
+		.evsel = evsel,
+	};
+	nd = rblist__find(&runtime_saved_values, &dm);
+	if (nd)
+		return container_of(nd, struct saved_value, rb_node);
+	if (create) {
+		rblist__add_node(&runtime_saved_values, &dm);
+		nd = rblist__find(&runtime_saved_values, &dm);
+		if (nd)
+			return container_of(nd, struct saved_value, rb_node);
+	}
+	return NULL;
+}
+
 void perf_stat__init_shadow_stats(void)
 {
 	have_frontend_stalled = pmu_have_event("cpu", "stalled-cycles-frontend");
+	rblist__init(&runtime_saved_values);
+	runtime_saved_values.node_cmp = saved_value_cmp;
+	runtime_saved_values.node_new = saved_value_new;
+	/* No delete for now */
 }
 
 static int evsel_context(struct perf_evsel *evsel)
@@ -70,6 +133,8 @@ static int evsel_context(struct perf_evsel *evsel)
 
 void perf_stat__reset_shadow_stats(void)
 {
+	struct rb_node *pos, *next;
+
 	memset(runtime_nsecs_stats, 0, sizeof(runtime_nsecs_stats));
 	memset(runtime_cycles_stats, 0, sizeof(runtime_cycles_stats));
 	memset(runtime_stalled_cycles_front_stats, 0, sizeof(runtime_stalled_cycles_front_stats));
@@ -92,6 +157,15 @@ void perf_stat__reset_shadow_stats(void)
 	memset(runtime_topdown_slots_issued, 0, sizeof(runtime_topdown_slots_issued));
 	memset(runtime_topdown_fetch_bubbles, 0, sizeof(runtime_topdown_fetch_bubbles));
 	memset(runtime_topdown_recovery_bubbles, 0, sizeof(runtime_topdown_recovery_bubbles));
+
+	next = rb_first(&runtime_saved_values.entries);
+	while (next) {
+		pos = next;
+		next = rb_next(pos);
+		memset(&container_of(pos, struct saved_value, rb_node)->stats,
+		       0,
+		       sizeof(struct stats));
+	}
 }
 
 /*
@@ -143,6 +217,12 @@ void perf_stat__update_shadow_stats(struct perf_evsel *counter, u64 *count,
 		update_stats(&runtime_dtlb_cache_stats[ctx][cpu], count[0]);
 	else if (perf_evsel__match(counter, HW_CACHE, HW_CACHE_ITLB))
 		update_stats(&runtime_itlb_cache_stats[ctx][cpu], count[0]);
+
+	if (counter->collect_stat) {
+		struct saved_value *v = saved_value_lookup(counter, cpu, ctx,
+							   true);
+		update_stats(&v->stats, count[0]);
+	}
 }
 
 /* used for get_ratio_color() */
@@ -172,6 +252,60 @@ static const char *get_ratio_color(enum grc_type type, double ratio)
 	return color;
 }
 
+static struct perf_evsel *perf_stat__find_event(struct perf_evlist *evsel_list,
+						const char *name)
+{
+	struct perf_evsel *c2;
+
+	evlist__for_each_entry (evsel_list, c2) {
+		if (!strcasecmp(c2->name, name))
+			return c2;
+	}
+	return NULL;
+}
+
+/* Mark DividedBy target events and link events using them to them. */
+void perf_stat__collect_metric_expr(struct perf_evlist *evsel_list)
+{
+	struct perf_evsel *counter, *leader, *c2;
+	bool found;
+	const char *metric_event;
+
+	evlist__for_each_entry(evsel_list, counter) {
+		leader = counter->leader;
+		if (!counter->metric_expr)
+			continue;
+		if (expr_find_other(counter->metric_expr, counter->name,
+					&metric_event) < 0)
+			continue;
+
+		found = false;
+		if (leader) {
+			/* Search in group */
+			for_each_group_member (c2, leader) {
+				if (!strcasecmp(c2->name, metric_event)) {
+					found = true;
+					break;
+				}
+			}
+		}
+		if (!found) {
+			/* Search ignoring groups */
+			c2 = perf_stat__find_event(evsel_list, metric_event);
+		}
+		if (!c2) {
+			/* Could try to automatically add the event here. */
+			fprintf(stderr, "Add %s to groups to get metric event for %s\n",
+						metric_event,
+						counter->name);
+			counter->metric_expr = NULL;
+			continue;
+		}
+		counter->metric_event = c2;
+		c2->collect_stat = true;
+	}
+}
+
 static void print_stalled_cycles_frontend(int cpu,
 					  struct perf_evsel *evsel, double avg,
 					  struct perf_stat_output_ctx *out)
@@ -614,6 +748,24 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 					be_bound * 100.);
 		else
 			print_metric(ctxp, NULL, NULL, name, 0);
+	} else if (evsel->metric_expr) {
+		struct saved_value *v = saved_value_lookup(evsel->metric_event, cpu, ctx,
+							   false);
+		if (v) {
+			struct parse_ctx pctx;
+			const char *p;
+
+			expr_ctx_init(&pctx);
+			expr_add_id(&pctx, evsel->name, avg);
+			expr_add_id(&pctx, evsel->metric_event->name, avg_stats(&v->stats));
+			p = evsel->metric_expr;
+			if (expr_parse(&ratio, &pctx, &p) == 0)
+				print_metric(ctxp, NULL, "%8.1f",
+					out->force_header ? evsel->name : "",
+					ratio);
+			else
+				print_metric(ctxp, NULL, NULL, "", 0);
+		}
 	} else if (runtime_nsecs_stats[cpu].n != 0) {
 		char unit = 'M';
 		char unit_buf[10];
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index c29bb94c48a4..0a65ae23f495 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -85,11 +85,13 @@ struct perf_stat_output_ctx {
 	void *ctx;
 	print_metric_t print_metric;
 	new_line_t new_line;
+	bool force_header;
 };
 
 void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 				   double avg, int cpu,
 				   struct perf_stat_output_ctx *out);
+void perf_stat__collect_metric_expr(struct perf_evlist *);
 
 int perf_evlist__alloc_stats(struct perf_evlist *evlist, bool alloc_raw);
 void perf_evlist__free_stats(struct perf_evlist *evlist);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 07/10] perf, tools: Expand PMU events by prefix match
  2017-01-28  2:03 ` [PATCH 07/10] perf, tools: Expand PMU events by prefix match Andi Kleen
@ 2017-02-08 11:30   ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2017-02-08 11:30 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Fri, Jan 27, 2017 at 06:03:42PM -0800, Andi Kleen wrote:

SNIP

>  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..1c2788f69bf1 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -224,68 +224,55 @@ 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) &&
> +			    strncmp($1, "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);

could you please split this into 2 changes:
  - adding parse_events_multi_pmu_add with no functional changes
  - uncore box related matching

thanks,
jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 08/10] perf, tools: Add a simple expression parser for JSON
  2017-01-28  2:03 ` [PATCH 08/10] perf, tools: Add a simple expression parser for JSON Andi Kleen
@ 2017-02-08 11:31   ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2017-02-08 11:31 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Fri, Jan 27, 2017 at 06:03:43PM -0800, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
> new file mode 100644
> index 000000000000..3b99fa282059
> --- /dev/null
> +++ b/tools/perf/util/expr.h
> @@ -0,0 +1,23 @@
> +#ifndef PARSE_CTX_H
> +#define PARSE_CTX_H 1
> +
> +#define MAX_PARSE_ID 2
> +
> +struct parse_id {
> +	const char *name;
> +	double val;
> +};
> +
> +struct parse_ctx {
> +	int num_ids;
> +	struct parse_id ids[MAX_PARSE_ID];
> +};
> +
> +void expr_ctx_init(struct parse_ctx *ctx);
> +void expr_add_id(struct parse_ctx *ctx, const char *id, double val);
> +#ifndef IN_EXPR_Y
> +int expr_parse(double *final_val, struct parse_ctx *ctx, const char **pp);
> +#endif
> +int expr_find_other(const char *p, const char *one, const char **other);

it all looks ok, just please follow the interface function
name style we try to keep with struct name separated with __

thanks,
jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 06/10] perf, tools: Collapse identically named events in perf stat
  2017-01-28  2:03 ` [PATCH 06/10] perf, tools: Collapse identically named events in perf stat Andi Kleen
@ 2017-02-08 11:31   ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2017-02-08 11:31 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Fri, Jan 27, 2017 at 06:03:41PM -0800, Andi Kleen wrote:

SNIP

> +static void collect_all_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);
> +	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->merged_stat = true;
> +		cb(alias, data, false);
> +	}
> +}
> +
> +static void collect_aliases(struct perf_evsel *counter,
> +			    void (*cb)(struct perf_evsel *counter, void *data,
> +				       bool first),
> +			    void *data)
> +{
> +	cb(counter, data, true);
> +	if (!no_merge)
> +		collect_all_aliases(counter, cb, data);
> +}

could you please split this into 2 changes:
  - adding adding collect_aliases function with no functional change
    just changing all the print callers to use it
  - adding collect_all_aliases change that actualy change the behaviour
    and merges the stats

also please change the collect_aliases name to something
generic like collect data

thanks,
jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 09/10] perf, tools: Support MetricExpr header in JSON event list
  2017-01-28  2:03 ` [PATCH 09/10] perf, tools: Support MetricExpr header in JSON event list Andi Kleen
@ 2017-02-08 11:31   ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2017-02-08 11:31 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Fri, Jan 27, 2017 at 06:03:44PM -0800, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 82a654dec666..b78b348068d7 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -231,7 +231,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, char *val,
>  				 char *long_desc, char *topic,
> -				 char *unit, char *perpkg)
> +				 char *unit, char *perpkg,
> +				 char *dividedby)
>  {
>  	struct perf_pmu_alias *alias;
>  	int ret;
> @@ -265,6 +266,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;

s/dividedby/metric_expr/ ?

  CC       util/pmu.o
util/pmu.c: In function ‘pmu_add_cpu_aliases’:
util/pmu.c:570:15: error: ‘struct pmu_event’ has no member named ‘dividedby’
     (char *)pe->dividedby);


>  	alias->desc = desc ? strdup(desc) : NULL;
>  	alias->long_desc = long_desc ? strdup(long_desc) :
>  				desc ? strdup(desc) : NULL;
> @@ -294,7 +296,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)
> @@ -564,7 +566,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);

ditto

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

ditto

>  };
>  
>  struct perf_pmu *perf_pmu__find(const char *name);
> -- 
> 2.9.3
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-01-28  2:03 ` [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric Andi Kleen
@ 2017-02-08 11:31   ` Jiri Olsa
  2017-02-08 21:51     ` Andi Kleen
  0 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2017-02-08 11:31 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Fri, Jan 27, 2017 at 06:03:45PM -0800, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add generic infrastructure to perf stat to output ratios for "MetricExpr"
> entries in the event lists. Many events are more useful as ratios
> than in raw form, typically some count in relation to total ticks.
> 
> Transfer the MetricExpr information from the alias to the evsel.
> 
> We mark the events that need to be collected for MetricExpr, and also
> link the events using them with a pointer. The code is careful
> to always prefer the right event in the same group to minimize
> multiplexing errors. At the moment only a single relation is supported.
> 
> Then add a rblist to the stat shadow code that remembers stats based
> on the cpu and context.
> 
> Then finally update and retrieve and print these values similarly to the
> existing hardcoded perf metrics. We use the simple expression parser
> added earlier to evaluate the expression.
> 
> Normally we just output the result without further commentary,
> but for --metric-only this would lead to empty columns. So for this
> case use the original event as description.
> 
> So far there is no attempt to automatically add the MetricExpr event,
> if it is missing, however we suggest it to the user.
> 
> $ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
>      1.000228813        800,139,950      unc_p_clockticks
>      1.000228813        789,833,783      unc_p_freq_max_os_cycles  #     98.7
>      2.000654229        800,308,990      unc_p_clockticks
>      2.000654229        396,214,238      unc_p_freq_max_os_cycles  #     49.5

[jolsa@krava perf]$ ./perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
invalid or unsupported event: '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
Run 'perf list' for a list of valid events

could you show an example of the MetricExpr?

it's part of the event record, what if you wanted to have 2 or more metrics defined for event?

who defines those expressions?

what if you dont provide the necessary events needed for the expression?

shouldnt we do it the other way around? like pick an expression
we are interested in and perf will configure the needful..

> 
> $ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}' --metric-only
>      1.000206740     48.0
>      2.000451543     48.1

how does user know what those number stand for?

is there a way for user to display the metric expression?

it's still feels to me like a hack without much concept behind

jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-08 11:31   ` Jiri Olsa
@ 2017-02-08 21:51     ` Andi Kleen
  2017-02-09 11:39       ` Jiri Olsa
  2017-02-09 11:42       ` Jiri Olsa
  0 siblings, 2 replies; 28+ messages in thread
From: Andi Kleen @ 2017-02-08 21:51 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Andi Kleen, acme, jolsa, linux-kernel, Andi Kleen

On Wed, Feb 08, 2017 at 12:31:34PM +0100, Jiri Olsa wrote:
> On Fri, Jan 27, 2017 at 06:03:45PM -0800, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Add generic infrastructure to perf stat to output ratios for "MetricExpr"
> > entries in the event lists. Many events are more useful as ratios
> > than in raw form, typically some count in relation to total ticks.
> > 
> > Transfer the MetricExpr information from the alias to the evsel.
> > 
> > We mark the events that need to be collected for MetricExpr, and also
> > link the events using them with a pointer. The code is careful
> > to always prefer the right event in the same group to minimize
> > multiplexing errors. At the moment only a single relation is supported.
> > 
> > Then add a rblist to the stat shadow code that remembers stats based
> > on the cpu and context.
> > 
> > Then finally update and retrieve and print these values similarly to the
> > existing hardcoded perf metrics. We use the simple expression parser
> > added earlier to evaluate the expression.
> > 
> > Normally we just output the result without further commentary,
> > but for --metric-only this would lead to empty columns. So for this
> > case use the original event as description.
> > 
> > So far there is no attempt to automatically add the MetricExpr event,
> > if it is missing, however we suggest it to the user.
> > 
> > $ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
> >      1.000228813        800,139,950      unc_p_clockticks
> >      1.000228813        789,833,783      unc_p_freq_max_os_cycles  #     98.7
> >      2.000654229        800,308,990      unc_p_clockticks
> >      2.000654229        396,214,238      unc_p_freq_max_os_cycles  #     49.5
> 
> [jolsa@krava perf]$ ./perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
> invalid or unsupported event: '{unc_p_clockticks,unc_p_freq_max_os_cycles}'
> Run 'perf list' for a list of valid events
> 
> could you show an example of the MetricExpr?

It's in the event list branch

https://git.kernel.org/cgit/linux/kernel/git/ak/linux-misc.git/log/?h=perf/intel-uncore-json-files-3

All the metrics currently do is just the same as DividedBy earlier:
generate a percentage out of a count, typically based on clock ticks.

+        "MetricExpr": "(UNC_M_POWER_CHANNEL_PPD / UNC_M_CLOCKTICKS) *
100.",



> 
> it's part of the event record, what if you wanted to have 2 or more metrics defined for event?

Would need multiple copies of the event.

> 
> who defines those expressions?

It's metrics used internally by Intel.

> 
> what if you dont provide the necessary events needed for the expression?

Then perf prints a warning suggesting the events.

It's currently a TODO to add them automatically. Could be added,
but the patch was already complex, so I didn't add it.

It's somewhat complicated because you would need to avoid
duplicates and have to handle groups correctly. perf stat
doesn't have the necessarily knowledge to fully understand
the constraints on groups. 

Then the extra event may not fit into the group, and it seemed
saner to let the user decide what to do then, instead of
generating a possible unschedulable group.

So it's not that easy to do it automatically.

> > 
> > $ perf stat -a -I 1000 -e '{unc_p_clockticks,unc_p_freq_max_os_cycles}' --metric-only
> >      1.000206740     48.0
> >      2.000451543     48.1
> 
> how does user know what those number stand for?

I assume it was obvious enough that it is the percentage. Could
add something more to the event description.

> 
> is there a way for user to display the metric expression?

Only in the source, but could be added to perf list -v

For most users metrics are much more useful than raw numbers.
I think they would rather consider raw numbers to be a "hack
with no concept"

-Andi

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-08 21:51     ` Andi Kleen
@ 2017-02-09 11:39       ` Jiri Olsa
  2017-02-09 17:00         ` Andi Kleen
  2017-02-09 11:42       ` Jiri Olsa
  1 sibling, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2017-02-09 11:39 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Wed, Feb 08, 2017 at 01:51:05PM -0800, Andi Kleen wrote:

SNIP

> > 
> > could you show an example of the MetricExpr?
> 
> It's in the event list branch
> 
> https://git.kernel.org/cgit/linux/kernel/git/ak/linux-misc.git/log/?h=perf/intel-uncore-json-files-3
> 
> All the metrics currently do is just the same as DividedBy earlier:
> generate a percentage out of a count, typically based on clock ticks.
> 
> +        "MetricExpr": "(UNC_M_POWER_CHANNEL_PPD / UNC_M_CLOCKTICKS) *
> 100.",
> 
> 
> 
> > 
> > it's part of the event record, what if you wanted to have 2 or more metrics defined for event?
> 
> Would need multiple copies of the event.

this ...

> 
> > 
> > who defines those expressions?
> 
> It's metrics used internally by Intel.
> 
> > 
> > what if you dont provide the necessary events needed for the expression?
> 
> Then perf prints a warning suggesting the events.
> 
> It's currently a TODO to add them automatically. Could be added,
> but the patch was already complex, so I didn't add it.
> 
> It's somewhat complicated because you would need to avoid
> duplicates and have to handle groups correctly. perf stat
> doesn't have the necessarily knowledge to fully understand
> the constraints on groups. 
> 
> Then the extra event may not fit into the group, and it seemed
> saner to let the user decide what to do then, instead of
> generating a possible unschedulable group.
> 
> So it's not that easy to do it automatically.

and this makes me think, that this is not the right approach

adding extra copy of an event when you want to add new expression?

why can't we have another list/file of those expressions
from which point we could point and configure events we need

jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-08 21:51     ` Andi Kleen
  2017-02-09 11:39       ` Jiri Olsa
@ 2017-02-09 11:42       ` Jiri Olsa
  1 sibling, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2017-02-09 11:42 UTC (permalink / raw)
  To: Andi Kleen; +Cc: acme, jolsa, linux-kernel, Andi Kleen

On Wed, Feb 08, 2017 at 01:51:05PM -0800, Andi Kleen wrote:

SNIP

> Only in the source, but could be added to perf list -v
> 
> For most users metrics are much more useful than raw numbers.
> I think they would rather consider raw numbers to be a "hack
> with no concept"

well, we can always fix that by showing the column header,
but that's not what I meant

jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-09 11:39       ` Jiri Olsa
@ 2017-02-09 17:00         ` Andi Kleen
  2017-02-09 18:37           ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-02-09 17:00 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Andi Kleen, acme, jolsa, linux-kernel

On Thu, Feb 09, 2017 at 12:39:37PM +0100, Jiri Olsa wrote:
> and this makes me think, that this is not the right approach
> 
> adding extra copy of an event when you want to add new expression?

I don't want to add new expressions.

I don't even need arbitrary expressions, just DividedBy
to get percentages, you just forced me to do the expressions.


> why can't we have another list/file of those expressions

The last time I proposed separate files Ingo vetoed it.
He wanted everything built in.

> from which point we could point and configure events we need

If you want full flexibility you can use your perf stat report
approach, or what most people do is to just run a script/spreadsheet
over the the -x; output. This all continues to work.

This is just a minimum approach to provide some convenience
integrated with the event list to provide something similar
as the built in expressions in stat-shadow. 

It's not trying to build the great perf scripting language.

-Andi

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-09 17:00         ` Andi Kleen
@ 2017-02-09 18:37           ` Jiri Olsa
  2017-02-09 18:59             ` Andi Kleen
  0 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2017-02-09 18:37 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andi Kleen, acme, jolsa, linux-kernel

On Thu, Feb 09, 2017 at 09:00:35AM -0800, Andi Kleen wrote:
> On Thu, Feb 09, 2017 at 12:39:37PM +0100, Jiri Olsa wrote:
> > and this makes me think, that this is not the right approach
> > 
> > adding extra copy of an event when you want to add new expression?
> 
> I don't want to add new expressions.
> 
> I don't even need arbitrary expressions, just DividedBy
> to get percentages, you just forced me to do the expressions.
> 
> 
> > why can't we have another list/file of those expressions
> 
> The last time I proposed separate files Ingo vetoed it.
> He wanted everything built in.

sure, he veto it for event files.. expressions could be built
in same way as we have events now

> > from which point we could point and configure events we need
> 
> If you want full flexibility you can use your perf stat report
> approach, or what most people do is to just run a script/spreadsheet
> over the the -x; output. This all continues to work.
> 
> This is just a minimum approach to provide some convenience
> integrated with the event list to provide something similar
> as the built in expressions in stat-shadow. 
> 
> It's not trying to build the great perf scripting language.

yea I understand that but can't ack that based on the points
I descibed in my other email

jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-09 18:37           ` Jiri Olsa
@ 2017-02-09 18:59             ` Andi Kleen
  2017-02-10  8:22               ` Jiri Olsa
  0 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2017-02-09 18:59 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Andi Kleen, Andi Kleen, acme, jolsa, linux-kernel

On Thu, Feb 09, 2017 at 07:37:55PM +0100, Jiri Olsa wrote:
> > The last time I proposed separate files Ingo vetoed it.
> > He wanted everything built in.
> 
> sure, he veto it for event files.. expressions could be built
> in same way as we have events now

That's exactly what I implemented. The expression is part
of the JSON file, which seems like the logical place.

You just want it in a separate file in the source?


> 
> > > from which point we could point and configure events we need
> > 
> > If you want full flexibility you can use your perf stat report
> > approach, or what most people do is to just run a script/spreadsheet
> > over the the -x; output. This all continues to work.
> > 
> > This is just a minimum approach to provide some convenience
> > integrated with the event list to provide something similar
> > as the built in expressions in stat-shadow. 
> > 
> > It's not trying to build the great perf scripting language.
> 
> yea I understand that but can't ack that based on the points
> I descibed in my other email

So what are the crucial points that prevent you?

- You want better column descriptions? 

I suppose could add another field for this.

- You want multiple expressions per event 
(even though they are not needed today)?

It could be implemented, but seems like unnecessary
complexity and overengineering to me at this point.
If nobody ever uses it would it be time spent well?
If someone really uses it they could add the support at that
time.

- You want automatic group creation?

It has nasty corner cases.
It would be possible to build something that works
for simple cases.

Anything I missed?

-Andi

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [tip:perf/core] perf jevents: Parse eventcode as number
  2017-01-28  2:03 ` [PATCH 01/10] perf, tools: Parse eventcode as number in jevents Andi Kleen
@ 2017-02-10  7:41   ` tip-bot for Andi Kleen
  0 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Andi Kleen @ 2017-02-10  7:41 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: jolsa, mingo, acme, hpa, tglx, linux-kernel, ak

Commit-ID:  d581141970ef3965c1624960fa928d765afd8a3e
Gitweb:     http://git.kernel.org/tip/d581141970ef3965c1624960fa928d765afd8a3e
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Jan 2017 18:03:36 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Feb 2017 08:55:02 -0300

perf jevents: Parse eventcode as number

The next patch needs to modify event code. Previously eventcode was just
passed through as a string. Now parse it as a number.

v2: Don't special case 0

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170128020345.19007-2-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/pmu-events/jevents.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 41611d7..551377b 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -135,7 +135,6 @@ static struct field {
 	const char *field;
 	const char *kernel;
 } fields[] = {
-	{ "EventCode",	"event=" },
 	{ "UMask",	"umask=" },
 	{ "CounterMask", "cmask=" },
 	{ "Invert",	"inv=" },
@@ -343,6 +342,7 @@ int json_events(const char *fn,
 	jsmntok_t *tokens, *tok;
 	int i, j, len;
 	char *map;
+	char buf[128];
 
 	if (!fn)
 		return -ENOENT;
@@ -356,6 +356,7 @@ int json_events(const char *fn,
 		char *event = NULL, *desc = NULL, *name = NULL;
 		char *long_desc = NULL;
 		char *extra_desc = NULL;
+		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
 		jsmntok_t *precise = NULL;
@@ -376,6 +377,11 @@ int json_events(const char *fn,
 			nz = !json_streq(map, val, "0");
 			if (match_field(map, field, nz, &event, val)) {
 				/* ok */
+			} else if (json_streq(map, field, "EventCode")) {
+				char *code = NULL;
+				addfield(map, &code, "", "", val);
+				eventcode |= strtoul(code, NULL, 0);
+				free(code);
 			} else if (json_streq(map, field, "EventName")) {
 				addfield(map, &name, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
@@ -410,6 +416,8 @@ int json_events(const char *fn,
 				addfield(map, &extra_desc, " ",
 						"(Precise event)", NULL);
 		}
+		snprintf(buf, sizeof buf, "event=%#llx", eventcode);
+		addfield(map, &event, ",", buf, NULL);
 		if (desc && extra_desc)
 			addfield(map, &desc, " ", extra_desc, NULL);
 		if (long_desc && extra_desc)

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [tip:perf/core] perf jevents: Add support for parsing uncore json files
  2017-01-28  2:03 ` [PATCH 02/10] perf, tools: Add support for parsing uncore json files Andi Kleen
@ 2017-02-10  7:41   ` tip-bot for Andi Kleen
  0 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Andi Kleen @ 2017-02-10  7:41 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, acme, jolsa, ak, mingo, hpa, tglx

Commit-ID:  fedb2b518239cbc00abcf0d200e0be8436251c11
Gitweb:     http://git.kernel.org/tip/fedb2b518239cbc00abcf0d200e0be8436251c11
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Jan 2017 18:03:37 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Feb 2017 08:55:03 -0300

perf jevents: Add support for parsing uncore json files

Handle the "Unit" field, which is needed to find the right PMU for an
event. We call it "pmu" and convert it to the perf pmu name with an
uncore prefix.

Handle the "ExtSel" field, which just extends the event mask with an
additional bit.

Handle the "Filter" field which adds parameters to the main event
to configure filtering.

Handle the "Unit" field which declares the unit the values should be
scaled too (similar to what the kernel exports)

Set up the "perpkg" field for uncore events so that perf knows they are
per package (similar to what the kernel exports)

Then output the fields into the pmu-events data structures which are
compiled into perf.

Filter out zero fields, except for the event itself.

v2: Fix compilation. Add uncore_ prefix at pre-processing time.
    Move eventcode change to separate patch.

v3: Remove extra __maybe_unused

v4: dont duplicate aliases for cpu pmu events

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170128020345.19007-3-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/pmu-events/jevents.c    | 74 +++++++++++++++++++++++++++++++++++---
 tools/perf/pmu-events/jevents.h    |  4 ++-
 tools/perf/pmu-events/pmu-events.h |  3 ++
 tools/perf/util/pmu.c              | 30 +++++++++++-----
 4 files changed, 98 insertions(+), 13 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 551377b..eed0934 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -188,6 +188,27 @@ static struct msrmap *lookup_msr(char *map, jsmntok_t *val)
 	return NULL;
 }
 
+static struct map {
+	const char *json;
+	const char *perf;
+} unit_to_pmu[] = {
+	{ "CBO", "uncore_cbox" },
+	{ "QPI LL", "uncore_qpi" },
+	{ "SBO", "uncore_sbox" },
+	{}
+};
+
+static const char *field_to_perf(struct map *table, char *map, jsmntok_t *val)
+{
+	int i;
+
+	for (i = 0; table[i].json; i++) {
+		if (json_streq(map, val, table[i].json))
+			return table[i].perf;
+	}
+	return NULL;
+}
+
 #define EXPECT(e, t, m) do { if (!(e)) {			\
 	jsmntok_t *loc = (t);					\
 	if (!(t)->start && (t) > tokens)			\
@@ -269,7 +290,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 *desc, char *long_desc,
+				    char *pmu, char *unit, char *perpkg)
 {
 	struct perf_entry_data *pd = data;
 	FILE *outfp = pd->outfp;
@@ -287,7 +309,12 @@ static int print_events_table_entry(void *data, char *name, char *event,
 	fprintf(outfp, "\t.topic = \"%s\",\n", topic);
 	if (long_desc && long_desc[0])
 		fprintf(outfp, "\t.long_desc = \"%s\",\n", long_desc);
-
+	if (pmu)
+		fprintf(outfp, "\t.pmu = \"%s\",\n", pmu);
+	if (unit)
+		fprintf(outfp, "\t.unit = \"%s\",\n", unit);
+	if (perpkg)
+		fprintf(outfp, "\t.perpkg = \"%s\",\n", perpkg);
 	fprintf(outfp, "},\n");
 
 	return 0;
@@ -334,7 +361,8 @@ static char *real_event(const char *name, char *event)
 /* Call func with each event in the json file */
 int json_events(const char *fn,
 	  int (*func)(void *data, char *name, char *event, char *desc,
-		      char *long_desc),
+		      char *long_desc,
+		      char *pmu, char *unit, char *perpkg),
 	  void *data)
 {
 	int err = -EIO;
@@ -356,6 +384,10 @@ int json_events(const char *fn,
 		char *event = NULL, *desc = NULL, *name = NULL;
 		char *long_desc = NULL;
 		char *extra_desc = NULL;
+		char *pmu = NULL;
+		char *filter = NULL;
+		char *perpkg = NULL;
+		char *unit = NULL;
 		unsigned long long eventcode = 0;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
@@ -382,6 +414,11 @@ int json_events(const char *fn,
 				addfield(map, &code, "", "", val);
 				eventcode |= strtoul(code, NULL, 0);
 				free(code);
+			} else if (json_streq(map, field, "ExtSel")) {
+				char *code = NULL;
+				addfield(map, &code, "", "", val);
+				eventcode |= strtoul(code, NULL, 0) << 21;
+				free(code);
 			} else if (json_streq(map, field, "EventName")) {
 				addfield(map, &name, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
@@ -405,6 +442,28 @@ int json_events(const char *fn,
 				addfield(map, &extra_desc, ". ",
 					" Supports address when precise",
 					NULL);
+			} else if (json_streq(map, field, "Unit")) {
+				const char *ppmu;
+				char *s;
+
+				ppmu = field_to_perf(unit_to_pmu, map, val);
+				if (ppmu) {
+					pmu = strdup(ppmu);
+				} else {
+					if (!pmu)
+						pmu = strdup("uncore_");
+					addfield(map, &pmu, "", "", val);
+					for (s = pmu; *s; s++)
+						*s = tolower(*s);
+				}
+				addfield(map, &desc, ". ", "Unit: ", NULL);
+				addfield(map, &desc, "", pmu, NULL);
+			} else if (json_streq(map, field, "Filter")) {
+				addfield(map, &filter, "", "", val);
+			} else if (json_streq(map, field, "ScaleUnit")) {
+				addfield(map, &unit, "", "", val);
+			} else if (json_streq(map, field, "PerPkg")) {
+				addfield(map, &perpkg, "", "", val);
 			}
 			/* ignore unknown fields */
 		}
@@ -422,16 +481,23 @@ int json_events(const char *fn,
 			addfield(map, &desc, " ", extra_desc, NULL);
 		if (long_desc && extra_desc)
 			addfield(map, &long_desc, " ", extra_desc, NULL);
+		if (filter)
+			addfield(map, &event, ",", filter, NULL);
 		if (msr != NULL)
 			addfield(map, &event, ",", msr->pname, msrval);
 		fixname(name);
 
-		err = func(data, name, real_event(name, event), desc, long_desc);
+		err = func(data, name, real_event(name, event), desc, long_desc,
+				pmu, unit, perpkg);
 		free(event);
 		free(desc);
 		free(name);
 		free(long_desc);
 		free(extra_desc);
+		free(pmu);
+		free(filter);
+		free(perpkg);
+		free(unit);
 		if (err)
 			break;
 		tok += j;
diff --git a/tools/perf/pmu-events/jevents.h b/tools/perf/pmu-events/jevents.h
index b0eb274..71e13de 100644
--- a/tools/perf/pmu-events/jevents.h
+++ b/tools/perf/pmu-events/jevents.h
@@ -3,7 +3,9 @@
 
 int json_events(const char *fn,
 		int (*func)(void *data, char *name, char *event, char *desc,
-				char *long_desc),
+				char *long_desc,
+				char *pmu,
+				char *unit, char *perpkg),
 		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 2eaef59..c669a3c 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -10,6 +10,9 @@ struct pmu_event {
 	const char *desc;
 	const char *topic;
 	const char *long_desc;
+	const char *pmu;
+	const char *unit;
+	const char *perpkg;
 };
 
 /*
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 78b1610..6dc3cc0 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -229,11 +229,13 @@ 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, char *val, char *long_desc,
-				 char *topic)
+				 char *desc, char *val,
+				 char *long_desc, char *topic,
+				 char *unit, char *perpkg)
 {
 	struct perf_pmu_alias *alias;
 	int ret;
+	int num;
 
 	alias = malloc(sizeof(*alias));
 	if (!alias)
@@ -267,7 +269,12 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 	alias->long_desc = long_desc ? strdup(long_desc) :
 				desc ? strdup(desc) : NULL;
 	alias->topic = topic ? strdup(topic) : NULL;
-
+	if (unit) {
+		if (convert_scale(unit, &unit, &alias->scale) < 0)
+			return -1;
+		snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
+	}
+	alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
 	list_add_tail(&alias->list, list);
 
 	return 0;
@@ -284,7 +291,8 @@ 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);
+	return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL, NULL,
+				     NULL);
 }
 
 static inline bool pmu_alias_info_file(char *name)
@@ -504,7 +512,7 @@ char * __weak get_cpuid_str(void)
  * 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)
+static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
 {
 	int i;
 	struct pmu_events_map *map;
@@ -540,14 +548,21 @@ static void pmu_add_cpu_aliases(struct list_head *head)
 	 */
 	i = 0;
 	while (1) {
+		const char *pname;
+
 		pe = &map->table[i++];
 		if (!pe->name)
 			break;
 
+		pname = pe->pmu ? pe->pmu : "cpu";
+		if (strncmp(pname, name, strlen(pname)))
+			continue;
+
 		/* need type casts to override 'const' */
 		__perf_pmu__new_alias(head, NULL, (char *)pe->name,
 				(char *)pe->desc, (char *)pe->event,
-				(char *)pe->long_desc, (char *)pe->topic);
+				(char *)pe->long_desc, (char *)pe->topic,
+				(char *)pe->unit, (char *)pe->perpkg);
 	}
 
 out:
@@ -578,8 +593,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_aliases(name, &aliases))
 		return NULL;
 
-	if (!strcmp(name, "cpu"))
-		pmu_add_cpu_aliases(&aliases);
+	pmu_add_cpu_aliases(&aliases, name);
 
 	if (pmu_type(name, &type))
 		return NULL;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [tip:perf/core] perf pmu: Support per pmu json aliases
  2017-01-28  2:03 ` [PATCH 03/10] perf, tools: Support per pmu json aliases Andi Kleen
@ 2017-02-10  7:42   ` tip-bot for Andi Kleen
  0 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Andi Kleen @ 2017-02-10  7:42 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: tglx, jolsa, mingo, ak, hpa, acme, linux-kernel

Commit-ID:  15b22ed369aa23ef4d083ffb9621650c353d3ddd
Gitweb:     http://git.kernel.org/tip/15b22ed369aa23ef4d083ffb9621650c353d3ddd
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Jan 2017 18:03:38 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Feb 2017 08:55:03 -0300

perf pmu: Support per pmu json aliases

Add support for registering json aliases per PMU. Any alias with an unit
matching the prefix is registered to the PMU.  Uncore has multiple
instances of most units, so all these aliases get registered for each
individual PMU (this is important later to run the event on every
instance of the PMU).

To avoid printing the events multiple times in perf list filter out
duplicated events during printing.

v2: Rely on uncore_ prefix already in unit
v3: Document why calls were reordered

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170128020345.19007-4-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6dc3cc0..8e9d00f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -590,14 +590,16 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	if (pmu_format(name, &format))
 		return NULL;
 
-	if (pmu_aliases(name, &aliases))
+	/*
+	 * Check the type first to avoid unnecessary work.
+	 */
+	if (pmu_type(name, &type))
 		return NULL;
 
-	pmu_add_cpu_aliases(&aliases, name);
-
-	if (pmu_type(name, &type))
+	if (pmu_aliases(name, &aliases))
 		return NULL;
 
+	pmu_add_cpu_aliases(&aliases, name);
 	pmu = zalloc(sizeof(*pmu));
 	if (!pmu)
 		return NULL;
@@ -1195,6 +1197,9 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 	len = j;
 	qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
 	for (j = 0; j < len; j++) {
+		/* Skip duplicates */
+		if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name))
+			continue;
 		if (name_only) {
 			printf("%s ", aliases[j].name);
 			continue;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [tip:perf/core] perf pmu: Support event aliases for non cpu// pmus
  2017-01-28  2:03 ` [PATCH 04/10] perf, tools: Support event aliases for non cpu// pmus Andi Kleen
@ 2017-02-10  7:42   ` tip-bot for Andi Kleen
  0 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Andi Kleen @ 2017-02-10  7:42 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: hpa, mingo, ak, tglx, linux-kernel, acme, jolsa

Commit-ID:  231bb2aa32498cbebef1306889a02114e9dfc934
Gitweb:     http://git.kernel.org/tip/231bb2aa32498cbebef1306889a02114e9dfc934
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Jan 2017 18:03:39 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Feb 2017 08:55:04 -0300

perf pmu: Support event aliases for non cpu// pmus

The code for handling pmu aliases without specifying the PMU hardcoded
only supported the cpu PMU.

This patch extends it to work for all PMUs. We always duplicate the
event for all PMUs that have an matching alias.  This allows to
automatically expand an alias for all instances of a PMU (so for example
you can monitor all cache boxes with a single event)

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170128020345.19007-5-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 46 ++++++++++++++++++++++++------------------
 tools/perf/util/parse-events.y | 32 ++++++++++++++++++++++-------
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3c876b8..6dbcba7 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1504,35 +1504,41 @@ static void perf_pmu__parse_init(void)
 	struct perf_pmu_alias *alias;
 	int len = 0;
 
-	pmu = perf_pmu__find("cpu");
-	if ((pmu == NULL) || list_empty(&pmu->aliases)) {
+	pmu = NULL;
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (strchr(alias->name, '-'))
+				len++;
+			len++;
+		}
+	}
+
+	if (len == 0) {
 		perf_pmu_events_list_num = -1;
 		return;
 	}
-	list_for_each_entry(alias, &pmu->aliases, list) {
-		if (strchr(alias->name, '-'))
-			len++;
-		len++;
-	}
 	perf_pmu_events_list = malloc(sizeof(struct perf_pmu_event_symbol) * len);
 	if (!perf_pmu_events_list)
 		return;
 	perf_pmu_events_list_num = len;
 
 	len = 0;
-	list_for_each_entry(alias, &pmu->aliases, list) {
-		struct perf_pmu_event_symbol *p = perf_pmu_events_list + len;
-		char *tmp = strchr(alias->name, '-');
-
-		if (tmp != NULL) {
-			SET_SYMBOL(strndup(alias->name, tmp - alias->name),
-					PMU_EVENT_SYMBOL_PREFIX);
-			p++;
-			SET_SYMBOL(strdup(++tmp), PMU_EVENT_SYMBOL_SUFFIX);
-			len += 2;
-		} else {
-			SET_SYMBOL(strdup(alias->name), PMU_EVENT_SYMBOL);
-			len++;
+	pmu = NULL;
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			struct perf_pmu_event_symbol *p = perf_pmu_events_list + len;
+			char *tmp = strchr(alias->name, '-');
+
+			if (tmp != NULL) {
+				SET_SYMBOL(strndup(alias->name, tmp - alias->name),
+						PMU_EVENT_SYMBOL_PREFIX);
+				p++;
+				SET_SYMBOL(strdup(++tmp), PMU_EVENT_SYMBOL_SUFFIX);
+				len += 2;
+			} else {
+				SET_SYMBOL(strdup(alias->name), PMU_EVENT_SYMBOL);
+				len++;
+			}
 		}
 	}
 	qsort(perf_pmu_events_list, len,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 879115f..f3b5ec9 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -12,6 +12,7 @@
 #include <linux/list.h>
 #include <linux/types.h>
 #include "util.h"
+#include "pmu.h"
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
@@ -236,15 +237,32 @@ PE_KERNEL_PMU_EVENT sep_dc
 	struct list_head *head;
 	struct parse_events_term *term;
 	struct list_head *list;
+	struct perf_pmu *pmu = NULL;
+	int ok = 0;
 
-	ALLOC_LIST(head);
-	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
-					$1, 1, &@1, NULL));
-	list_add_tail(&term->list, head);
-
+	/* Add it for all PMUs that support the alias */
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(data, list, "cpu", head));
-	parse_events_terms__delete(head);
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		struct perf_pmu_alias *alias;
+
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (!strcasecmp(alias->name, $1)) {
+				ALLOC_LIST(head);
+				ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, 1, &@1, NULL));
+				list_add_tail(&term->list, head);
+
+				if (!parse_events_add_pmu(data, list,
+						  pmu->name, head)) {
+					ok++;
+				}
+
+				parse_events_terms__delete(head);
+			}
+		}
+	}
+	if (!ok)
+		YYABORT;
 	$$ = list;
 }
 |

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [tip:perf/core] perf list: Add debug support for outputing alias string
  2017-01-28  2:03 ` [PATCH 05/10] perf, tools: Add debug support for outputing alias string Andi Kleen
@ 2017-02-10  7:43   ` tip-bot for Andi Kleen
  0 siblings, 0 replies; 28+ messages in thread
From: tip-bot for Andi Kleen @ 2017-02-10  7:43 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, acme, mingo, tglx, jolsa, ak, hpa

Commit-ID:  f23610245c1aa0e912476e642bd5107d04122230
Gitweb:     http://git.kernel.org/tip/f23610245c1aa0e912476e642bd5107d04122230
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Jan 2017 18:03:40 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 8 Feb 2017 08:55:04 -0300

perf list: Add debug support for outputing alias string

For debugging and testing it is useful to see the converted alias
string. Add support to perf stat/record and perf list to print the alias
conversion. The text string is saved in the alias structure.  For perf
stat/record it is folded into the normal -v. For perf list -v was taken,
so we use --debug.

Before:

% perf list
...
cache:
  l1d.replacement
       [L1D data line replacements]
  l1d_pend_miss.fb_full
       [Cycles a demand request was blocked due to Fill Buffers inavailability]

After

% perf list --debug
...
cache:
  l1d.replacement
       [L1D data line replacements]
        cpu/umask=0x1,period=2000003,event=0x51/
  l1d_pend_miss.fb_full
       [Cycles a demand request was blocked due to Fill Buffers inavailability]
        cpu/umask=0x2,period=2000003,cmask=1,event=0x48/

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170128020345.19007-6-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-list.c      | 3 +++
 tools/perf/util/parse-events.y | 3 +++
 tools/perf/util/pmu.c          | 8 ++++++++
 tools/perf/util/pmu.h          | 1 +
 4 files changed, 15 insertions(+)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index ba9322f..3b9d98b 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -14,6 +14,7 @@
 #include "util/parse-events.h"
 #include "util/cache.h"
 #include "util/pmu.h"
+#include "util/debug.h"
 #include <subcmd/parse-options.h>
 
 static bool desc_flag = true;
@@ -29,6 +30,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 			    "Print extra event descriptions. --no-desc to not print."),
 		OPT_BOOLEAN('v', "long-desc", &long_desc_flag,
 			    "Print longer event descriptions."),
+		OPT_INCR(0, "debug", &verbose,
+			     "Enable debugging output"),
 		OPT_END()
 	};
 	const char * const list_usage[] = {
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index f3b5ec9..3a51963 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -13,6 +13,7 @@
 #include <linux/types.h>
 #include "util.h"
 #include "pmu.h"
+#include "debug.h"
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
@@ -254,6 +255,8 @@ PE_KERNEL_PMU_EVENT sep_dc
 
 				if (!parse_events_add_pmu(data, list,
 						  pmu->name, head)) {
+					pr_debug("%s -> %s/%s/\n", $1,
+						 pmu->name, alias->str);
 					ok++;
 				}
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8e9d00f..82a654de 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -275,6 +275,8 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 		snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
 	}
 	alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
+	alias->str = strdup(val);
+
 	list_add_tail(&alias->list, list);
 
 	return 0;
@@ -1087,6 +1089,8 @@ struct sevent {
 	char *name;
 	char *desc;
 	char *topic;
+	char *str;
+	char *pmu;
 };
 
 static int cmp_sevent(const void *a, const void *b)
@@ -1183,6 +1187,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 			aliases[j].desc = long_desc ? alias->long_desc :
 						alias->desc;
 			aliases[j].topic = alias->topic;
+			aliases[j].str = alias->str;
+			aliases[j].pmu = pmu->name;
 			j++;
 		}
 		if (pmu->selectable &&
@@ -1217,6 +1223,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 			printf("%*s", 8, "[");
 			wordwrap(aliases[j].desc, 8, columns, 0);
 			printf("]\n");
+			if (verbose)
+				printf("%*s%s/%s/\n", 8, "", aliases[j].pmu, aliases[j].str);
 		} else
 			printf("  %-50s [Kernel PMU event]\n", aliases[j].name);
 		printed++;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 2571203..00852dd 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -43,6 +43,7 @@ struct perf_pmu_alias {
 	char *desc;
 	char *long_desc;
 	char *topic;
+	char *str;
 	struct list_head terms; /* HEAD struct parse_events_term -> list */
 	struct list_head list;  /* ELEM */
 	char unit[UNIT_MAX_LEN+1];

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric
  2017-02-09 18:59             ` Andi Kleen
@ 2017-02-10  8:22               ` Jiri Olsa
  0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2017-02-10  8:22 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andi Kleen, acme, jolsa, linux-kernel

On Thu, Feb 09, 2017 at 10:59:43AM -0800, Andi Kleen wrote:
> On Thu, Feb 09, 2017 at 07:37:55PM +0100, Jiri Olsa wrote:
> > > The last time I proposed separate files Ingo vetoed it.
> > > He wanted everything built in.
> > 
> > sure, he veto it for event files.. expressions could be built
> > in same way as we have events now
> 
> That's exactly what I implemented. The expression is part
> of the JSON file, which seems like the logical place.
> 
> You just want it in a separate file in the source?
> 
> 
> > 
> > > > from which point we could point and configure events we need
> > > 
> > > If you want full flexibility you can use your perf stat report
> > > approach, or what most people do is to just run a script/spreadsheet
> > > over the the -x; output. This all continues to work.
> > > 
> > > This is just a minimum approach to provide some convenience
> > > integrated with the event list to provide something similar
> > > as the built in expressions in stat-shadow. 
> > > 
> > > It's not trying to build the great perf scripting language.
> > 
> > yea I understand that but can't ack that based on the points
> > I descibed in my other email
> 
> So what are the crucial points that prevent you?
> 
> - You want better column descriptions? 
> 
> I suppose could add another field for this.
> 
> - You want multiple expressions per event 
> (even though they are not needed today)?
> 
> It could be implemented, but seems like unnecessary
> complexity and overengineering to me at this point.
> If nobody ever uses it would it be time spent well?
> If someone really uses it they could add the support at that
> time.

ok, I checked optimization doc and you might be right about this one,
I couldn't find an example that'd prove you wrong ;-)

however I dont think expression should be annonymous number
tied to an event. For example there's this one:

  B.7.9.2  
  Fast Synchronization Penalty
  50. Locked Operations Impact: (L1D_CACHE_LOCK_DURATION + 20 * L1D_CACHE_LOCK.MESI) / CPU_CLK_UNHALTED.CORE * 100

  Fast synchronization is frequently implemented usin
  g locked memory accesses. A high value for Locked 
  Operations Impact indicates that locked operations us
  ed in the workload have high penalty. The latency 
  of a locked operation depends on the location of the 
  data: L1 data cache, L2 cache, other core cache or 
  memory.

There's a name and description.

which event will pick this expression?  L1D_CACHE_LOCK_DURATION or L1D_CACHE_LOCK?

How about we add:

"MetricExpr":      "(L1D_CACHE_LOCK_DURATION + 20 * L1D_CACHE_LOCK.MESI) / CPU_CLK_UNHALTED.CORE * 100"
"MetricExprName":  "Fast Synchronization Penalty"
"MetricExprDoc":   "Fast synchronization is frequently implemented... "

then it could be part of the event record and we can display it
like use the name in column and doc in the perf list

We could append number or something (MetricExpr2..) if there'll
be multiple expressions in some case.. or something like that

> 
> - You want automatic group creation?
> 
> It has nasty corner cases.
> It would be possible to build something that works
> for simple cases.

that would be nice.. when user says perf stat -e 'Fast Synchronization Penalty' ...
it'd create necessary events for him.. or some shorted name probably

jirka

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-02-10  8:43 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-28  2:03 Support Intel uncore event lists Andi Kleen
2017-01-28  2:03 ` [PATCH 01/10] perf, tools: Parse eventcode as number in jevents Andi Kleen
2017-02-10  7:41   ` [tip:perf/core] perf jevents: Parse eventcode as number tip-bot for Andi Kleen
2017-01-28  2:03 ` [PATCH 02/10] perf, tools: Add support for parsing uncore json files Andi Kleen
2017-02-10  7:41   ` [tip:perf/core] perf jevents: " tip-bot for Andi Kleen
2017-01-28  2:03 ` [PATCH 03/10] perf, tools: Support per pmu json aliases Andi Kleen
2017-02-10  7:42   ` [tip:perf/core] perf pmu: " tip-bot for Andi Kleen
2017-01-28  2:03 ` [PATCH 04/10] perf, tools: Support event aliases for non cpu// pmus Andi Kleen
2017-02-10  7:42   ` [tip:perf/core] perf pmu: " tip-bot for Andi Kleen
2017-01-28  2:03 ` [PATCH 05/10] perf, tools: Add debug support for outputing alias string Andi Kleen
2017-02-10  7:43   ` [tip:perf/core] perf list: " tip-bot for Andi Kleen
2017-01-28  2:03 ` [PATCH 06/10] perf, tools: Collapse identically named events in perf stat Andi Kleen
2017-02-08 11:31   ` Jiri Olsa
2017-01-28  2:03 ` [PATCH 07/10] perf, tools: Expand PMU events by prefix match Andi Kleen
2017-02-08 11:30   ` Jiri Olsa
2017-01-28  2:03 ` [PATCH 08/10] perf, tools: Add a simple expression parser for JSON Andi Kleen
2017-02-08 11:31   ` Jiri Olsa
2017-01-28  2:03 ` [PATCH 09/10] perf, tools: Support MetricExpr header in JSON event list Andi Kleen
2017-02-08 11:31   ` Jiri Olsa
2017-01-28  2:03 ` [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric Andi Kleen
2017-02-08 11:31   ` Jiri Olsa
2017-02-08 21:51     ` Andi Kleen
2017-02-09 11:39       ` Jiri Olsa
2017-02-09 17:00         ` Andi Kleen
2017-02-09 18:37           ` Jiri Olsa
2017-02-09 18:59             ` Andi Kleen
2017-02-10  8:22               ` Jiri Olsa
2017-02-09 11:42       ` Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.