[RFCv2 0/10] perf, tool: Allow to use hw events in PMU syntax

[RFCv2 0/10] perf, tool: Allow to use hw events in PMU syntax

[Jiri Olsa]

hi,
here's the change to make following syntax available:
  perf stat -e cpu/event=instructions/u ls

this is identical to:
  perf stat -e instructions:u ls

v2 changes:
  - making the hw events translations available under the 'events',
    the userspace trnaslation is then done by existing term aliasing
    code with some little tweeks ;)
  - patches 1-3 are independent fixies

Attached patches:
  01/10 perf, tool: Add empty rule for new line in event syntax parsing
  02/10 perf, tool: Fix pmu object initialization
  03/10 perf, tool: Properly free format data
  04/10 perf, x86: Making hardware events translations available in sysfs
  05/10 perf, tool: Split out PE_VALUE_SYM parsing token to SW and HW tokens
  06/10 perf, tool: Split event symbols arrays to hw and sw parts
  07/10 perf, tool: Add support to specify hw event as pmu event term
  08/10 perf, tool: Add sysfs read file interface
  09/10 perf, test: Use ARRAY_SIZE in parse events tests
  10/10 perf, test: Add automated tests for pmu sysfs translated events

jirka
---
 arch/x86/kernel/cpu/perf_event.c    |   44 ++++++++++++++
 tools/perf/util/parse-events-test.c |  158 ++++++++++++++++++++++++++++++++++++++++++++------
 tools/perf/util/parse-events.c      |  203 +++++++++++++++++++++++++++++++++++++++++++++++------------------
 tools/perf/util/parse-events.h      |    2 +
 tools/perf/util/parse-events.l      |    3 +-
 tools/perf/util/parse-events.y      |   24 ++++++--
 tools/perf/util/pmu.c               |   60 ++++++++++++-------
 tools/perf/util/sysfs.c             |   19 ++++++
 tools/perf/util/sysfs.h             |    1 +
 9 files changed, 413 insertions(+), 101 deletions(-)

[PATCH 01/10] perf, tool: Add empty rule for new line in event syntax parsing

[Jiri Olsa]

The flex generator prints out each input character that is ignored
by lex rules.

Since the alias processing, we can have '\n' characters on input. We
need to assign empty rule to it, so it's not printed out.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/parse-events.l |    1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 488362e..c05dbef 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -152,6 +152,7 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 ,			{ return ','; }
 :			{ return ':'; }
 =			{ return '='; }
+\n			{ }
 
 <mem>{
 {modifier_bp}		{ return str(yyscanner, PE_MODIFIER_BP); }
-- 
1.7.10.4

[PATCH 02/10] perf, tool: Fix pmu object initialization

[Jiri Olsa]

The internal pmu list was never used. With each perf_pmu__find() call
the pmu structure was created new by parsing sysfs. Beside this it
caused memory leaks. We now keep all pmus by adding them to the list.

Also, pmu_lookup() should return pmus that do not expose the format
specifier in sysfs.

We need a valid internal pmu list in a later patch to iterate over all
pmus that exist in the system.

Signed-off-by: Robert Richter <robert.richter@amd.com>
[ added same treatment for 'event' sysfs group attribute ]
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/pmu.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 74d0948e..f1f83e6 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -72,7 +72,7 @@ static int pmu_format(char *name, struct list_head *format)
 		 "%s/bus/event_source/devices/%s/format", sysfs, name);
 
 	if (stat(path, &st) < 0)
-		return -1;
+		return 0;	/* no error if 'format' does not exist */
 
 	if (pmu_format_parse(path, format))
 		return -1;
@@ -161,7 +161,7 @@ static int pmu_aliases(char *name, struct list_head *head)
 		 "%s/bus/event_source/devices/%s/events", sysfs, name);
 
 	if (stat(path, &st) < 0)
-		return -1;
+		return 0;	/* no error if 'events' does not exist */
 
 	if (pmu_aliases_parse(path, head))
 		return -1;
@@ -237,6 +237,9 @@ static struct perf_pmu *pmu_lookup(char *name)
 	if (pmu_format(name, &format))
 		return NULL;
 
+	if (pmu_aliases(name, &aliases))
+		return NULL;
+
 	if (pmu_type(name, &type))
 		return NULL;
 
@@ -244,14 +247,13 @@ static struct perf_pmu *pmu_lookup(char *name)
 	if (!pmu)
 		return NULL;
 
-	pmu_aliases(name, &aliases);
-
 	INIT_LIST_HEAD(&pmu->format);
 	INIT_LIST_HEAD(&pmu->aliases);
 	list_splice(&format, &pmu->format);
 	list_splice(&aliases, &pmu->aliases);
 	pmu->name = strdup(name);
 	pmu->type = type;
+	list_add_tail(&pmu->list, &pmus);
 	return pmu;
 }
 
-- 
1.7.10.4

[PATCH 03/10] perf, tool: Properly free format data

[Jiri Olsa]

Release format data in case there's failure during PMU load.

Format data are allocated during PMU lookup. If the lookup
fails in next steps, we dont release the format data.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/pmu.c |   58 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f1f83e6..9429ba7 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -15,6 +15,14 @@ extern FILE *perf_pmu_in;
 
 static LIST_HEAD(pmus);
 
+static void pmu_format_release(struct list_head *head)
+{
+	struct perf_pmu__format *format, *h;
+
+	list_for_each_entry_safe(format, h, head, list)
+		free(format);
+}
+
 /*
  * Parse & process all the sysfs attributes located under
  * the directory specified in 'dir' parameter.
@@ -229,32 +237,38 @@ static struct perf_pmu *pmu_lookup(char *name)
 	LIST_HEAD(aliases);
 	__u32 type;
 
-	/*
-	 * The pmu data we store & need consists of the pmu
-	 * type value and format definitions. Load both right
-	 * now.
-	 */
-	if (pmu_format(name, &format))
-		return NULL;
+	do {
+		/*
+		* The pmu data we store & need consists of the pmu
+		* type value, format and events definitions. Load
+		* all of them right now.
+		*/
+		if (pmu_format(name, &format))
+			break;
 
-	if (pmu_aliases(name, &aliases))
-		return NULL;
+		if (pmu_aliases(name, &aliases))
+			break;
 
-	if (pmu_type(name, &type))
-		return NULL;
+		if (pmu_type(name, &type))
+			break;
 
-	pmu = zalloc(sizeof(*pmu));
-	if (!pmu)
-		return NULL;
+		pmu = zalloc(sizeof(*pmu));
+		if (!pmu)
+			break;
 
-	INIT_LIST_HEAD(&pmu->format);
-	INIT_LIST_HEAD(&pmu->aliases);
-	list_splice(&format, &pmu->format);
-	list_splice(&aliases, &pmu->aliases);
-	pmu->name = strdup(name);
-	pmu->type = type;
-	list_add_tail(&pmu->list, &pmus);
-	return pmu;
+		INIT_LIST_HEAD(&pmu->format);
+		INIT_LIST_HEAD(&pmu->aliases);
+		list_splice(&format, &pmu->format);
+		list_splice(&aliases, &pmu->aliases);
+		pmu->name = strdup(name);
+		pmu->type = type;
+		list_add_tail(&pmu->list, &pmus);
+		return pmu;
+
+	} while (0);
+
+	pmu_format_release(&format);
+	return NULL;
 }
 
 static struct perf_pmu *pmu_find(char *name)
-- 
1.7.10.4

[PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Jiri Olsa]

Making hardware events translations available through the sysfs.
Adding 'events' group attribute under the sysfs x86 PMU record
with attribute/file for each hardware event:

  # ls  /sys/devices/cpu/events/
  branch_instructions
  branch_misses
  bus_cycles
  cache_misses
  cache_references
  cycles
  instructions
  ref_cycles
  stalled_cycles_backend
  stalled_cycles_frontend

The file - hw event ID mappings is:

  file                      hw event ID
  ---------------------------------------------------------------
  cycles                    PERF_COUNT_HW_CPU_CYCLES
  instructions              PERF_COUNT_HW_INSTRUCTIONS
  cache_references          PERF_COUNT_HW_CACHE_REFERENCES
  cache_misses              PERF_COUNT_HW_CACHE_MISSES
  branch_instructions       PERF_COUNT_HW_BRANCH_INSTRUCTIONS
  branch_misses             PERF_COUNT_HW_BRANCH_MISSES
  bus_cycles                PERF_COUNT_HW_BUS_CYCLES
  stalled_cycles_frontend   PERF_COUNT_HW_STALLED_CYCLES_FRONTEND
  stalled_cycles_backend    PERF_COUNT_HW_STALLED_CYCLES_BACKEND
  ref_cycles                PERF_COUNT_HW_REF_CPU_CYCLES

Each file in 'events' directory contains term translation for the
symbolic hw event for the currently running cpu model.

  # cat /sys/devices/cpu/events/instructions
  config=0xc0

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 arch/x86/kernel/cpu/perf_event.c |   44 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 41db515..e7aab56 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1666,9 +1666,53 @@ static struct attribute_group x86_pmu_attr_group = {
 	.attrs = x86_pmu_attrs,
 };
 
