Skip to contentSkip to conversation

perf: Additional improvements for json event lists

PATCH In review Andi Kleen 3 participants 6 messages May 28, 2015 perf events

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

First support PublicDescription again using perf list -v, as suggested by Jiri. Then I fixed the PERF_CPUID patch; sent an outdated broken version earlier. Finally this fixes the Makefile dependencies for the JSON file to force a rebuild when anything changes in JSON.

All on top of Sukadev's tree and my earlier group patch.

-Andi

author
#1 lore raw

PATCH 1/3 perf, tools: Add override support for event list CPUID

From: Andi Kleen <[email protected]>

Add a PERF_CPUID variable to override the CPUID of the current CPU. This is useful for testing, so that all event lists can be tested on a single system.

v2: Fix double free in earlier version.

Signed-off-by Andi Kleen <[email protected]>
---
 tools/perf/util/pmu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/util/pmu.c +5 -1
index ba50300..59e0f4e 100644
@@ -474,7 +474,11 @@ static int pmu_add_cpu_aliases(void *data)
 	struct pmu_event *pe;
 	char *cpuid;
 
-	cpuid = get_cpuid_str();
+	cpuid = getenv("PERF_CPUID");
+	if (cpuid)
+		cpuid = strdup(cpuid);
+	if (!cpuid)
+		cpuid = get_cpuid_str();
 	if (!cpuid)
 		return 0;
Signature · 1 line
2.1.0
author
#2 lore raw

PATCH 2/3 perf, tools: Support long descriptions with perf list -v

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

Previously we were dropping the useful longer descriptions that some events have in the event list completely. This patch makes them appear with perf list.

Old perf list:

baclears:
baclears.all
[Counts the number of baclears]

vs new:

perf list -v:
...
baclears:
baclears.all
[The BACLEARS event counts the number of times the front end is resteered, mainly when
the Branch Prediction Unit cannot provide a correct prediction and this is corrected by the Branch Address Calculator at the front end. The BACLEARS.ANY event counts the number of baclears for any type of branch]

Signed-off-by Andi Kleen <[email protected]>
---
 tools/perf/builtin-list.c          |  9 ++++++---
 tools/perf/pmu-events/jevents.c    | 29 ++++++++++++++++++++---------
 tools/perf/pmu-events/jevents.h    |  2 +-
 tools/perf/pmu-events/pmu-events.h |  1 +
 tools/perf/util/parse-events.c     |  4 ++--
 tools/perf/util/parse-events.h     |  2 +-
 tools/perf/util/pmu.c              | 17 ++++++++++++-----
 tools/perf/util/pmu.h              |  4 +++-
 8 files changed, 46 insertions(+), 22 deletions(-)
tools/perf/builtin-list.c +6 -3
index 3f058f7..c912368 100644
@@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int i;
 	bool raw_dump = false;
+	bool long_desc_flag = false;
 	struct option list_options[] = {
 		OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
 		OPT_BOOLEAN('d', "desc", &desc_flag,
 			    "Print extra event descriptions. --no-desc to not print."),
+		OPT_BOOLEAN('v', "long-desc", &long_desc_flag,
+			    "Print longer event descriptions."),
 		OPT_END()
 	};
 	const char * const list_usage[] = {

@@ -44,7 +47,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 		printf("\nList of pre-defined events (to be used in -e):\n\n");
 
 	if (argc == 0) {
-		print_events(NULL, raw_dump, !desc_flag);
+		print_events(NULL, raw_dump, !desc_flag, long_desc_flag);
 		return 0;
 	}
 

@@ -63,13 +66,13 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 			 strcmp(argv[i], "hwcache") == 0)
 			print_hwcache_events(NULL, raw_dump);
 		else if (strcmp(argv[i], "pmu") == 0)
