Skip to contentSkip to conversation

Revert "perf tools: Default to cpu// for events v5"

PATCH Superseded [email protected] 2 participants 4 messages Sep 1, 2014 – Sep 2, 2014 perf events

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/3 Revert "perf tools: Default to cpu// for events v5"

From: Kan Liang <[email protected]>

This reverts commit 50e200f07948 ("perf tools: Default to cpu// for events v5")

The fixup cannot handle the case that
new style format(which without //) mixed with
other different formats.

For example,
group events with new style format: {mem-stores,mem-loads} some hardware event + new style event: cycles,mem-loads Cache event + new style event: LLC-loads,mem-loads
Raw event + new style event:
cpu/event=0xc8,umask=0x08/,mem-loads
old style event and new stytle mixture: mem-stores,cpu/mem-loads/

---
 tools/perf/util/include/linux/string.h |  1 -
 tools/perf/util/parse-events.c         | 30 +-----------------------------
 tools/perf/util/string.c               | 24 ------------------------
 3 files changed, 1 insertion(+), 54 deletions(-)
tools/perf/util/include/linux/string.h +0 -1
index 97a8007..6f19c54 100644
@@ -1,4 +1,3 @@
 #include <string.h>
 
 void *memdup(const void *src, size_t len);
-int str_append(char **s, int *len, const char *a);
tools/perf/util/parse-events.c +1 -29
index 1e15df1..7a0aa75 100644
@@ -6,7 +6,7 @@
 #include "parse-options.h"
 #include "parse-events.h"
 #include "exec_cmd.h"
-#include "linux/string.h"
+#include "string.h"
 #include "symbol.h"
 #include "cache.h"
 #include "header.h"
@@ -852,32 +852,6 @@ int parse_events_name(struct list_head *list, char *name)
 	return 0;
 }
 
-static int parse_events__scanner(const char *str, void *data, int start_token);
-
-static int parse_events_fixup(int ret, const char *str, void *data,
-			      int start_token)
-{
-	char *o = strdup(str);
-	char *s = NULL;
-	char *t = o;
-	char *p;
-	int len = 0;
-
-	if (!o)
-		return ret;
-	while ((p = strsep(&t, ",")) != NULL) {
-		if (s)
-			str_append(&s, &len, ",");
-		str_append(&s, &len, "cpu/");
-		str_append(&s, &len, p);
-		str_append(&s, &len, "/");
-	}
-	free(o);
-	if (!s)
-		return -ENOMEM;
-	return parse_events__scanner(s, data, start_token);
-}
-
 static int parse_events__scanner(const char *str, void *data, int start_token)
 {
 	YY_BUFFER_STATE buffer;
@@ -898,8 +872,6 @@ static int parse_events__scanner(const char *str, void *data, int start_token)
 	parse_events__flush_buffer(buffer, scanner);
 	parse_events__delete_buffer(buffer, scanner);
 	parse_events_lex_destroy(scanner);
-	if (ret && !strchr(str, '/'))
-		ret = parse_events_fixup(ret, str, data, start_token);
 	return ret;
 }
 
tools/perf/util/string.c +0 -24
index 2553e5b..4b0ff22 100644
@@ -387,27 +387,3 @@ void *memdup(const void *src, size_t len)
 
 	return p;
 }
-
-/**
- * str_append - reallocate string and append another
- * @s: pointer to string pointer
- * @len: pointer to len (initialized)
- * @a: string to append.
- */
-int str_append(char **s, int *len, const char *a)
-{
-	int olen = *s ? strlen(*s) : 0;
-	int nlen = olen + strlen(a) + 1;
-	if (*len < nlen) {
-		*len = *len * 2;
-		if (*len < nlen)
-			*len = nlen;
-		*s = realloc(*s, *len);
-		if (!*s)
-			return -ENOMEM;
-		if (olen == 0)
-			**s = 0;
-	}
-	strcat(*s, a);
-	return 0;
-}
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH v3 2/3 perf tools: parse the pmu event prefix and surfix

