* [PATCH v4 1/3] Revert "perf tools: Default to cpu// for events v5"
@ 2014-09-02 15:29 kan.liang
2014-09-02 15:29 ` [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix kan.liang
2014-09-02 15:29 ` [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event kan.liang
0 siblings, 2 replies; 8+ messages in thread
From: kan.liang @ 2014-09-02 15:29 UTC (permalink / raw)
To: acme, jolsa; +Cc: linux-kernel, ak, 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 e34c81a..75e9ebe 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"
@@ -853,32 +853,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;
@@ -899,8 +873,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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix
2014-09-02 15:29 [PATCH v4 1/3] Revert "perf tools: Default to cpu// for events v5" kan.liang
@ 2014-09-02 15:29 ` kan.liang
2014-09-06 19:38 ` Jiri Olsa
2014-09-06 19:39 ` Jiri Olsa
2014-09-02 15:29 ` [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event kan.liang
1 sibling, 2 replies; 8+ messages in thread
From: kan.liang @ 2014-09-02 15:29 UTC (permalink / raw)
To: acme, jolsa; +Cc: linux-kernel, ak, 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 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.
Signed-off-by: Kan Liang <kan.liang@intel.com>
---
v2: Read kernel PMU events from sysfs at runtime
v3: Use strlcpy to replace strncpyv2: Read kernel PMU events from sysfs at runtime
v4: rebase to git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core
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(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 75e9ebe..9112413 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -30,6 +30,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",
@@ -853,6 +856,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;
@@ -906,7 +1006,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);
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index df094b4..d06fec4 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -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);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9bf5827..16d5c1a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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; /* HEAD struct parse_events_term -> list */
- struct list_head list; /* ELEM */
- char unit[UNIT_MAX_LEN+1];
- double scale;
-};
-
struct perf_pmu_format {
char *name;
int value;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 1c1e2ee..8adf27d 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -22,6 +22,16 @@ struct perf_pmu {
struct list_head list; /* ELEM */
};
+#define UNIT_MAX_LEN 31 /* max length for event unit name */
+
+struct perf_pmu_alias {
+ char *name;
+ struct list_head terms; /* HEAD struct parse_events_term -> list */
+ struct list_head list; /* ELEM */
+ 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);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event
2014-09-02 15:29 [PATCH v4 1/3] Revert "perf tools: Default to cpu// for events v5" kan.liang
2014-09-02 15:29 ` [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix kan.liang
@ 2014-09-02 15:29 ` kan.liang
2014-09-06 19:39 ` Jiri Olsa
2014-09-06 19:39 ` Jiri Olsa
1 sibling, 2 replies; 8+ messages in thread
From: kan.liang @ 2014-09-02 15:29 UTC (permalink / raw)
To: acme, jolsa; +Cc: linux-kernel, ak, 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 | 30 +++++++++++++++++++++++++++++-
tools/perf/util/parse-events.y | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 3432995..4dd7f04 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -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 ','; }
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix
2014-09-02 15:29 ` [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix kan.liang
@ 2014-09-06 19:38 ` Jiri Olsa
2014-09-06 19:39 ` Jiri Olsa
1 sibling, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2014-09-06 19:38 UTC (permalink / raw)
To: kan.liang; +Cc: acme, linux-kernel, ak
On Tue, Sep 02, 2014 at 11:29:29AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
SNIP
> + * 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) {
Why do we need to call scan here? Looks like:
pmu = pmu_lookup("cpu")
should be enough.. and could be used below as well
> + 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);
> +
> +}
SNIP
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix
2014-09-02 15:29 ` [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix kan.liang
2014-09-06 19:38 ` Jiri Olsa
@ 2014-09-06 19:39 ` Jiri Olsa
1 sibling, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2014-09-06 19:39 UTC (permalink / raw)
To: kan.liang; +Cc: acme, linux-kernel, ak
On Tue, Sep 02, 2014 at 11:29:29AM -0400, kan.liang@intel.com wrote:
SNIP
> {
> YY_BUFFER_STATE buffer;
> @@ -906,7 +1006,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);
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index df094b4..d06fec4 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -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
hum.. so roughly for 15 (+-) strings of size around 20 bytes we will
use 15K of memory.. seems like overkill
seems better to allocate each symbol string separatelly, and
update the release function
> +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;
> +};
> +
also, I think this is more pmu object related stuff.. how about:
enum perf_pmu_event_symbol_type
struct perf_pmu_event_symbol
perf_pmu__parse_init
perf_pmu__parse_cleanup
perf_pmu__parse_check
with perf_pmu__parse_init being called from perf_pmu__parse_check
in case it's needed..
and perf_pmu__parse_cleanup being called from parse_events same as
release_kernel_pmu_events_list
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event
2014-09-02 15:29 ` [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event kan.liang
@ 2014-09-06 19:39 ` Jiri Olsa
2014-09-06 19:39 ` Jiri Olsa
1 sibling, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2014-09-06 19:39 UTC (permalink / raw)
To: kan.liang; +Cc: acme, linux-kernel, ak
On Tue, Sep 02, 2014 at 11:29:30AM -0400, kan.liang@intel.com wrote:
SNIP
> +++ 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));
could you please use ALLOC_LIST(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));
same here
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event
2014-09-02 15:29 ` [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event kan.liang
2014-09-06 19:39 ` Jiri Olsa
@ 2014-09-06 19:39 ` Jiri Olsa
2014-09-08 15:09 ` Liang, Kan
1 sibling, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2014-09-06 19:39 UTC (permalink / raw)
To: kan.liang; +Cc: acme, linux-kernel, ak
On Tue, Sep 02, 2014 at 11:29:30AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
SNIP
> }
> +|
> +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
so, how about the event alias has more than 2 parts a-b-c ?
if I remove 'stalled-cycles-frontend' from lexer (patch attached),
I'll get error parsing it with this code:
---
[jolsa@krava perf]$ ls -l /sys/devices/cpu/events/stalled-cycles-frontend
-r--r--r-- 1 root root 4096 Sep 6 21:17 /sys/devices/cpu/events/stalled-cycles-frontend
[jolsa@krava perf]$ ./perf record -e stalled-cycles-frontend ls
invalid or unsupported event: 'stalled-cycles-frontend'
Run 'perf list' for a list of valid events
usage: perf record [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>]
-e, --event <event> event selector. use 'perf list' to list available events
---
and.. if we will actually handle this correctly, I think maybe we want
to remove all the PERF_TYPE_HARDWARE equivalents?
thanks,
jirka
---
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4dd7f0467dd1..c1a354f651ca 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -176,8 +176,6 @@ branch_type { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE
}
cpu-cycles|cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES); }
-stalled-cycles-frontend|idle-cycles-frontend { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
-stalled-cycles-backend|idle-cycles-backend { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
instructions { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS); }
cache-references { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_REFERENCES); }
cache-misses { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_MISSES); }
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event
2014-09-06 19:39 ` Jiri Olsa
@ 2014-09-08 15:09 ` Liang, Kan
0 siblings, 0 replies; 8+ messages in thread
From: Liang, Kan @ 2014-09-08 15:09 UTC (permalink / raw)
To: Jiri Olsa; +Cc: acme, linux-kernel, ak
>
> On Tue, Sep 02, 2014 at 11:29:30AM -0400, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
>
> SNIP
>
> > }
> > +|
> > +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
>
> so, how about the event alias has more than 2 parts a-b-c ?
>
Currently, the patch only handle the format for "a-b" or "a". I think it's enough for current usage.
The event alias with more than 2 parts a-b-c are already handled by hardcode in the lexer.
So I don't think we really need to support a-b-c format now.
> if I remove 'stalled-cycles-frontend' from lexer (patch attached), I'll get error
> parsing it with this code:
>
> ---
> [jolsa@krava perf]$ ls -l /sys/devices/cpu/events/stalled-cycles-frontend
> -r--r--r-- 1 root root 4096 Sep 6 21:17 /sys/devices/cpu/events/stalled-
> cycles-frontend
> [jolsa@krava perf]$ ./perf record -e stalled-cycles-frontend ls invalid or
> unsupported event: 'stalled-cycles-frontend'
> Run 'perf list' for a list of valid events
>
> usage: perf record [<options>] [<command>]
> or: perf record [<options>] -- <command> [<options>]
>
> -e, --event <event> event selector. use 'perf list' to list available events
> ---
>
> and.. if we will actually handle this correctly, I think maybe we want to
> remove all the equivalents?
Yes, there will be error if 'stalled-cycles-frontend' from lexer is removed. It's because not only the unsupported a-b-c format but also the middle name cycles. (Cycles could be individual event name or prefix/suffix of many event names. Lexer cannot distinguish them.)
However, why we want to remove it?
The current codes work well. PERF_TYPE_HARDWARE events are seldom changed. So I think it's OK to keep them unchanged.
We only need to handle possible extended events. (Actually, the new PMU events are not frequently added. I think "a-b" format is enough.)
Thanks,
Kan
>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 4dd7f0467dd1..c1a354f651ca 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -176,8 +176,6 @@ branch_type { return term(yyscanner,
> PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE
> }
>
> cpu-cycles|cycles { return sym(yyscanner,
> PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES); }
> -stalled-cycles-frontend|idle-cycles-frontend { return sym(yyscanner,
> PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
> -stalled-cycles-backend|idle-cycles-backend { return sym(yyscanner,
> PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
> instructions { return sym(yyscanner,
> PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS); }
> cache-references { return sym(yyscanner,
> PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_REFERENCES); }
> cache-misses { return sym(yyscanner,
> PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_MISSES); }
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-08 15:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-02 15:29 [PATCH v4 1/3] Revert "perf tools: Default to cpu// for events v5" kan.liang
2014-09-02 15:29 ` [PATCH v4 2/3] perf tools: parse the pmu event prefix and surfix kan.liang
2014-09-06 19:38 ` Jiri Olsa
2014-09-06 19:39 ` Jiri Olsa
2014-09-02 15:29 ` [PATCH v4 3/3] perf tools: Add support to new style format of kernel PMU event kan.liang
2014-09-06 19:39 ` Jiri Olsa
2014-09-06 19:39 ` Jiri Olsa
2014-09-08 15:09 ` Liang, Kan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).