-			print_pmu_events(NULL, raw_dump, !desc_flag);
+			print_pmu_events(NULL, raw_dump, !desc_flag, long_desc_flag);
 		else {
 			char *sep = strchr(argv[i], ':'), *s;
 			int sep_idx;
 
 			if (sep == NULL) {
-				print_events(argv[i], raw_dump, !desc_flag);
+				print_events(argv[i], raw_dump, !desc_flag, long_desc_flag);
 				continue;
 			}
 			sep_idx = sep - argv[i];
tools/perf/pmu-events/jevents.c +20 -9
index 43651cc..da9408c 100644
@@ -202,7 +202,7 @@ 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 *desc, char *long_desc)
 {
 	FILE *outfp = data;
 	/*

@@ -214,7 +214,8 @@ static int print_events_table_entry(void *data, char *name, char *event,
 	fprintf(outfp, "\t.name = \"%s\",\n", name);
 	fprintf(outfp, "\t.event = \"%s\",\n", event);
 	fprintf(outfp, "\t.desc = \"%s\",\n", desc);
-
+	if (long_desc && long_desc[0])
+		fprintf(outfp, "\t.long_desc = \"%s\"", long_desc);
 	fprintf(outfp, "},\n");
 
 	return 0;

@@ -234,7 +235,7 @@ static void print_events_table_suffix(FILE *outfp)
 
 /* 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),
+	  int (*func)(void *data, char *name, char *event, char *desc, char *long_desc),
 	  void *data)
 {
 	int err = -EIO;

@@ -252,7 +253,8 @@ int json_events(const char *fn,
 	EXPECT(tokens->type == JSMN_ARRAY, tokens, "expected top level array");
 	tok = tokens + 1;
 	for (i = 0; i < tokens->size; i++) {
-		char *event = NULL, *desc = NULL, *name = NULL;
+		char *event = NULL, *desc = NULL, *name = NULL, *long_desc = NULL;
+		char *extra_desc = NULL;
 		struct msrmap *msr = NULL;
 		jsmntok_t *msrval = NULL;
 		jsmntok_t *precise = NULL;

@@ -278,6 +280,9 @@ int json_events(const char *fn,
 			} else if (json_streq(map, field, "BriefDescription")) {
 				addfield(map, &desc, "", "", val);
 				fixdesc(desc);
+			} else if (json_streq(map, field, "PublicDescription")) {
+				addfield(map, &long_desc, "", "", val);
+				fixdesc(long_desc);
 			} else if (json_streq(map, field, "PEBS") && nz) {
 				precise = val;
 			} else if (json_streq(map, field, "MSRIndex") && nz) {

@@ -286,10 +291,10 @@ int json_events(const char *fn,
 				msrval = val;
 			} else if (json_streq(map, field, "Errata") &&
 				   !json_streq(map, val, "null")) {
-				addfield(map, &desc, ". ",
+				addfield(map, &extra_desc, ". ",
 					" Spec update: ", val);
 			} else if (json_streq(map, field, "Data_LA") && nz) {
-				addfield(map, &desc, ". ",
+				addfield(map, &extra_desc, ". ",
 					" Supports address when precise",
 					NULL);
 			}

@@ -297,19 +302,25 @@ int json_events(const char *fn,
 		}
 		if (precise && !strstr(desc, "(Precise Event)")) {
 			if (json_streq(map, precise, "2"))
-				addfield(map, &desc, " ", "(Must be precise)",
+				addfield(map, &extra_desc, " ", "(Must be precise)",
 						NULL);
 			else
-				addfield(map, &desc, " ",
+				addfield(map, &extra_desc, " ",
 						"(Precise event)", NULL);
 		}
+		if (desc && extra_desc)
+			addfield(map, &desc, " ", extra_desc, NULL);
+		if (long_desc && extra_desc)
+			addfield(map, &long_desc, " ", extra_desc, NULL);
 		if (msr != NULL)
 			addfield(map, &event, ",", msr->pname, msrval);
 		fixname(name);
-		err = func(data, name, event, desc);
+		err = func(data, name, event, desc, long_desc);
 		free(event);
 		free(desc);
 		free(name);
+		free(extra_desc);
+		free(long_desc);
 		if (err)
 			break;
 		tok += j;
tools/perf/pmu-events/jevents.h +1 -1
index 996601f..2168ad4 100644
@@ -2,7 +2,7 @@
 #define JEVENTS_H 1
 
 int json_events(const char *fn,
-		int (*func)(void *data, char *name, char *event, char *desc),
+		int (*func)(void *data, char *name, char *event, char *desc, char *long_desc),
 		void *data);
 char *get_cpu_str(void);
tools/perf/pmu-events/pmu-events.h +1 -0
index 39fec04..711f049 100644
@@ -8,6 +8,7 @@ struct pmu_event {
 	const char *name;
 	const char *event;
 	const char *desc;
+	const char *long_desc;
 };
 
 /*
tools/perf/util/parse-events.c +2 -2
index c660061..a6bf781 100644
@@ -1419,7 +1419,7 @@ out_enomem:
 /*
  * Print the help text for the event symbols:
  */
-void print_events(const char *event_glob, bool name_only, bool quiet_flag)
+void print_events(const char *event_glob, bool name_only, bool quiet_flag, bool long_desc)
 {
 	print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
 			    event_symbols_hw, PERF_COUNT_HW_MAX, name_only);

@@ -1429,7 +1429,7 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag)
 
 	print_hwcache_events(event_glob, name_only);
 
