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

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

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

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/

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 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(-)

diff --git a/tools/perf/util/include/linux/string.h b/tools/perf/util/include/linux/string.h
index 97a8007..6f19c54 100644
--- a/tools/perf/util/include/linux/string.h
+++ b/tools/perf/util/include/linux/string.h
@@ -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);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1e15df1..7a0aa75 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -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;
 }
 
diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 2553e5b..4b0ff22 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -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;
-}
-- 
1.8.3.2

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

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

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 newly introduced 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 implementation is also extensible. For supporting more PMU events,
we only need to simply add the name in the array.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/parse-events.c | 34 ++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h | 12 ++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7a0aa75..6354648 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -666,6 +666,40 @@ int parse_events_add_pmu(struct list_head *list, int *idx,
 	return evsel ? 0 : -ENOMEM;
 }
 
+/* The contents of the pmu_events should be in ascending sorted by symbol*/
+static struct pmu_event_symbol pmu_events[] = {
+	{ "abort", false},
+	{ "capacity", false},
+	{ "commit", false},
+	{ "conflict", false},
+	{ "el", true},
+	{ "start", false},
+	{ "tx", true},
+};
+
+static int
+comp_pmu(const void *p1, const void *p2)
+{
+	struct pmu_event_symbol *pmu1 = (struct pmu_event_symbol *) p1;
+	struct pmu_event_symbol *pmu2 = (struct pmu_event_symbol *) p2;
+
+	return strcmp(pmu1->symbol, pmu2->symbol);
+}
+
+int parse_events_pmu_check(const char *name)
+{
+	struct pmu_event_symbol p, *r;
+
+	p.symbol = name;
+	r = bsearch(&p, pmu_events, ARRAY_SIZE(pmu_events),
+		sizeof(struct pmu_event_symbol), comp_pmu);
+	if (r == NULL)
+		return NONE_PMU_EVENT_FIX;
+
+	return r->is_prefix ?
+			PMU_EVENT_PREFIX : PMU_EVENT_SUFFIX;
+}
+
 int parse_events__modifier_group(struct list_head *list,
 				 char *event_mod)
 {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index df094b4..946249e 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -36,6 +36,17 @@ extern int parse_filter(const struct option *opt, const char *str, int unset);
 #define EVENTS_HELP_MAX (128*1024)
 
 enum {
+	NONE_PMU_EVENT_FIX,
+	PMU_EVENT_PREFIX,
+	PMU_EVENT_SUFFIX,
+};
+
+struct pmu_event_symbol {
+	const char	*symbol;
+	bool		is_prefix;	/* the prefix of the pmu event */
+};
+
+enum {
 	PARSE_EVENTS__TERM_TYPE_NUM,
 	PARSE_EVENTS__TERM_TYPE_STR,
 };
@@ -95,6 +106,7 @@ 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);
+int 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);
-- 
1.8.3.2

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

[kan.liang]

From: Kan Liang <kan.liang@intel.com>

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/

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/parse-events.l | 28 +++++++++++++++++++++++++++-
 tools/perf/util/parse-events.y | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 3432995..ea07fde 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -51,6 +51,22 @@ 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 PMU_EVENT_PREFIX:
+			return PE_PMU_EVENT_PRE;
+		case PMU_EVENT_SUFFIX:
+			return PE_PMU_EVENT_SUF;
+		default:
+			return PE_NAME;
+	}
+}
+
 static int sym(yyscan_t scanner, int type, int config)
 {
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
@@ -178,6 +194,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 +225,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 ','; }
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 0bc87ba..77e01e5 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -47,6 +47,7 @@ static inc_group_count(struct list_head *list,
 %token PE_NAME_CACHE_TYPE PE_NAME_CACHE_OP_RESULT
 %token PE_PREFIX_MEM PE_PREFIX_RAW PE_PREFIX_GROUP
 %token PE_ERROR
+%token PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
 %type <num> PE_VALUE
 %type <num> PE_VALUE_SYM_HW
 %type <num> PE_VALUE_SYM_SW
@@ -58,6 +59,7 @@ static inc_group_count(struct list_head *list,
 %type <str> PE_MODIFIER_EVENT
 %type <str> PE_MODIFIER_BP
 %type <str> PE_EVENT_NAME
+%type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
 %type <num> value_sym
 %type <head> event_config
 %type <term> event_term
@@ -210,6 +212,46 @@ PE_NAME '/' event_config '/'
 	parse_events__free_terms($3);
 	$$ = list;
 }
+|
+PE_KERNEL_PMU_EVENT
+{
+	struct parse_events_evlist *data = _data;
+	struct list_head *head = malloc(sizeof(*head));
+	struct parse_events_term *term;
+	struct list_head *list;
+
+	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, 1));
+	ABORT_ON(!head);
+	INIT_LIST_HEAD(head);
+	list_add_tail(&term->list, head);
+
+	ALLOC_LIST(list);
+	ABORT_ON(parse_events_add_pmu(list, &data->idx, "cpu", head));
+	parse_events__free_terms(head);
+	$$ = list;
+}
+|
+PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF
+{
+	struct parse_events_evlist *data = _data;
+	struct list_head *head = malloc(sizeof(*head));
+	struct parse_events_term *term;
+	struct list_head *list;
+	char pmu_name[128];
+	snprintf(&pmu_name, 128, "%s-%s", $1, $3);
+
+	ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					&pmu_name, 1));
+	ABORT_ON(!head);
+	INIT_LIST_HEAD(head);
+	list_add_tail(&term->list, head);
+
+	ALLOC_LIST(list);
+	ABORT_ON(parse_events_add_pmu(list, &data->idx, "cpu", head));
+	parse_events__free_terms(head);
+	$$ = list;
+}
 
 value_sym:
 PE_VALUE_SYM_HW
-- 
1.8.3.2

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

[Andi Kleen]

> +/* The contents of the pmu_events should be in ascending sorted by symbol*/
> +static struct pmu_event_symbol pmu_events[] = {
> +	{ "abort", false},
> +	{ "capacity", false},
> +	{ "commit", false},
> +	{ "conflict", false},
> +	{ "el", true},
> +	{ "start", false},
> +	{ "tx", true},

This list should be read from sysfs, not hard coded.

-Andi