+#define PMU_EVENTS_ATTR_CONFIG(_name, _id)				\
+static ssize_t								\
+_name##_show(struct device *dev,					\
+			   struct device_attribute *attr,		\
+			   char *page)					\
+{									\
+	u64 val = x86_pmu.event_map(_id);				\
+	BUILD_BUG_ON(_id >= PERF_COUNT_HW_MAX);				\
+	return sprintf(page, "config=0x%llx\n", val);			\
+}									\
+									\
+static struct device_attribute event_attr_##_name = __ATTR_RO(_name)
+
+PMU_EVENTS_ATTR_CONFIG(cycles, PERF_COUNT_HW_CPU_CYCLES);
+PMU_EVENTS_ATTR_CONFIG(instructions, PERF_COUNT_HW_INSTRUCTIONS);
+PMU_EVENTS_ATTR_CONFIG(cache_references, PERF_COUNT_HW_CACHE_REFERENCES);
+PMU_EVENTS_ATTR_CONFIG(cache_misses, PERF_COUNT_HW_CACHE_MISSES);
+PMU_EVENTS_ATTR_CONFIG(branch_instructions, PERF_COUNT_HW_BRANCH_INSTRUCTIONS);
+PMU_EVENTS_ATTR_CONFIG(branch_misses, PERF_COUNT_HW_BRANCH_MISSES);
+PMU_EVENTS_ATTR_CONFIG(bus_cycles, PERF_COUNT_HW_BUS_CYCLES);
+PMU_EVENTS_ATTR_CONFIG(stalled_cycles_frontend, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND);
+PMU_EVENTS_ATTR_CONFIG(stalled_cycles_backend, PERF_COUNT_HW_STALLED_CYCLES_BACKEND);
+PMU_EVENTS_ATTR_CONFIG(ref_cycles, PERF_COUNT_HW_REF_CPU_CYCLES);
+
+static struct attribute *events_attr[] = {
+	&event_attr_cycles.attr,
+	&event_attr_instructions.attr,
+	&event_attr_cache_references.attr,
+	&event_attr_cache_misses.attr,
+	&event_attr_branch_instructions.attr,
+	&event_attr_branch_misses.attr,
+	&event_attr_bus_cycles.attr,
+	&event_attr_stalled_cycles_frontend.attr,
+	&event_attr_stalled_cycles_backend.attr,
+	&event_attr_ref_cycles.attr,
+	NULL,
+};
+
+static struct attribute_group x86_pmu_events_group = {
+	.name = "events",
+	.attrs = events_attr,
+};
+
 static const struct attribute_group *x86_pmu_attr_groups[] = {
 	&x86_pmu_attr_group,
 	&x86_pmu_format_group,
+	&x86_pmu_events_group,
 	NULL,
 };
 
-- 
1.7.10.4

[PATCH 05/10] perf, tool: Split out PE_VALUE_SYM parsing token to SW and HW tokens

[Jiri Olsa]

Spliting PE_VALUE_SYM token to PE_VALUE_SYM_HW and PE_VALUE_SYM_SW
tokens to separate hardware and software symbols.

This will be usefull in upcomming patch where we want to be able
to parse out only hardware events.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/parse-events.l |    2 +-
 tools/perf/util/parse-events.y |   15 +++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index c05dbef..86b27a5 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -56,7 +56,7 @@ static int sym(yyscan_t scanner, int type, int config)
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
 
 	yylval->num = (type << 16) + config;
-	return PE_VALUE_SYM;
+	return type == PERF_TYPE_HARDWARE ? PE_VALUE_SYM_HW : PE_VALUE_SYM_SW;
 }
 
 static int term(yyscan_t scanner, int type)
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 9525c45..2bc5fbf 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -26,14 +26,15 @@ do { \
 %}
 
 %token PE_START_EVENTS PE_START_TERMS
-%token PE_VALUE PE_VALUE_SYM PE_RAW PE_TERM
+%token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_RAW PE_TERM
 %token PE_NAME
 %token PE_MODIFIER_EVENT PE_MODIFIER_BP
 %token PE_NAME_CACHE_TYPE PE_NAME_CACHE_OP_RESULT
 %token PE_PREFIX_MEM PE_PREFIX_RAW
 %token PE_ERROR
 %type <num> PE_VALUE
-%type <num> PE_VALUE_SYM
+%type <num> PE_VALUE_SYM_HW
+%type <num> PE_VALUE_SYM_SW
 %type <num> PE_RAW
 %type <num> PE_TERM
 %type <str> PE_NAME
@@ -41,6 +42,7 @@ do { \
 %type <str> PE_NAME_CACHE_OP_RESULT
 %type <str> PE_MODIFIER_EVENT
 %type <str> PE_MODIFIER_BP
+%type <num> value_sym
 %type <head> event_config
 %type <term> event_term
 %type <head> event_pmu
@@ -109,8 +111,13 @@ PE_NAME '/' event_config '/'
 	$$ = list;
 }
 
+value_sym:
+PE_VALUE_SYM_HW
+|
+PE_VALUE_SYM_SW
+
 event_legacy_symbol:
-PE_VALUE_SYM '/' event_config '/'
+value_sym '/' event_config '/'
 {
 	struct parse_events_data__events *data = _data;
 	struct list_head *list = NULL;
@@ -123,7 +130,7 @@ PE_VALUE_SYM '/' event_config '/'
 	$$ = list;
 }
 |