From: Kan Liang <[email protected]>

There are two types of event formats for PMU events. E.g. el-abort OR cpu/el-abort/. However, the lexer mistakenly recognizes the simple style format as two events.

The parse_events_pmu_check function uses bsearch to search the name in known pmu event list. It can tell the lexer that the name is a PE_NAME or a PMU event name prefix or a PMU event name suffix. All these information will be used for accurately parsing kernel PMU events.

The pmu events list will be read from sysfs at runtime.

---
v2: Read kernel PMU events from sysfs at runtime
v3: Use strlcpy to replace strncpy

 tools/perf/util/parse-events.c | 103 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h |  15 ++++++
 tools/perf/util/pmu.c          |  10 ----
 tools/perf/util/pmu.h          |  10 ++++
 4 files changed, 128 insertions(+), 10 deletions(-)
tools/perf/util/parse-events.c +103 -0
index 7a0aa75..313dadd 100644
@@ -29,6 +29,9 @@ extern int parse_events_debug;
 #endif
 int parse_events_parse(void *data, void *scanner);
 
+static struct kernel_pmu_event_symbol *kernel_pmu_events_list;
+static size_t kernel_pmu_events_list_num;
+
 static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
 	[PERF_COUNT_HW_CPU_CYCLES] = {
 		.symbol = "cpu-cycles",
@@ -852,6 +855,103 @@ int parse_events_name(struct list_head *list, char *name)
 	return 0;
 }
 
+static int
+comp_pmu(const void *p1, const void *p2)
+{
+	struct kernel_pmu_event_symbol *pmu1 =
+			(struct kernel_pmu_event_symbol *) p1;
+	struct kernel_pmu_event_symbol *pmu2 =
+			(struct kernel_pmu_event_symbol *) p2;
+
+	return strcmp(pmu1->symbol, pmu2->symbol);
+}
+
+enum kernel_pmu_event_type
+parse_events_pmu_check(const char *name)
+{
+	struct kernel_pmu_event_symbol p, *r;
+
+	/*
+	 * name "cpu" could be prefix of cpu-cycles or cpu// events.
+	 * cpu-cycles has been handled by hardcode.
+	 * So it must be cpu// events, not kernel pmu event.
+	 */
+	if (!kernel_pmu_events_list_num || !strcmp(name, "cpu"))
+		return NONE_KERNEL_PMU_EVENT;
+
+	strcpy(p.symbol, name);
+	r = bsearch(&p, kernel_pmu_events_list,
+			kernel_pmu_events_list_num,
+			sizeof(struct kernel_pmu_event_symbol), comp_pmu);
+	if (r == NULL)
+		return NONE_KERNEL_PMU_EVENT;
+	return r->type;
+}
+
+/*
+ * Read the pmu events list from sysfs
+ * Save it into kernel_pmu_events_list
+ */
+static void scan_kernel_pmu_events_list(void)
+{
+
+	struct perf_pmu *pmu = NULL;
+	struct perf_pmu_alias *alias;
+	int len = 0;
+
+	while ((pmu = perf_pmu__scan(pmu)) != NULL)
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (!strcmp(pmu->name, "cpu")) {
+				if (strchr(alias->name, '-'))
+					len++;
+				len++;
+			}
+	}
+	if (len == 0)
+		return;
+	kernel_pmu_events_list =
+		malloc(sizeof(struct kernel_pmu_event_symbol) * len);
+	kernel_pmu_events_list_num = len;
+
+	pmu = NULL;
+	len = 0;
+	while ((pmu = perf_pmu__scan(pmu)) != NULL)
+		list_for_each_entry(alias, &pmu->aliases, list) {
+			if (!strcmp(pmu->name, "cpu")) {
+				struct kernel_pmu_event_symbol *p =
+					kernel_pmu_events_list + len;
+				char *tmp = strchr(alias->name, '-');
+
+				if (tmp != NULL) {
+					strlcpy(p->symbol, alias->name,
+						tmp - alias->name + 1);
+					p->type = KERNEL_PMU_EVENT_PREFIX;
+					tmp++;
+					p++;
+					strcpy(p->symbol, tmp);
+					p->type = KERNEL_PMU_EVENT_SUFFIX;
+					len += 2;
+				} else {
+					strcpy(p->symbol, alias->name);
+					p->type = KERNEL_PMU_EVENT;
+					len++;
+				}
+			}
+		}
+	qsort(kernel_pmu_events_list, len,
+		sizeof(struct kernel_pmu_event_symbol), comp_pmu);
+
+}
+
+static void release_kernel_pmu_events_list(void)
+{
+	if (kernel_pmu_events_list) {
+		free(kernel_pmu_events_list);
+		kernel_pmu_events_list = NULL;
+	}
+	kernel_pmu_events_list_num = 0;
+}
+
 static int parse_events__scanner(const char *str, void *data, int start_token)
 {
 	YY_BUFFER_STATE buffer;
@@ -905,7 +1005,10 @@ int parse_events(struct perf_evlist *evlist, const char *str)
 	};
 	int ret;
 