-	print_pmu_events(event_glob, name_only, quiet_flag);
+	print_pmu_events(event_glob, name_only, quiet_flag, long_desc);
 
 	if (event_glob != NULL)
 		return;
tools/perf/util/parse-events.h +1 -1
index cdb100e..698cf45 100644
@@ -115,7 +115,7 @@ void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
 void parse_events_error(void *data, void *scanner, char const *msg);
 
-void print_events(const char *event_glob, bool name_only, bool quiet);
+void print_events(const char *event_glob, bool name_only, bool quiet, bool long_desc);
 
 struct event_symbol {
 	const char	*symbol;
tools/perf/util/pmu.c +12 -5
index 59e0f4e..d78b4ab 100644
@@ -201,7 +201,8 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias *alias,
 	return 0;
 }
 
-static int __perf_pmu__new_alias(struct list_head *list, char *name, char *dir, char *desc __maybe_unused, char *val)
+static int __perf_pmu__new_alias(struct list_head *list, char *name, char *dir, char *desc __maybe_unused, char *val,
+				 char *long_desc)
 {
 	struct perf_pmu_alias *alias;
 	int ret;

@@ -234,6 +235,9 @@ static int __perf_pmu__new_alias(struct list_head *list, char *name, char *dir,
 	}
 
 	alias->desc = desc ? strdup(desc) : NULL;
+	alias->long_desc = long_desc ?
+			strdup(long_desc) :
+			desc ? strdup(desc) : NULL;
 
 	list_add_tail(&alias->list, list);
 

@@ -250,7 +254,7 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
 		return -EINVAL;
 	buf[ret] = 0;
 
-	return __perf_pmu__new_alias(list, name, dir, NULL, buf);
+	return __perf_pmu__new_alias(list, name, dir, NULL, buf, NULL);
 }
 
 static inline bool pmu_alias_info_file(char *name)

@@ -503,7 +507,8 @@ static int pmu_add_cpu_aliases(void *data)
 
 		/* need type casts to override 'const' */
 		__perf_pmu__new_alias(head, (char *)pe->name, NULL,
-				(char *)pe->desc, (char *)pe->event);
+				(char *)pe->desc, (char *)pe->event,
+				(char *)pe->long_desc);
 	}
 
 	free(cpuid);

@@ -976,7 +981,8 @@ static void wordwrap(char *s, int start, int max, int corr)
 	}
 }
 
-void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag)
+void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
+		      bool long_desc)
 {
 	struct perf_pmu *pmu;
 	struct perf_pmu_alias *alias;

@@ -1023,7 +1029,8 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag)
 								  pmu, alias);
 			aliases[j].name = strdup(aliases[j].name);
 			/* failure harmless */