-PE_VALUE_SYM sep_slash_dc
+value_sym sep_slash_dc
 {
 	struct parse_events_data__events *data = _data;
 	struct list_head *list = NULL;
-- 
1.7.10.4

[PATCH 06/10] perf, tool: Split event symbols arrays to hw and sw parts

[Jiri Olsa]

It'll be convenient in upcoming patch to access hw event symbols
strings via enum perf_hw_id indexes. In order not to duplicate
the data, creating two separate arrays:

  event_symbols_hw for enum perf_hw_id events
  event_symbols_sw for enum perf_sw_ids events

Changing the current event list code to follow the change.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/parse-events.c |  174 +++++++++++++++++++++++++++-------------
 1 file changed, 117 insertions(+), 57 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0cc27da..6d12b39 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -19,8 +19,6 @@
 #define MAX_NAME_LEN 100
 
 struct event_symbol {
-	u8		type;
-	u64		config;
 	const char	*symbol;
 	const char	*alias;
 };
@@ -30,30 +28,86 @@ extern int parse_events_debug;
 #endif
 int parse_events_parse(void *data, void *scanner);
 
-#define CHW(x) .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_##x
-#define CSW(x) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_##x
-
-static struct event_symbol event_symbols[] = {
-  { CHW(CPU_CYCLES),			"cpu-cycles",			"cycles"		},
-  { CHW(STALLED_CYCLES_FRONTEND),	"stalled-cycles-frontend",	"idle-cycles-frontend"	},
-  { CHW(STALLED_CYCLES_BACKEND),	"stalled-cycles-backend",	"idle-cycles-backend"	},
-  { CHW(INSTRUCTIONS),			"instructions",			""			},
-  { CHW(CACHE_REFERENCES),		"cache-references",		""			},
-  { CHW(CACHE_MISSES),			"cache-misses",			""			},
-  { CHW(BRANCH_INSTRUCTIONS),		"branch-instructions",		"branches"		},
-  { CHW(BRANCH_MISSES),			"branch-misses",		""			},
-  { CHW(BUS_CYCLES),			"bus-cycles",			""			},
-  { CHW(REF_CPU_CYCLES),		"ref-cycles",			""			},
-
-  { CSW(CPU_CLOCK),			"cpu-clock",			""			},
-  { CSW(TASK_CLOCK),			"task-clock",			""			},
-  { CSW(PAGE_FAULTS),			"page-faults",			"faults"		},
-  { CSW(PAGE_FAULTS_MIN),		"minor-faults",			""			},
-  { CSW(PAGE_FAULTS_MAJ),		"major-faults",			""			},
-  { CSW(CONTEXT_SWITCHES),		"context-switches",		"cs"			},
-  { CSW(CPU_MIGRATIONS),		"cpu-migrations",		"migrations"		},
-  { CSW(ALIGNMENT_FAULTS),		"alignment-faults",		""			},
-  { CSW(EMULATION_FAULTS),		"emulation-faults",		""			},
+static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
+	[PERF_COUNT_HW_CPU_CYCLES] = {
+		.symbol = "cpu-cycles",
+		.alias  = "cycles",
+	},
+	[PERF_COUNT_HW_INSTRUCTIONS] = {
+		.symbol = "instructions",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_CACHE_REFERENCES] = {
+		.symbol = "cache-references",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_CACHE_MISSES] = {
+		.symbol = "cache-misses",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = {
+		.symbol = "branch-instructions",
+		.alias  = "branches",
+	},
+	[PERF_COUNT_HW_BRANCH_MISSES] = {
+		.symbol = "branch-misses",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_BUS_CYCLES] = {
+		.symbol = "bus-cycles",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = {
+		.symbol = "stalled-cycles-frontend",
+		.alias  = "idle-cycles-frontend",
+	},
+	[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = {
+		.symbol = "stalled-cycles-backend",
+		.alias  = "idle-cycles-backend",
+	},
+	[PERF_COUNT_HW_REF_CPU_CYCLES] = {
+		.symbol = "ref-cycles",
+		.alias  = "",
+	},
+};
+
+static struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
+	[PERF_COUNT_SW_CPU_CLOCK] = {
+		.symbol = "cpu-clock",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_TASK_CLOCK] = {
+		.symbol = "task-clock",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_PAGE_FAULTS] = {
+		.symbol = "page-faults",
+		.alias  = "faults",
+	},
+	[PERF_COUNT_SW_CONTEXT_SWITCHES] = {
+		.symbol = "context-switches",
+		.alias  = "cs",
+	},
+	[PERF_COUNT_SW_CPU_MIGRATIONS] = {
+		.symbol = "cpu-migrations",
+		.alias  = "migrations",
+	},
+	[PERF_COUNT_SW_PAGE_FAULTS_MIN] = {
+		.symbol = "minor-faults",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_PAGE_FAULTS_MAJ] = {
+		.symbol = "major-faults",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_ALIGNMENT_FAULTS] = {
+		.symbol = "alignment-faults",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_EMULATION_FAULTS] = {
+		.symbol = "emulation-faults",
+		.alias  = "",
+	},
 };
 
 #define __PERF_EVENT_FIELD(config, name) \
@@ -816,16 +870,13 @@ int is_valid_tracepoint(const char *event_string)
 	return 0;
 }
 
-void print_events_type(u8 type)
+static void __print_events_type(u8 type, struct event_symbol *syms,
+				unsigned max)
 {
-	struct event_symbol *syms = event_symbols;
-	unsigned int i;
 	char name[64];
+	unsigned i;
 
-	for (i = 0; i < ARRAY_SIZE(event_symbols); i++, syms++) {
-		if (type != syms->type)
-			continue;
-
+	for (i = 0; i < max ; i++, syms++) {
 		if (strlen(syms->alias))
 			snprintf(name, sizeof(name),  "%s OR %s",
 				 syms->symbol, syms->alias);
@@ -837,6 +888,14 @@ void print_events_type(u8 type)
 	}
 }
 
+void print_events_type(u8 type)
+{
+	if (type == PERF_TYPE_SOFTWARE)
+		__print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
+	else
+		__print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
+}
+
 int print_hwcache_events(const char *event_glob)
 {
 	unsigned int type, op, i, printed = 0;
@@ -864,26 +923,13 @@ int print_hwcache_events(const char *event_glob)
 	return printed;
 }
 
-/*
- * Print the help text for the event symbols:
- */
-void print_events(const char *event_glob)
+static void print_symbol_events(const char *event_glob, unsigned type,
+				struct event_symbol *syms, unsigned max)
 {
-	unsigned int i, type, prev_type = -1, printed = 0, ntypes_printed = 0;
-	struct event_symbol *syms = event_symbols;
+	unsigned i, printed = 0;
 	char name[MAX_NAME_LEN];
 
-	printf("\n");
-	printf("List of pre-defined events (to be used in -e):\n");
-
-	for (i = 0; i < ARRAY_SIZE(event_symbols); i++, syms++) {
-		type = syms->type;
-
-		if (type != prev_type && printed) {
-			printf("\n");
-			printed = 0;
-			ntypes_printed++;
-		}
+	for (i = 0; i < max; i++, syms++) {
 
 		if (event_glob != NULL && 
 		    !(strglobmatch(syms->symbol, event_glob) ||
@@ -894,17 +940,31 @@ void print_events(const char *event_glob)
 			snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
 		else
 			strncpy(name, syms->symbol, MAX_NAME_LEN);
-		printf("  %-50s [%s]\n", name,
-			event_type_descriptors[type]);
 
-		prev_type = type;
-		++printed;
+		printf("  %-50s [%s]\n", name, event_type_descriptors[type]);
+
+		printed++;
 	}
 
-	if (ntypes_printed) {
-		printed = 0;
+	if (printed)
 		printf("\n");
-	}
+}
+
+/*
+ * Print the help text for the event symbols:
+ */
+void print_events(const char *event_glob)
+{
+
+	printf("\n");
+	printf("List of pre-defined events (to be used in -e):\n");
+
+	print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
+			    event_symbols_hw, PERF_COUNT_HW_MAX);
+
+	print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
+			    event_symbols_sw, PERF_COUNT_SW_MAX);
+
 	print_hwcache_events(event_glob);
 
 	if (event_glob != NULL)
-- 
1.7.10.4

[PATCH 07/10] perf, tool: Add support to specify hw event as pmu event term

[Jiri Olsa]

Adding a way to specify hw event as pmu event term like:
 'cpu/event=cycles/u'
 'cpu/event=instructions,.../u'

The 'event=cycles' term needs to be replaced/translated by the hw
events term translation, which is exposed by sysfs 'events' group
attribute.

The 'events' attribute is handled by the pmu alias code, so the
only thing we need to ensure is to map the event name to the proper
sysfs attribute name.

It's possible to use sysfs attribute name as 'event' term value,
or any possible alias for hw event, e.g.:
  cycles                     OR cpu-cycles
  branch_misses              OR branch-misses
  bus_cycles                 OR bus-cycles
  cache_misses               OR cache-misses
  cache_references           OR cache-references
  ref_cycles                 OR ref-cycles
  stalled_cycles_backend     OR stalled-cycles-backend
  stalled_cycles_frontend    OR stalled-cycles-frontend

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/parse-events.c |   29 +++++++++++++++++++++++++++++
 tools/perf/util/parse-events.h |    2 ++
 tools/perf/util/parse-events.y |    9 +++++++++
 3 files changed, 40 insertions(+)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6d12b39..a0fb567 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -21,6 +21,11 @@
 struct event_symbol {
 	const char	*symbol;
 	const char	*alias;
+	/*
+	 * The attribute name inside the 'events' sysfs
+	 * group. Only for HW events.
+	 */
+	const char	*sysfs;
 };
 
 #ifdef PARSER_DEBUG
@@ -32,42 +37,52 @@ static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
 	[PERF_COUNT_HW_CPU_CYCLES] = {
 		.symbol = "cpu-cycles",
 		.alias  = "cycles",
+		.sysfs  = "cycles",
 	},
 	[PERF_COUNT_HW_INSTRUCTIONS] = {
 		.symbol = "instructions",
 		.alias  = "",
+		.sysfs  = "instructions",
 	},
 	[PERF_COUNT_HW_CACHE_REFERENCES] = {
 		.symbol = "cache-references",
 		.alias  = "",
+		.sysfs  = "cache_references",
 	},
 	[PERF_COUNT_HW_CACHE_MISSES] = {
 		.symbol = "cache-misses",
 		.alias  = "",
+		.sysfs  = "cache_misses",
 	},
 	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = {
 		.symbol = "branch-instructions",
 		.alias  = "branches",
+		.sysfs  = "branch_instructions",
 	},
 	[PERF_COUNT_HW_BRANCH_MISSES] = {
 		.symbol = "branch-misses",
 		.alias  = "",
+		.sysfs  = "branch_misses",
 	},
 	[PERF_COUNT_HW_BUS_CYCLES] = {
 		.symbol = "bus-cycles",
 		.alias  = "",
+		.sysfs  = "bus_cycles",
 	},
 	[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = {
 		.symbol = "stalled-cycles-frontend",
 		.alias  = "idle-cycles-frontend",
+		.sysfs  = "stalled_cycles_frontend",
 	},
 	[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = {
 		.symbol = "stalled-cycles-backend",
 		.alias  = "idle-cycles-backend",
+		.sysfs  = "stalled_cycles_backend",
 	},
 	[PERF_COUNT_HW_REF_CPU_CYCLES] = {
 		.symbol = "ref-cycles",
 		.alias  = "",
+		.sysfs  = "ref_cycles",
 	},
 };
 
@@ -1037,6 +1052,20 @@ int parse_events__term_str(struct parse_events__term **term,
 			config, str, 0);
 }
 
+int parse_events__term_sym_hw(struct parse_events__term **term,
+			      char *config, unsigned idx)
+{
+	struct event_symbol *sym;
+
+	BUG_ON(idx >= PERF_COUNT_HW_MAX);
+	sym = &event_symbols_hw[idx];
+	BUG_ON(!sym->sysfs);
+
+	return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
+			PARSE_EVENTS__TERM_TYPE_USER, config,
+			(char *) sym->sysfs, 0);
+}
+
 int parse_events__term_clone(struct parse_events__term **new,
 			     struct parse_events__term *term)
 {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ee9c218..eaf2fd7 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -76,6 +76,8 @@ int parse_events__term_num(struct parse_events__term **_term,
 			   int type_term, char *config, long num);
 int parse_events__term_str(struct parse_events__term **_term,
 			   int type_term, char *config, char *str);
+int parse_events__term_sym_hw(struct parse_events__term **term,
+			      char *config, unsigned idx);
 int parse_events__term_clone(struct parse_events__term **new,
 			     struct parse_events__term *term);
 void parse_events__free_terms(struct list_head *terms);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 2bc5fbf..9c74d8a 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -269,6 +269,15 @@ PE_NAME '=' PE_VALUE
 	$$ = term;
 }
 |
+PE_NAME '=' PE_VALUE_SYM_HW
+{
+	struct parse_events__term *term;
+	int config = $3 & 255;
+
+	ABORT_ON(parse_events__term_sym_hw(&term, $1, config));
+	$$ = term;
+}
+|
 PE_NAME
 {
 	struct parse_events__term *term;
-- 
1.7.10.4

[PATCH 08/10] perf, tool: Add sysfs read file interface

[Jiri Olsa]

Adding read file interface to access sysfs files data.

  int sysfs_read_file(const char *file, char *buf, size_t size)

The 'file' argument is file path relative to the sysfs mount.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/sysfs.c |   19 +++++++++++++++++++
 tools/perf/util/sysfs.h |    1 +
 2 files changed, 20 insertions(+)

diff --git a/tools/perf/util/sysfs.c b/tools/perf/util/sysfs.c
index 48c6902..4ee886a 100644
--- a/tools/perf/util/sysfs.c
+++ b/tools/perf/util/sysfs.c
@@ -58,3 +58,22 @@ const char *sysfs_find_mountpoint(void)
 
 	return sysfs_found ? sysfs_mountpoint : NULL;
 }
+
+int sysfs_read_file(const char *file, char *buf, size_t size)
+{
+	char path[PATH_MAX];
+	FILE *fp;
+	int ret = 0;
+
+	snprintf(path, PATH_MAX, "%s/%s", sysfs_mountpoint, file);
+
+	fp = fopen(path, "r");
+	if (!fp)
+		return -errno;
+
+	if (fread(buf, 1, size, fp) == 0)
+		ret = -errno;
+
+	fclose(fp);
+	return ret;
+}
diff --git a/tools/perf/util/sysfs.h b/tools/perf/util/sysfs.h
index a813b72..5ce99e7 100644
--- a/tools/perf/util/sysfs.h
+++ b/tools/perf/util/sysfs.h
@@ -2,5 +2,6 @@
 #define __SYSFS_H__
 
 const char *sysfs_find_mountpoint(void);
+int sysfs_read_file(const char *file, char *buf, size_t size);
 
 #endif /* __DEBUGFS_H__ */
-- 
1.7.10.4

[PATCH 09/10] perf, test: Use ARRAY_SIZE in parse events tests

[Jiri Olsa]

Use ARRAY_SIZE instead of defining the sizes separately for
each test arrays.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/parse-events-test.c |   29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index a0f61a2..ef5cce0 100644
--- a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -587,8 +587,6 @@ static struct test__event_st test__events[] = {
 	},
 };
 
-#define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
-
 static struct test__event_st test__events_pmu[] = {
 	[0] = {
 		.name  = "cpu/config=10,config1,config2=3,period=1000/u",
@@ -600,9 +598,6 @@ static struct test__event_st test__events_pmu[] = {
 	},
 };
 
-#define TEST__EVENTS_PMU_CNT (sizeof(test__events_pmu) / \
-			      sizeof(struct test__event_st))
-
 struct test__term {
 	const char *str;
 	__u32 type;
@@ -718,21 +713,17 @@ int parse_events__test(void)
 {
 	int ret;
 
-	do {
-		ret = test_events(test__events, TEST__EVENTS_CNT);
-		if (ret)
-			break;
-
-		if (test_pmu()) {
-			ret = test_events(test__events_pmu,
-					  TEST__EVENTS_PMU_CNT);
-			if (ret)
-				break;
-		}
+#define TEST_EVENTS(tests)				\
+do {							\
+	ret = test_events(tests, ARRAY_SIZE(tests));	\
+	if (ret)					\
+		return ret;				\
+} while (0)
 
-		ret = test_terms(test__terms, TEST__TERMS_CNT);
+	TEST_EVENTS(test__events);
 
-	} while (0);
+	if (test_pmu())
+		TEST_EVENTS(test__events_pmu);
 
-	return ret;
+	return test_terms(test__terms, ARRAY_SIZE(test__terms));
 }
-- 
1.7.10.4

[PATCH 10/10] perf, test: Add automated tests for pmu sysfs translated events

[Jiri Olsa]

Addint automated tests for following events:
  cpu/event=cycles/u
  cpu/event=instructions/u
  cpu/event=branch_instructions/u
  cpu/event=branch_misses/u
  cpu/event=bus_cycles/u
  cpu/event=cache_misses/u
  cpu/event=cache_references/u
  cpu/event=ref_cycles/u
  cpu/event=stalled_cycles_backend/u
  cpu/event=stalled_cycles_frontend/u
  /* dash variants */
  cpu/event=branch-instructions/u
  cpu/event=branch-misses/u
  cpu/event=bus-cycles/u
  cpu/event=cache-misses/u
  cpu/event=cache-references/u
  cpu/event=ref-cycles/u
  cpu/event=stalled-cycles-backend/u
  cpu/event=stalled-cycles-frontend/u

The 'event=xxx' term is translated to the cpu specific term.
We check the final perf_event_attr::config to follow this term.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/parse-events-test.c |  133 +++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index ef5cce0..95a501d 100644
--- a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -431,6 +431,45 @@ static int test__checkevent_pmu_name(struct perf_evlist *evlist)
 	return 0;
 }
 
+static __u64 get_config(const char *file)
+{
+#define DATASIZE 64
+	char data[DATASIZE];
+	u64 config;
+
+	TEST_ASSERT_VAL("cannot read sysfs events data",
+			!sysfs_read_file(file, data, DATASIZE));
+
+	TEST_ASSERT_VAL("cannot parse sysfs events data",
+			1 == sscanf(data, "config=0x%" PRIx64 "\n", &config));
+
+	return config;
+#undef DATASIZE
+}
+
+#define TEST__CHECKEVENT_PMU_EVENTS(event)				     \
+static int  test__checkevent_pmu_events_##event(struct perf_evlist *evlist)  \
+{									     \
+	struct perf_evsel *evsel;					     \
+	u64 config = get_config("devices/cpu/events/" # event);		     \
+	evsel = list_entry(evlist->entries.next, struct perf_evsel, node);   \
+	TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->nr_entries); \
+	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->attr.type);    \
+	TEST_ASSERT_VAL("wrong config", evsel->attr.config == config);	     \
+	return 0;							     \
+}
+
+TEST__CHECKEVENT_PMU_EVENTS(cycles)
+TEST__CHECKEVENT_PMU_EVENTS(instructions)
+TEST__CHECKEVENT_PMU_EVENTS(branch_instructions)
+TEST__CHECKEVENT_PMU_EVENTS(branch_misses)
+TEST__CHECKEVENT_PMU_EVENTS(bus_cycles)
+TEST__CHECKEVENT_PMU_EVENTS(cache_misses)
+TEST__CHECKEVENT_PMU_EVENTS(cache_references)
+TEST__CHECKEVENT_PMU_EVENTS(ref_cycles)
+TEST__CHECKEVENT_PMU_EVENTS(stalled_cycles_backend)
+TEST__CHECKEVENT_PMU_EVENTS(stalled_cycles_frontend)
+
 static int test__checkterms_simple(struct list_head *terms)
 {
 	struct parse_events__term *term;
@@ -598,6 +637,82 @@ static struct test__event_st test__events_pmu[] = {
 	},
 };
 
+static struct test__event_st test__events_pmu_events[] = {
+	[0] = {
+		.name  = "cpu/event=cycles/u",
+		.check = test__checkevent_pmu_events_cycles,
+	},
+	[1] = {
+		.name  = "cpu/event=instructions/u",
+		.check = test__checkevent_pmu_events_instructions,
+	},
+	[2] = {
+		.name  = "cpu/event=branch_instructions/u",
+		.check = test__checkevent_pmu_events_branch_instructions,
+	},
+	[3] = {
+		.name  = "cpu/event=branch_misses/u",
+		.check = test__checkevent_pmu_events_branch_misses,
+	},
+	[4] = {
+		.name  = "cpu/event=bus_cycles/u",
+		.check = test__checkevent_pmu_events_bus_cycles,
+	},
+	[5] = {
+		.name  = "cpu/event=cache_misses/u",
+		.check = test__checkevent_pmu_events_cache_misses,
+	},
+	[6] = {
+		.name  = "cpu/event=cache_references/u",
+		.check = test__checkevent_pmu_events_cache_references,
+	},
+	[7] = {
+		.name  = "cpu/event=ref_cycles/u",
+		.check = test__checkevent_pmu_events_ref_cycles,
+	},
+	[8] = {
+		.name  = "cpu/event=stalled_cycles_backend/u",
+		.check = test__checkevent_pmu_events_stalled_cycles_backend,
+	},
+	[9] = {
+		.name  = "cpu/event=stalled_cycles_frontend/u",
+		.check = test__checkevent_pmu_events_stalled_cycles_frontend,
+	},
+	/* dash variants */
+	[10] = {
+		.name  = "cpu/event=branch-instructions/u",
+		.check = test__checkevent_pmu_events_branch_instructions,
+	},
+	[11] = {
+		.name  = "cpu/event=branch-misses/u",
+		.check = test__checkevent_pmu_events_branch_misses,
+	},
+	[12] = {
+		.name  = "cpu/event=bus-cycles/u",
+		.check = test__checkevent_pmu_events_bus_cycles,
+	},
+	[13] = {
+		.name  = "cpu/event=cache-misses/u",
+		.check = test__checkevent_pmu_events_cache_misses,
+	},
+	[14] = {
+		.name  = "cpu/event=cache-references/u",
+		.check = test__checkevent_pmu_events_cache_references,
+	},
+	[15] = {
+		.name  = "cpu/event=ref-cycles/u",
+		.check = test__checkevent_pmu_events_ref_cycles,
+	},
+	[16] = {
+		.name  = "cpu/event=stalled-cycles-backend/u",
+		.check = test__checkevent_pmu_events_stalled_cycles_backend,
+	},
+	[17] = {
+		.name  = "cpu/event=stalled-cycles-frontend/u",
+		.check = test__checkevent_pmu_events_stalled_cycles_frontend,
+	},
+};
+
 struct test__term {
 	const char *str;
 	__u32 type;
@@ -709,6 +824,21 @@ static int test_pmu(void)
 	return !ret;
 }
 
+static int test_pmu_events(void)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	int ret;
+
+	snprintf(path, PATH_MAX, "%s/bus/event_source/devices/cpu/events/",
+		 sysfs_find_mountpoint());
+
+	ret = stat(path, &st);
+	if (ret)
+		pr_debug("ommiting PMU cpu events tests\n");
+	return !ret;
+}
+
 int parse_events__test(void)
 {
 	int ret;
@@ -725,5 +855,8 @@ do {							\
 	if (test_pmu())
 		TEST_EVENTS(test__events_pmu);
 
+	if (test_pmu() && test_pmu_events())
+		TEST_EVENTS(test__events_pmu_events);
+
 	return test_terms(test__terms, ARRAY_SIZE(test__terms));
 }
-- 
1.7.10.4

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Peter Zijlstra]

On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> +#define PMU_EVENTS_ATTR_CONFIG(_name, _id)                             \
> +static ssize_t                                                         \
> +_name##_show(struct device *dev,                                       \
> +                          struct device_attribute *attr,               \
> +                          char *page)                                  \
> +{                                                                      \
> +       u64 val = x86_pmu.event_map(_id);                               \
> +       BUILD_BUG_ON(_id >= PERF_COUNT_HW_MAX);                         \
> +       return sprintf(page, "config=0x%llx\n", val);                   \
> +}                                                                      \ 