+	/* scan kernel pmu events from sysfs */
+	scan_kernel_pmu_events_list();
 	ret = parse_events__scanner(str, &data, PE_START_EVENTS);
+	release_kernel_pmu_events_list();
 	if (!ret) {
 		int entries = data.idx - evlist->nr_entries;
 		perf_evlist__splice_list_tail(evlist, &data.list, entries);
tools/perf/util/parse-events.h +15 -0
index df094b4..d06fec4 100644
@@ -35,6 +35,19 @@ extern int parse_filter(const struct option *opt, const char *str, int unset);
 
 #define EVENTS_HELP_MAX (128*1024)
 
+#define KERNEL_PMU_EVENT_MAX	1024
+enum kernel_pmu_event_type {
+	NONE_KERNEL_PMU_EVENT,		/* not a PMU EVENT */
+	KERNEL_PMU_EVENT,		/* normal style PMU event */
+	KERNEL_PMU_EVENT_PREFIX,	/* prefix of pre-suf style event */
+	KERNEL_PMU_EVENT_SUFFIX,	/* suffix of pre-suf style event */
+};
+
+struct kernel_pmu_event_symbol {
+	char	symbol[KERNEL_PMU_EVENT_MAX];
+	enum kernel_pmu_event_type	type;
+};
+
 enum {
 	PARSE_EVENTS__TERM_TYPE_NUM,
 	PARSE_EVENTS__TERM_TYPE_STR,
@@ -95,6 +108,8 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 				void *ptr, char *type);
 int parse_events_add_pmu(struct list_head *list, int *idx,
 			 char *pmu , struct list_head *head_config);
+enum kernel_pmu_event_type
+parse_events_pmu_check(const char *name);
 void parse_events__set_leader(char *name, struct list_head *list);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
tools/perf/util/pmu.c +0 -10
index 7a811eb..16d5c1a 100644
@@ -10,16 +10,6 @@
 #include "parse-events.h"
 #include "cpumap.h"
 
-#define UNIT_MAX_LEN	31 /* max length for event unit name */
-
-struct perf_pmu_alias {
-	char *name;
-	struct list_head terms;
-	struct list_head list;
-	char unit[UNIT_MAX_LEN+1];
-	double scale;
-};
-
 struct perf_pmu_format {
 	char *name;
 	int value;
tools/perf/util/pmu.h +10 -0
index c14a543..d9e00d6 100644
@@ -22,6 +22,16 @@ struct perf_pmu {
 	struct list_head list;
 };
 
+#define UNIT_MAX_LEN	31 /* max length for event unit name */
+
+struct perf_pmu_alias {
+	char *name;
+	struct list_head terms;
+	struct list_head list;
+	char unit[UNIT_MAX_LEN+1];
+	double scale;
+};
+
 struct perf_pmu *perf_pmu__find(const char *name);
 int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 		     struct list_head *head_terms);
Signature · 1 line
1.8.3.2
#2 lore raw

Re: [PATCH v3 2/3] perf tools: parse the pmu event prefix and surfix

Quoted 6 lines · On Mon, Sep 01, 2014 at 12:29:46PM -0400, [email protected] wrote:

From: Kan Liang <[email protected]>

There are two types of event formats for PMU events. E.g. el-abort OR cpu/el-abort/. However, the lexer mistakenly recognizes the simple style format as two events.

Quoted 6 lines

The parse_events_pmu_check function uses bsearch to search the name in known pmu event list. It can tell the lexer that the name is a PE_NAME or a PMU event name prefix or a PMU event name suffix. All these information will be used for accurately parsing kernel PMU events.

The pmu events list will be read from sysfs at runtime.

this one does not apply to latest acme's perf/core

could you please rebase your patchset to:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core

thanks,
jirka

#3 lore raw

PATCH v3 3/3 perf tools: Add support to new style format of kernel PMU event

In reply to [email protected]

From: Kan Liang <[email protected]>

Add new rules for kernel PMU event.

event_pmu:
PE_KERNEL_PMU_EVENT
|
PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF

PE_KERNEL_PMU_EVENT token is for
cycles-ct/cycles-t/mem-loads/mem-stores.
The prefix cycles is mixed up with cpu-cycles.
loads and stores are mixed up with cache event
So they have to be hardcode in lex.

PE_PMU_EVENT_PRE and PE_PMU_EVENT_SUF tokens are for other PMU events.
The lex looks generic identifier up in the table and return the matched token. If there is no match, generic PE_NAME token will be return.

Using the rules, kernel PMU event could use new style format without //

so you can use

perf record -e mem-loads ...

instead of

perf record -e cpu/mem-loads/

---
 tools/perf/util/parse-events.l | 30 +++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.y | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)