-			aliases[j].desc = alias->desc;
+			aliases[j].desc = long_desc ? alias->long_desc :
+						      alias->desc;
 			j++;
 		}
 		if (pmu->selectable) {
tools/perf/util/pmu.h +3 -1
index 3ca632b..e0f7ed3 100644
@@ -38,6 +38,7 @@ struct perf_pmu_info {
 struct perf_pmu_alias {
 	char *name;
 	char *desc;
+	char *long_desc;
 	struct list_head terms; /* HEAD struct parse_events_term -> list */
 	struct list_head list;  /* ELEM */
 	char unit[UNIT_MAX_LEN+1];

@@ -67,7 +68,8 @@ int perf_pmu__format_parse(char *dir, struct list_head *head);
 
 struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);
 
-void print_pmu_events(const char *event_glob, bool name_only, bool quiet);
+void print_pmu_events(const char *event_glob, bool name_only, bool quiet,
+			bool long_desc);
 bool pmu_have_event(const char *pname, const char *name);
 
 int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt,
Signature · 1 line
2.1.0
author
#3 lore raw

PATCH 3/3 perf, tools: Fix jevents dependencies

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

The "find" based dependencies for the JSON files didn't work me. I didn't get a rebuild when the JSON files changed. Change it to use wildcard. Also add a dependency for the jevents binary itself and for the mapfile.

Signed-off-by Andi Kleen <[email protected]>
---
 tools/perf/pmu-events/Build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/pmu-events/Build +4 -2
index 7e0c85c..e900ecc 100644
@@ -1,10 +1,12 @@
 jevents-y	+= json.o jsmn.o jevents.o
 pmu-events-y	+= pmu-events.o
-JSON		=  $(shell find pmu-events/arch/$(ARCH) -name '*.json')
 
 #
 # Locate/process JSON files in pmu-events/arch/
 # directory and create tables in pmu-events.c.
 #
-$(OUTPUT)pmu-events/pmu-events.c: $(JSON)
+$(OUTPUT)pmu-events/pmu-events.c: $(wildcard pmu-events/arch/$(ARCH)/*.json) \
+				  $(wildcard pmu-events/arch/$(ARCH)/*/*.json) \
+				  pmu-events/arch/$(ARCH)/mapfile.csv \
+				  $(JEVENTS)
 	$(Q)$(call echo-cmd,gen)$(JEVENTS) $(ARCH) pmu-events/arch $(OUTPUT)pmu-events/pmu-events.c
Signature · 1 line
2.1.0
#4 lore raw

Re: [PATCH 3/3] perf, tools: Fix jevents dependencies

Andi Kleen [[email protected]] wrote:

| From: Andi Kleen <[email protected]>
| 
| The "find" based dependencies for the JSON files didn't work me. I
| didn't get a rebuild when the JSON files changed.

Hmm, it works for me every time. I like the 'find' better because it is more compact and allows mulitple levels.

| Change it to
| use wildcard. Also add a dependency for the jevents binary itself
| and for the mapfile.

I was trying this to address Jiri's comment about being silent if some architectures don't have JSON files (and based on your comments above, added the depedency for mapfile and jevents)

tools/perf/pmu-events/Build +5 -3
index 7e0c85c..45b05a7 100644
@@ -1,10 +1,12 @@
 jevents-y      += json.o jsmn.o jevents.o
 pmu-events-y   += pmu-events.o
-JSON           =  $(shell find pmu-events/arch/$(ARCH) -name '*.json')
-
+EVDIR          = pmu-events/arch/$(ARCH)
+JSON           =  $(shell [ -d $(EVDIR) ] && \
+                       find $(EVDIR) -name '*.json' -o -name 'mapfile*')
+#
 #
 # Locate/process JSON files in pmu-events/arch/
 # directory and create tables in pmu-events.c.
 #
-$(OUTPUT)pmu-events/pmu-events.c: $(JSON)
+$(OUTPUT)pmu-events/pmu-events.c: $(JSON) $(JEVENTS)
        $(Q)$(call echo-cmd,gen)$(JEVENTS) $(ARCH) pmu-events/arch $(OUTPUT)pmu-events/pmu-events.c
#5 lore raw

Re: [PATCH 3/3] perf, tools: Fix jevents dependencies

Quoted 8 lines · Sukadev Bhattiprolu

Andi Kleen [[email protected]] wrote:

| From: Andi Kleen <[email protected]>
| 
| The "find" based dependencies for the JSON files didn't work me. I
| didn't get a rebuild when the JSON files changed.

Hmm, it works for me every time. I like the 'find' better because it is more compact and allows mulitple levels.

You're right. On rechecking it seems to work. Perhaps was some other problem earlier. So the patch can be dropped.

Quoted 8 lines
| Change it to
| use wildcard. Also add a dependency for the jevents binary itself
| and for the mapfile.

I was trying this to address Jiri's comment about being silent if some architectures don't have JSON files (and based on your comments above, added the depedency for mapfile and jevents)

Ok.

tools/perf/pmu-events/Build line 5

Quoted 13 lines
tools/perf/pmu-events/Build +3 -2
index 7e0c85c..45b05a7 100644
@@ -1,10 +1,12 @@
 jevents-y      += json.o jsmn.o jevents.o
 pmu-events-y   += pmu-events.o
-JSON           =  $(shell find pmu-events/arch/$(ARCH) -name '*.json')
-
+EVDIR          = pmu-events/arch/$(ARCH)
+JSON           =  $(shell [ -d $(EVDIR) ] && \
+                       find $(EVDIR) -name '*.json' -o -name 'mapfile*')

BTW should only process a single mapfile called mapfile.csv

Currently it processes editor backup files and similar too, which is not really intended.

-Andi