How about something like:

 u64 config = x86_pmu.event_map(_id);
 u64 event = (config & ARCH_PERFMON_EVENTSEL_EVENT) |
	     (config & AMD64_EVENTSEL_EVENT) >> 24;
 u64 umask = (config & ARCH_PERFMON_EVENTSEL_UMASK) >> 8;

 WARN_ON_ONCE(config & (ARCH_PERFMON_EVENTSEL_INV |
                        ARCH_PERFMON_EVENTSEL_CMASK));

 sprintf("event=0x%02llx,umask=0x%02llx\n", event, umask);

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Peter Zijlstra]

On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> +static struct device_attribute event_attr_##_name = __ATTR_RO(_name)

Note that it would now be trivial to allow writing to this file to
update the kernel table.

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Peter Zijlstra]

On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> +static struct attribute *events_attr[] = {
> +       &event_attr_cycles.attr,
> +       &event_attr_instructions.attr,
> +       &event_attr_cache_references.attr,
> +       &event_attr_cache_misses.attr,
> +       &event_attr_branch_instructions.attr,
> +       &event_attr_branch_misses.attr,
> +       &event_attr_bus_cycles.attr,
> +       &event_attr_stalled_cycles_frontend.attr,
> +       &event_attr_stalled_cycles_backend.attr,
> +       &event_attr_ref_cycles.attr,
> +       NULL,
> +}; 