tools/perf/util/parse-events.l +29 -1
index 3432995..4dd7f04 100644
@@ -51,6 +51,24 @@ static int str(yyscan_t scanner, int token)
 	return token;
 }
 
+static int pmu_str_check(yyscan_t scanner)
+{
+	YYSTYPE *yylval = parse_events_get_lval(scanner);
+	char *text = parse_events_get_text(scanner);
+
+	yylval->str = strdup(text);
+	switch (parse_events_pmu_check(text)) {
+		case KERNEL_PMU_EVENT_PREFIX:
+			return PE_PMU_EVENT_PRE;
+		case KERNEL_PMU_EVENT_SUFFIX:
+			return PE_PMU_EVENT_SUF;
+		case KERNEL_PMU_EVENT:
+			return PE_KERNEL_PMU_EVENT;
+		default:
+			return PE_NAME;
+	}
+}
+
 static int sym(yyscan_t scanner, int type, int config)
 {
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
@@ -178,6 +196,16 @@ alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_AL
 emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
 dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
 
+	/*
+	 * We have to handle the kernel PMU event cycles-ct/cycles-t/mem-loads/mem-stores separately.
+	 * Because the prefix cycles is mixed up with cpu-cycles.
+	 * loads and stores are mixed up with cache event
+	 */
+cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+
 L1-dcache|l1-d|l1d|L1-data		|
 L1-icache|l1-i|l1i|L1-instruction	|
 LLC|L2					|
@@ -199,7 +227,7 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 {num_hex}		{ return value(yyscanner, 16); }
 
 {modifier_event}	{ return str(yyscanner, PE_MODIFIER_EVENT); }
-{name}			{ return str(yyscanner, PE_NAME); }
+{name}			{ return pmu_str_check(yyscanner); }
 "/"			{ BEGIN(config); return '/'; }
 -			{ return '-'; }
 ,			{ BEGIN(event); return ','; }
tools/perf/util/parse-events.y +42 -0

60 lines not on this page · raw message

Signature · 1 line
1.8.3.2