Hmm, should we do:

  if (!config)
    return -EINVAL;

or somesuch to clearly indicate an event isn't supported?

Its currently a bit of a mixed bag between 0 and -1.. we might want to
clean that up too.

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Peter Zijlstra]

On Wed, 2012-07-04 at 12:24 +0200, Peter Zijlstra wrote:
> On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> > +static struct attribute *events_attr[] = {
> > +       &event_attr_cycles.attr,
> > +       &event_attr_instructions.attr,
> > +       &event_attr_cache_references.attr,
> > +       &event_attr_cache_misses.attr,
> > +       &event_attr_branch_instructions.attr,
> > +       &event_attr_branch_misses.attr,
> > +       &event_attr_bus_cycles.attr,
> > +       &event_attr_stalled_cycles_frontend.attr,
> > +       &event_attr_stalled_cycles_backend.attr,
> > +       &event_attr_ref_cycles.attr,
> > +       NULL,
> > +}; 
> 
> Hmm, should we do:
> 
>   if (!config)
>     return -EINVAL;
> 
> or somesuch to clearly indicate an event isn't supported?
> 
> Its currently a bit of a mixed bag between 0 and -1.. we might want to
> clean that up too.

Alternatively, we'd do something like:

  for (i = 0; events_attr[i]; i++) {
    if (x86_pmu.event_map(i))
      continue;

    for (j = i; events_attr[j]; j++)
      events_attr[j] = events_attr[j+1];
  }

On init to filter out all unset events so they don't even show up in
sysfs.
      

Re: [PATCH 02/10] perf, tool: Fix pmu object initialization

[Peter Zijlstra]

On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> The internal pmu list was never used. With each perf_pmu__find() call
> the pmu structure was created new by parsing sysfs. Beside this it
> caused memory leaks. We now keep all pmus by adding them to the list.
> 
> Also, pmu_lookup() should return pmus that do not expose the format
> specifier in sysfs.
> 
> We need a valid internal pmu list in a later patch to iterate over all
> pmus that exist in the system.
> 
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> [ added same treatment for 'event' sysfs group attribute ]
> Signed-off-by: Jiri Olsa <jolsa@redhat.com> 

Are we missing a From: ?

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Peter Zijlstra]

On Wed, 2012-07-04 at 12:22 +0200, Peter Zijlstra wrote:
> How about something like:
> 
>  u64 config = x86_pmu.event_map(_id);
>  u64 event = (config & ARCH_PERFMON_EVENTSEL_EVENT) |
>              (config & AMD64_EVENTSEL_EVENT) >> 24;
>  u64 umask = (config & ARCH_PERFMON_EVENTSEL_UMASK) >> 8;
> 
>  WARN_ON_ONCE(config & (ARCH_PERFMON_EVENTSEL_INV |
>                         ARCH_PERFMON_EVENTSEL_CMASK));
> 
>  sprintf("event=0x%02llx,umask=0x%02llx\n", event, umask); 

Oh wait, silly me, we actually need the inv and cmask too.

So that gets to be something like:

  u64 config = x86_pmu.event_map(_id);
  u64 event = (config & ARCH_PERFMON_EVENTSEL_EVENT) |
              (config & AMD64_EVENTSEL_EVENT) >> 24;
  u64 umask = (config & ARCH_PERFMON_EVENTSEL_UMASK) >> 8;
  u64 inv   = (config & ARCH_PERFMON_EVENTSEL_INV) >> 23;
  u64 cmask = (config & ARCH_PERFMON_EVENTSEL_CMASK) >> 24;
  ssize_t ret;

  ret = sprintf(page, "event=0x%02llx", event);
  if (umask)
    ret += sprintf(page + ret, ",umask=0x%02llx", umask);
  if (inv)
    ret += sprintf(page + ret, ",inv");
  if (cmask)
    ret += sprintf(page + ret, ",cmask=0x%02llx", cmask);
  sprintf(page + ret, "\n");
 
  return ret;

Re: [PATCH 07/10] perf, tool: Add support to specify hw event as pmu event term

[Peter Zijlstra]

On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> 
> It's possible to use sysfs attribute name as 'event' term value,
> or any possible alias for hw event, e.g.:
>   cycles                     OR cpu-cycles
>   branch_misses              OR branch-misses
>   bus_cycles                 OR bus-cycles
>   cache_misses               OR cache-misses
>   cache_references           OR cache-references
>   ref_cycles                 OR ref-cycles
>   stalled_cycles_backend     OR stalled-cycles-backend
>   stalled_cycles_frontend    OR stalled-cycles-frontend 

Do we really want to do that?

Re: [PATCH 08/10] perf, tool: Add sysfs read file interface

[Peter Zijlstra]

On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> +int sysfs_read_file(const char *file, char *buf, size_t size)
> +{
> +       char path[PATH_MAX];
> +       FILE *fp;
> +       int ret = 0;
> +
> +       snprintf(path, PATH_MAX, "%s/%s", sysfs_mountpoint, file);
> +
> +       fp = fopen(path, "r");
> +       if (!fp)
> +               return -errno;
> +
> +       if (fread(buf, 1, size, fp) == 0)
> +               ret = -errno;
> +
> +       fclose(fp);
> +       return ret; 

Does sysfs guarantee a single read is complete? If not, we need a while
loop around that read until EOF or size.

Re: [PATCH 02/10] perf, tool: Fix pmu object initialization

[Jiri Olsa]

On Wed, Jul 04, 2012 at 12:30:48PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> > The internal pmu list was never used. With each perf_pmu__find() call
> > the pmu structure was created new by parsing sysfs. Beside this it
> > caused memory leaks. We now keep all pmus by adding them to the list.
> > 
> > Also, pmu_lookup() should return pmus that do not expose the format
> > specifier in sysfs.
> > 
> > We need a valid internal pmu list in a later patch to iterate over all
> > pmus that exist in the system.
> > 
> > Signed-off-by: Robert Richter <robert.richter@amd.com>
> > [ added same treatment for 'event' sysfs group attribute ]
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com> 
> 
> Are we missing a From: ?

so first SOB is not enough then.. I'll add it in next version

jirka
> 

Re: [PATCH 02/10] perf, tool: Fix pmu object initialization

[Peter Zijlstra]

On Wed, 2012-07-04 at 13:40 +0200, Jiri Olsa wrote:
> On Wed, Jul 04, 2012 at 12:30:48PM +0200, Peter Zijlstra wrote:
> > On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> > > The internal pmu list was never used. With each perf_pmu__find() call
> > > the pmu structure was created new by parsing sysfs. Beside this it
> > > caused memory leaks. We now keep all pmus by adding them to the list.
> > > 
> > > Also, pmu_lookup() should return pmus that do not expose the format
> > > specifier in sysfs.
> > > 
> > > We need a valid internal pmu list in a later patch to iterate over all
> > > pmus that exist in the system.
> > > 
> > > Signed-off-by: Robert Richter <robert.richter@amd.com>
> > > [ added same treatment for 'event' sysfs group attribute ]
> > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> 
> > 
> > Are we missing a From: ?
> 
> so first SOB is not enough then.. I'll add it in next version

Not if the patch was written by Robert, in that case you need a From: or
Author: header on top so that git-am will know to attribute the patch to
Robert.

Re: [PATCH 07/10] perf, tool: Add support to specify hw event as pmu event term

[Jiri Olsa]

On Wed, Jul 04, 2012 at 12:39:20PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> > 
> > It's possible to use sysfs attribute name as 'event' term value,
> > or any possible alias for hw event, e.g.:
> >   cycles                     OR cpu-cycles
> >   branch_misses              OR branch-misses
> >   bus_cycles                 OR bus-cycles
> >   cache_misses               OR cache-misses
> >   cache_references           OR cache-references
> >   ref_cycles                 OR ref-cycles
> >   stalled_cycles_backend     OR stalled-cycles-backend
> >   stalled_cycles_frontend    OR stalled-cycles-frontend 
> 
> Do we really want to do that?

well, the aliasing works on sysfs filename base.. so when you specify
   event=str 

'str' file is looked up in 'events' dir.

We use '_' in file names and '-' in event symbol names. I thought it might be
confusing allowing just '_' so I added also the '-' version.

I could keep just the '-' version.. with some more work on kernel ATTR
function names ;) but it seems like common sysfs practise to use '_'.

jirka

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Jiri Olsa]

On Wed, Jul 04, 2012 at 12:28:58PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-07-04 at 12:24 +0200, Peter Zijlstra wrote:
> > On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> > > +static struct attribute *events_attr[] = {
> > > +       &event_attr_cycles.attr,
> > > +       &event_attr_instructions.attr,
> > > +       &event_attr_cache_references.attr,
> > > +       &event_attr_cache_misses.attr,
> > > +       &event_attr_branch_instructions.attr,
> > > +       &event_attr_branch_misses.attr,
> > > +       &event_attr_bus_cycles.attr,
> > > +       &event_attr_stalled_cycles_frontend.attr,
> > > +       &event_attr_stalled_cycles_backend.attr,
> > > +       &event_attr_ref_cycles.attr,
> > > +       NULL,
> > > +}; 
> > 
> > Hmm, should we do:
> > 
> >   if (!config)
> >     return -EINVAL;
> > 
> > or somesuch to clearly indicate an event isn't supported?
> > 
> > Its currently a bit of a mixed bag between 0 and -1.. we might want to
> > clean that up too.
> 
> Alternatively, we'd do something like:
> 
>   for (i = 0; events_attr[i]; i++) {
>     if (x86_pmu.event_map(i))
>       continue;
> 
>     for (j = i; events_attr[j]; j++)
>       events_attr[j] = events_attr[j+1];
>   }
> 
> On init to filter out all unset events so they don't even show up in
> sysfs.
>       
> 

ok

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Jiri Olsa]

On Wed, Jul 04, 2012 at 12:38:31PM +0200, Peter Zijlstra wrote:
> On Wed, 2012-07-04 at 12:22 +0200, Peter Zijlstra wrote:
> > How about something like:
> > 
> >  u64 config = x86_pmu.event_map(_id);
> >  u64 event = (config & ARCH_PERFMON_EVENTSEL_EVENT) |
> >              (config & AMD64_EVENTSEL_EVENT) >> 24;
> >  u64 umask = (config & ARCH_PERFMON_EVENTSEL_UMASK) >> 8;
> > 
> >  WARN_ON_ONCE(config & (ARCH_PERFMON_EVENTSEL_INV |
> >                         ARCH_PERFMON_EVENTSEL_CMASK));
> > 
> >  sprintf("event=0x%02llx,umask=0x%02llx\n", event, umask); 
> 
> Oh wait, silly me, we actually need the inv and cmask too.
> 
> So that gets to be something like:
> 
>   u64 config = x86_pmu.event_map(_id);
>   u64 event = (config & ARCH_PERFMON_EVENTSEL_EVENT) |
>               (config & AMD64_EVENTSEL_EVENT) >> 24;
>   u64 umask = (config & ARCH_PERFMON_EVENTSEL_UMASK) >> 8;
>   u64 inv   = (config & ARCH_PERFMON_EVENTSEL_INV) >> 23;
>   u64 cmask = (config & ARCH_PERFMON_EVENTSEL_CMASK) >> 24;
>   ssize_t ret;
> 
>   ret = sprintf(page, "event=0x%02llx", event);
>   if (umask)
>     ret += sprintf(page + ret, ",umask=0x%02llx", umask);
>   if (inv)
>     ret += sprintf(page + ret, ",inv");
>   if (cmask)
>     ret += sprintf(page + ret, ",cmask=0x%02llx", cmask);
>   sprintf(page + ret, "\n");
>  
>   return ret;
> 

ok

Re: [PATCH 07/10] perf, tool: Add support to specify hw event as pmu event term

[Peter Zijlstra]

On Wed, 2012-07-04 at 14:00 +0200, Jiri Olsa wrote:
> On Wed, Jul 04, 2012 at 12:39:20PM +0200, Peter Zijlstra wrote:
> > On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
> > > 
> > > It's possible to use sysfs attribute name as 'event' term value,
> > > or any possible alias for hw event, e.g.:
> > >   cycles                     OR cpu-cycles
> > >   branch_misses              OR branch-misses
> > >   bus_cycles                 OR bus-cycles
> > >   cache_misses               OR cache-misses
> > >   cache_references           OR cache-references
> > >   ref_cycles                 OR ref-cycles
> > >   stalled_cycles_backend     OR stalled-cycles-backend
> > >   stalled_cycles_frontend    OR stalled-cycles-frontend 
> > 
> > Do we really want to do that?
> 
> well, the aliasing works on sysfs filename base.. so when you specify
>    event=str 
> 
> 'str' file is looked up in 'events' dir.

Right..

> We use '_' in file names and '-' in event symbol names. I thought it might be
> confusing allowing just '_' so I added also the '-' version.
> 
> I could keep just the '-' version.. with some more work on kernel ATTR
> function names ;) but it seems like common sysfs practise to use '_'.

We could map '-' to '_', but I'm not sure about the aliases in general.
I mean if the sysfs event name is 'cycles' also allowing 'cpu_cycles' is
somewhat confusing and arbitrary.

I think it would be best to stick to discoverable names only. Anybody?

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Peter Zijlstra]

On Wed, 2012-07-04 at 14:01 +0200, Jiri Olsa wrote:
> >   ret = sprintf(page, "event=0x%02llx", event);
> >   if (umask)
> >     ret += sprintf(page + ret, ",umask=0x%02llx", umask);
> >   if (inv)
> >     ret += sprintf(page + ret, ",inv");
> >   if (cmask)
> >     ret += sprintf(page + ret, ",cmask=0x%02llx", cmask);
> >   sprintf(page + ret, "\n");

There's a ret += missing there.

Re: [PATCH 04/10] perf, x86: Making hardware events translations available in sysfs

[Arnaldo Carvalho de Melo]

Em Wed, Jul 04, 2012 at 02:14:48PM +0200, Peter Zijlstra escreveu:
> On Wed, 2012-07-04 at 14:01 +0200, Jiri Olsa wrote:
> > >   ret = sprintf(page, "event=0x%02llx", event);
> > >   if (umask)
> > >     ret += sprintf(page + ret, ",umask=0x%02llx", umask);
> > >   if (inv)
> > >     ret += sprintf(page + ret, ",inv");
> > >   if (cmask)
> > >     ret += sprintf(page + ret, ",cmask=0x%02llx", cmask);
> > >   sprintf(page + ret, "\n");
> 
> There's a ret += missing there.

And also don't use sprintf or snprintf, use scnprintf, the former have
confusing semantics for the return value, see the man pages.

- Arnaldo

Re: [PATCH 02/10] perf, tool: Fix pmu object initialization

[Arnaldo Carvalho de Melo]

Em Wed, Jul 04, 2012 at 12:00:40AM +0200, Jiri Olsa escreveu:
> The internal pmu list was never used. With each perf_pmu__find() call
> the pmu structure was created new by parsing sysfs. Beside this it
> caused memory leaks. We now keep all pmus by adding them to the list.
> 
> Also, pmu_lookup() should return pmus that do not expose the format
> specifier in sysfs.
> 
> We need a valid internal pmu list in a later patch to iterate over all
> pmus that exist in the system.
> 
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> [ added same treatment for 'event' sysfs group attribute ]

The original patch from Robert is already in my perf/core branch, please
resubmit with just your additional code.

- Arnaldo

> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
>  tools/perf/util/pmu.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 74d0948e..f1f83e6 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -72,7 +72,7 @@ static int pmu_format(char *name, struct list_head *format)
>  		 "%s/bus/event_source/devices/%s/format", sysfs, name);
>  
>  	if (stat(path, &st) < 0)
> -		return -1;
> +		return 0;	/* no error if 'format' does not exist */
>  
>  	if (pmu_format_parse(path, format))
>  		return -1;
> @@ -161,7 +161,7 @@ static int pmu_aliases(char *name, struct list_head *head)
>  		 "%s/bus/event_source/devices/%s/events", sysfs, name);
>  
>  	if (stat(path, &st) < 0)
> -		return -1;
> +		return 0;	/* no error if 'events' does not exist */
>  
>  	if (pmu_aliases_parse(path, head))
>  		return -1;
> @@ -237,6 +237,9 @@ static struct perf_pmu *pmu_lookup(char *name)
>  	if (pmu_format(name, &format))
>  		return NULL;
>  
> +	if (pmu_aliases(name, &aliases))
> +		return NULL;
> +
>  	if (pmu_type(name, &type))
>  		return NULL;
>  
> @@ -244,14 +247,13 @@ static struct perf_pmu *pmu_lookup(char *name)
>  	if (!pmu)
>  		return NULL;
>  
> -	pmu_aliases(name, &aliases);
> -
>  	INIT_LIST_HEAD(&pmu->format);
>  	INIT_LIST_HEAD(&pmu->aliases);
>  	list_splice(&format, &pmu->format);
>  	list_splice(&aliases, &pmu->aliases);
>  	pmu->name = strdup(name);
>  	pmu->type = type;
> +	list_add_tail(&pmu->list, &pmus);
>  	return pmu;
>  }
>  
> -- 
> 1.7.10.4

Re: [PATCH 07/10] perf, tool: Add support to specify hw event as pmu event term

[Stephane Eranian]

On Wed, Jul 4, 2012 at 2:13 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Wed, 2012-07-04 at 14:00 +0200, Jiri Olsa wrote:
>> On Wed, Jul 04, 2012 at 12:39:20PM +0200, Peter Zijlstra wrote:
>> > On Wed, 2012-07-04 at 00:00 +0200, Jiri Olsa wrote:
>> > >
>> > > It's possible to use sysfs attribute name as 'event' term value,
>> > > or any possible alias for hw event, e.g.:
>> > >   cycles                     OR cpu-cycles
>> > >   branch_misses              OR branch-misses
>> > >   bus_cycles                 OR bus-cycles
>> > >   cache_misses               OR cache-misses
>> > >   cache_references           OR cache-references
>> > >   ref_cycles                 OR ref-cycles
>> > >   stalled_cycles_backend     OR stalled-cycles-backend
>> > >   stalled_cycles_frontend    OR stalled-cycles-frontend
>> >
>> > Do we really want to do that?
>>
>> well, the aliasing works on sysfs filename base.. so when you specify
>>    event=str
>>
>> 'str' file is looked up in 'events' dir.
>
> Right..
>
>> We use '_' in file names and '-' in event symbol names. I thought it might be
>> confusing allowing just '_' so I added also the '-' version.
>>
What's wrong with keeping the - in the filenames as well? That would help
keep things simple. That would avoid yet another layer of aliases.


>> I could keep just the '-' version.. with some more work on kernel ATTR
>> function names ;) but it seems like common sysfs practise to use '_'.
>
> We could map '-' to '_', but I'm not sure about the aliases in general.
> I mean if the sysfs event name is 'cycles' also allowing 'cpu_cycles' is
> somewhat confusing and arbitrary.
>
> I think it would be best to stick to discoverable names only. Anybody?
>
>

[tip:perf/core] perf test: Use ARRAY_SIZE in parse events tests

[tip-bot for Jiri Olsa]

Commit-ID:  ebf124ffab59e9e1c6179347fe95fe5baf32dcd7
Gitweb:     http://git.kernel.org/tip/ebf124ffab59e9e1c6179347fe95fe5baf32dcd7
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 4 Jul 2012 00:00:47 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 4 Jul 2012 11:31:26 -0300

perf test: Use ARRAY_SIZE in parse events tests

Use ARRAY_SIZE instead of defining the sizes separately for each test
arrays.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1341352848-11833-10-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events-test.c |   29 ++++++++++-------------------
 1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/parse-events-test.c b/tools/perf/util/parse-events-test.c
index dd0c306..1b997d2 100644
--- a/tools/perf/util/parse-events-test.c
+++ b/tools/perf/util/parse-events-test.c
@@ -634,8 +634,6 @@ static struct test__event_st test__events[] = {
 	},
 };
 
-#define TEST__EVENTS_CNT (sizeof(test__events) / sizeof(struct test__event_st))
-
 static struct test__event_st test__events_pmu[] = {
 	[0] = {
 		.name  = "cpu/config=10,config1,config2=3,period=1000/u",
@@ -647,9 +645,6 @@ static struct test__event_st test__events_pmu[] = {
 	},
 };
 
-#define TEST__EVENTS_PMU_CNT (sizeof(test__events_pmu) / \
-			      sizeof(struct test__event_st))
-
 struct test__term {
 	const char *str;
 	__u32 type;
@@ -765,21 +760,17 @@ int parse_events__test(void)
 {
 	int ret;
 
-	do {
-		ret = test_events(test__events, TEST__EVENTS_CNT);
-		if (ret)
-			break;
-
-		if (test_pmu()) {
-			ret = test_events(test__events_pmu,
-					  TEST__EVENTS_PMU_CNT);
-			if (ret)
-				break;
-		}
+#define TEST_EVENTS(tests)				\
+do {							\
+	ret = test_events(tests, ARRAY_SIZE(tests));	\
+	if (ret)					\
+		return ret;				\
+} while (0)
 
-		ret = test_terms(test__terms, TEST__TERMS_CNT);
+	TEST_EVENTS(test__events);
 
-	} while (0);
+	if (test_pmu())
+		TEST_EVENTS(test__events_pmu);
 
-	return ret;
+	return test_terms(test__terms, ARRAY_SIZE(test__terms));
 }

[tip:perf/core] perf tools: Add empty rule for new line in event syntax parsing

[tip-bot for Jiri Olsa]

Commit-ID:  8c5f0a84c6b16e04195001bfd78281909519cf09
Gitweb:     http://git.kernel.org/tip/8c5f0a84c6b16e04195001bfd78281909519cf09
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 4 Jul 2012 00:00:39 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Jul 2012 11:22:29 -0300

perf tools: Add empty rule for new line in event syntax parsing

The flex generator prints out each input character that is ignored by
lex rules.

Since the alias processing, we can have '\n' characters on input. We
need to assign empty rule to it, so it's not printed out.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1341352848-11833-2-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.l |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index a066894..6cbe092 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -152,6 +152,7 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 ,			{ return ','; }
 :			{ return ':'; }
 =			{ return '='; }
+\n			{ }
 
 <mem>{
 {modifier_bp}		{ return str(yyscanner, PE_MODIFIER_BP); }

[tip:perf/core] perf tools: Split out PE_VALUE_SYM parsing token to SW and HW tokens

[tip-bot for Jiri Olsa]

Commit-ID:  cf3506dcc4a855d11af20bcb271ac921033ba0de
Gitweb:     http://git.kernel.org/tip/cf3506dcc4a855d11af20bcb271ac921033ba0de
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 4 Jul 2012 00:00:43 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Jul 2012 11:29:56 -0300

perf tools: Split out PE_VALUE_SYM parsing token to SW and HW tokens

Spliting PE_VALUE_SYM token to PE_VALUE_SYM_HW and PE_VALUE_SYM_SW
tokens to separate hardware and software symbols.

This will be useful in upcomming patch where we want to be able to parse
out only hardware events.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1341352848-11833-6-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.l |    2 +-
 tools/perf/util/parse-events.y |   15 +++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 6cbe092..384ca74 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -56,7 +56,7 @@ static int sym(yyscan_t scanner, int type, int config)
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
 
 	yylval->num = (type << 16) + config;
-	return PE_VALUE_SYM;
+	return type == PERF_TYPE_HARDWARE ? PE_VALUE_SYM_HW : PE_VALUE_SYM_SW;
 }
 
 static int term(yyscan_t scanner, int type)
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 9525c45..2bc5fbf 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -26,14 +26,15 @@ do { \
 %}
 
 %token PE_START_EVENTS PE_START_TERMS
-%token PE_VALUE PE_VALUE_SYM PE_RAW PE_TERM
+%token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_RAW PE_TERM
 %token PE_NAME
 %token PE_MODIFIER_EVENT PE_MODIFIER_BP
 %token PE_NAME_CACHE_TYPE PE_NAME_CACHE_OP_RESULT
 %token PE_PREFIX_MEM PE_PREFIX_RAW
 %token PE_ERROR
 %type <num> PE_VALUE
-%type <num> PE_VALUE_SYM
+%type <num> PE_VALUE_SYM_HW
+%type <num> PE_VALUE_SYM_SW
 %type <num> PE_RAW
 %type <num> PE_TERM
 %type <str> PE_NAME
@@ -41,6 +42,7 @@ do { \
 %type <str> PE_NAME_CACHE_OP_RESULT
 %type <str> PE_MODIFIER_EVENT
 %type <str> PE_MODIFIER_BP
+%type <num> value_sym
 %type <head> event_config
 %type <term> event_term
 %type <head> event_pmu
@@ -109,8 +111,13 @@ PE_NAME '/' event_config '/'
 	$$ = list;
 }
 
+value_sym:
+PE_VALUE_SYM_HW
+|
+PE_VALUE_SYM_SW
+
 event_legacy_symbol:
-PE_VALUE_SYM '/' event_config '/'
+value_sym '/' event_config '/'
 {
 	struct parse_events_data__events *data = _data;
 	struct list_head *list = NULL;
@@ -123,7 +130,7 @@ PE_VALUE_SYM '/' event_config '/'
 	$$ = list;
 }
 |
-PE_VALUE_SYM sep_slash_dc
+value_sym sep_slash_dc
 {
 	struct parse_events_data__events *data = _data;
 	struct list_head *list = NULL;

[tip:perf/core] perf tools: Split event symbols arrays to hw and sw parts

[tip-bot for Jiri Olsa]

Commit-ID:  1dc12760854a670d0366dcfd8ad179e3574c1712
Gitweb:     http://git.kernel.org/tip/1dc12760854a670d0366dcfd8ad179e3574c1712
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 4 Jul 2012 00:00:44 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 5 Jul 2012 11:31:13 -0300

perf tools: Split event symbols arrays to hw and sw parts

It'll be convenient in upcoming patch to access hw event symbols
strings via enum perf_hw_id indexes. In order not to duplicate
the data, creating two separate arrays:

  event_symbols_hw for enum perf_hw_id events
  event_symbols_sw for enum perf_sw_ids events

Changing the current event list code to follow the change.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1341352848-11833-7-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c |  174 +++++++++++++++++++++++++++-------------
 1 files changed, 117 insertions(+), 57 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1dc44dc..1aa721d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -19,8 +19,6 @@
 #define MAX_NAME_LEN 100
 
 struct event_symbol {
-	u8		type;
-	u64		config;
 	const char	*symbol;
 	const char	*alias;
 };
@@ -30,30 +28,86 @@ extern int parse_events_debug;
 #endif
 int parse_events_parse(void *data, void *scanner);
 
-#define CHW(x) .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_##x
-#define CSW(x) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_##x
-
-static struct event_symbol event_symbols[] = {
-  { CHW(CPU_CYCLES),			"cpu-cycles",			"cycles"		},
-  { CHW(STALLED_CYCLES_FRONTEND),	"stalled-cycles-frontend",	"idle-cycles-frontend"	},
-  { CHW(STALLED_CYCLES_BACKEND),	"stalled-cycles-backend",	"idle-cycles-backend"	},
-  { CHW(INSTRUCTIONS),			"instructions",			""			},
-  { CHW(CACHE_REFERENCES),		"cache-references",		""			},
-  { CHW(CACHE_MISSES),			"cache-misses",			""			},
-  { CHW(BRANCH_INSTRUCTIONS),		"branch-instructions",		"branches"		},
-  { CHW(BRANCH_MISSES),			"branch-misses",		""			},
-  { CHW(BUS_CYCLES),			"bus-cycles",			""			},
-  { CHW(REF_CPU_CYCLES),		"ref-cycles",			""			},
-
-  { CSW(CPU_CLOCK),			"cpu-clock",			""			},
-  { CSW(TASK_CLOCK),			"task-clock",			""			},
-  { CSW(PAGE_FAULTS),			"page-faults",			"faults"		},
-  { CSW(PAGE_FAULTS_MIN),		"minor-faults",			""			},
-  { CSW(PAGE_FAULTS_MAJ),		"major-faults",			""			},
-  { CSW(CONTEXT_SWITCHES),		"context-switches",		"cs"			},
-  { CSW(CPU_MIGRATIONS),		"cpu-migrations",		"migrations"		},
-  { CSW(ALIGNMENT_FAULTS),		"alignment-faults",		""			},
-  { CSW(EMULATION_FAULTS),		"emulation-faults",		""			},
+static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
+	[PERF_COUNT_HW_CPU_CYCLES] = {
+		.symbol = "cpu-cycles",
+		.alias  = "cycles",
+	},
+	[PERF_COUNT_HW_INSTRUCTIONS] = {
+		.symbol = "instructions",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_CACHE_REFERENCES] = {
+		.symbol = "cache-references",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_CACHE_MISSES] = {
+		.symbol = "cache-misses",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = {
+		.symbol = "branch-instructions",
+		.alias  = "branches",
+	},
+	[PERF_COUNT_HW_BRANCH_MISSES] = {
+		.symbol = "branch-misses",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_BUS_CYCLES] = {
+		.symbol = "bus-cycles",
+		.alias  = "",
+	},
+	[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = {
+		.symbol = "stalled-cycles-frontend",
+		.alias  = "idle-cycles-frontend",
+	},
+	[PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = {
+		.symbol = "stalled-cycles-backend",
+		.alias  = "idle-cycles-backend",
+	},
+	[PERF_COUNT_HW_REF_CPU_CYCLES] = {
+		.symbol = "ref-cycles",
+		.alias  = "",
+	},
+};
+
+static struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
+	[PERF_COUNT_SW_CPU_CLOCK] = {
+		.symbol = "cpu-clock",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_TASK_CLOCK] = {
+		.symbol = "task-clock",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_PAGE_FAULTS] = {
+		.symbol = "page-faults",
+		.alias  = "faults",
+	},
+	[PERF_COUNT_SW_CONTEXT_SWITCHES] = {
+		.symbol = "context-switches",
+		.alias  = "cs",
+	},
+	[PERF_COUNT_SW_CPU_MIGRATIONS] = {
+		.symbol = "cpu-migrations",
+		.alias  = "migrations",
+	},
+	[PERF_COUNT_SW_PAGE_FAULTS_MIN] = {
+		.symbol = "minor-faults",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_PAGE_FAULTS_MAJ] = {
+		.symbol = "major-faults",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_ALIGNMENT_FAULTS] = {
+		.symbol = "alignment-faults",
+		.alias  = "",
+	},
+	[PERF_COUNT_SW_EMULATION_FAULTS] = {
+		.symbol = "emulation-faults",
+		.alias  = "",
+	},
 };
 
 #define __PERF_EVENT_FIELD(config, name) \
@@ -824,16 +878,13 @@ int is_valid_tracepoint(const char *event_string)
 	return 0;
 }
 
-void print_events_type(u8 type)
+static void __print_events_type(u8 type, struct event_symbol *syms,
+				unsigned max)
 {
-	struct event_symbol *syms = event_symbols;
-	unsigned int i;
 	char name[64];
+	unsigned i;
 
-	for (i = 0; i < ARRAY_SIZE(event_symbols); i++, syms++) {
-		if (type != syms->type)
-			continue;
-
+	for (i = 0; i < max ; i++, syms++) {
 		if (strlen(syms->alias))
 			snprintf(name, sizeof(name),  "%s OR %s",
 				 syms->symbol, syms->alias);
@@ -845,6 +896,14 @@ void print_events_type(u8 type)
 	}
 }
 
+void print_events_type(u8 type)
+{
+	if (type == PERF_TYPE_SOFTWARE)
+		__print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
+	else
+		__print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
+}
+
 int print_hwcache_events(const char *event_glob)
 {
 	unsigned int type, op, i, printed = 0;
@@ -872,26 +931,13 @@ int print_hwcache_events(const char *event_glob)
 	return printed;
 }
 
-/*
- * Print the help text for the event symbols:
- */
-void print_events(const char *event_glob)
+static void print_symbol_events(const char *event_glob, unsigned type,
+				struct event_symbol *syms, unsigned max)
 {
-	unsigned int i, type, prev_type = -1, printed = 0, ntypes_printed = 0;
-	struct event_symbol *syms = event_symbols;
+	unsigned i, printed = 0;
 	char name[MAX_NAME_LEN];
 
-	printf("\n");
-	printf("List of pre-defined events (to be used in -e):\n");
-
-	for (i = 0; i < ARRAY_SIZE(event_symbols); i++, syms++) {
-		type = syms->type;
-
-		if (type != prev_type && printed) {
-			printf("\n");
-			printed = 0;
-			ntypes_printed++;
-		}
+	for (i = 0; i < max; i++, syms++) {
 
 		if (event_glob != NULL && 
 		    !(strglobmatch(syms->symbol, event_glob) ||
@@ -902,17 +948,31 @@ void print_events(const char *event_glob)
 			snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
 		else
 			strncpy(name, syms->symbol, MAX_NAME_LEN);
-		printf("  %-50s [%s]\n", name,
-			event_type_descriptors[type]);
 
-		prev_type = type;
-		++printed;
+		printf("  %-50s [%s]\n", name, event_type_descriptors[type]);
+
+		printed++;
 	}
 
-	if (ntypes_printed) {
-		printed = 0;
+	if (printed)
 		printf("\n");
-	}
+}
+
+/*
+ * Print the help text for the event symbols:
+ */
+void print_events(const char *event_glob)
+{
+
+	printf("\n");
+	printf("List of pre-defined events (to be used in -e):\n");
+
+	print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
+			    event_symbols_hw, PERF_COUNT_HW_MAX);
+
+	print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
+			    event_symbols_sw, PERF_COUNT_SW_MAX);
+
 	print_hwcache_events(event_glob);
 
 	if (event_glob != NULL)

Re: [PATCH 07/10] perf, tool: Add support to specify hw event as pmu event term

[Peter Zijlstra]

On Fri, 2012-07-06 at 03:08 +0200, Stephane Eranian wrote:
> >> We use '_' in file names and '-' in event symbol names. I thought it might be
> >> confusing allowing just '_' so I added also the '-' version.
> >>
> What's wrong with keeping the - in the filenames as well? That would help
> keep things simple. That would avoid yet another layer of aliases. 

No idea, my fingers prefer _ but it doesn't really matter all that much.

# find /sys | grep _ | wc -l
11753
# find /sys | grep - | wc -l
3700


So sysfs prefers _ as well, but there's enough - to not be the first ;-)