As discussed in:
https://lore.kernel.org/lkml/[email protected]/
preferring sysfs/json events consistently (with or without a given
PMU) will enable RISC-V's hope to customize legacy events in the perf
tool.
Some minor clean-up is performed on the way.
v2. Additional cleanup particularly adding better error messages. Fix
some line length issues on the earlier patches.
Ian Rogers (16):
perf parse-events: Factor out '<event_or_pmu>/.../' parsing
perf parse-events: Directly pass PMU to parse_events_add_pmu
perf parse-events: Avoid copying an empty list
perf pmu: Refactor perf_pmu__match
perf tests parse-events: Use branches rather than cache-references
perf parse-events: Legacy cache names on all PMUs and lower priority
perf parse-events: Handle PE_TERM_HW in name_or_raw
perf parse-events: Constify parse_events_add_numeric
perf parse-events: Prefer sysfs/json hardware events over legacy
perf parse-events: Inline parse_events_update_lists
perf parse-events: Improve error message for bad numbers
perf parse-events: Inline parse_events_evlist_error
perf parse-events: Improvements to modifier parsing
perf parse-event: Constify event_symbol arrays
perf parse-events: Minor grouping tidy up
perf parse-events: Tidy the setting of the default event name
tools/perf/tests/parse-events.c | 6 +-
tools/perf/util/parse-events.c | 482 ++++++++++++++++----------------
tools/perf/util/parse-events.h | 49 ++--
tools/perf/util/parse-events.l | 196 +++++++++----
tools/perf/util/parse-events.y | 261 +++++++----------
tools/perf/util/pmu.c | 27 +-
tools/perf/util/pmu.h | 2 +-
7 files changed, 540 insertions(+), 483 deletions(-)
--
2.44.0.683.g7961c838ac-goog
Factor out the case of an event or PMU name followed by a slash based
term list. This is with a view to sharing the code with new legacy
hardware parsing. Use early return to reduce indentation in the code.
Make parse_events_add_pmu static now it doesn't need sharing with
parse-events.y.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 70 +++++++++++++++++++++++++++++++-
tools/perf/util/parse-events.h | 10 +++--
tools/perf/util/parse-events.y | 73 +++-------------------------------
3 files changed, 80 insertions(+), 73 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6f8b0fa17689..a6f71165ee1a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1385,7 +1385,7 @@ static bool config_term_percore(struct list_head *config_terms)
return false;
}
-int parse_events_add_pmu(struct parse_events_state *parse_state,
+static int parse_events_add_pmu(struct parse_events_state *parse_state,
struct list_head *list, const char *name,
const struct parse_events_terms *const_parsed_terms,
bool auto_merge_stats, void *loc_)
@@ -1618,6 +1618,74 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
return ok ? 0 : -1;
}
+int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state,
+ const char *event_or_pmu,
+ const struct parse_events_terms *const_parsed_terms,
+ struct list_head **listp,
+ void *loc_)
+{
+ char *pattern = NULL;
+ YYLTYPE *loc = loc_;
+ struct perf_pmu *pmu = NULL;
+ int ok = 0;
+ char *help;
+
+ *listp = malloc(sizeof(**listp));
+ if (!*listp)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(*listp);
+
+ /* Attempt to add to list assuming event_or_pmu is a PMU name. */
+ if (!parse_events_add_pmu(parse_state, *listp, event_or_pmu, const_parsed_terms,
+ /*auto_merge_stats=*/false, loc))
+ return 0;
+
+ /* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
+ if (asprintf(&pattern, "%s*", event_or_pmu) < 0) {
+ zfree(listp);
+ return -ENOMEM;
+ }
+
+ while ((pmu = perf_pmus__scan(pmu)) != NULL) {
+ const char *name = pmu->name;
+
+ if (parse_events__filter_pmu(parse_state, pmu))
+ continue;
+
+ if (!strncmp(name, "uncore_", 7) &&
+ strncmp(event_or_pmu, "uncore_", 7))
+ name += 7;
+ if (!perf_pmu__match(pattern, name, event_or_pmu) ||
+ !perf_pmu__match(pattern, pmu->alias_name, event_or_pmu)) {
+ bool auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
+
+ if (!parse_events_add_pmu(parse_state, *listp, pmu->name,
+ const_parsed_terms,
+ auto_merge_stats, loc)) {
+ ok++;
+ parse_state->wild_card_pmus = true;
+ }
+ }
+ }
+ zfree(&pattern);
+ if (ok)
+ return 0;
+
+ /* Failure to add, assume event_or_pmu is an event name. */
+ zfree(listp);
+ if (!parse_events_multi_pmu_add(parse_state, event_or_pmu, const_parsed_terms, listp, loc))
+ return 0;
+
+ if (asprintf(&help, "Unable to find PMU or event on a PMU of '%s'", event_or_pmu) < 0)
+ help = NULL;
+ parse_events_error__handle(parse_state->error, loc->first_column,
+ strdup("Bad event or PMU"),
+ help);
+ zfree(listp);
+ return -EINVAL;
+}
+
int parse_events__modifier_group(struct list_head *list,
char *event_mod)
{
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 809359e8544e..a331b9f0da2b 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -209,10 +209,6 @@ int parse_events_add_breakpoint(struct parse_events_state *parse_state,
struct list_head *list,
u64 addr, char *type, u64 len,
struct parse_events_terms *head_config);
-int parse_events_add_pmu(struct parse_events_state *parse_state,
- struct list_head *list, const char *name,
- const struct parse_events_terms *const_parsed_terms,
- bool auto_merge_stats, void *loc);
struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
const char *name, const char *metric_id,
@@ -223,6 +219,12 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
const struct parse_events_terms *const_parsed_terms,
struct list_head **listp, void *loc);
+int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state,
+ const char *event_or_pmu,
+ const struct parse_events_terms *const_parsed_terms,
+ struct list_head **listp,
+ void *loc_);
+
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/parse-events.y b/tools/perf/util/parse-events.y
index d70f5d84af92..7764e5895210 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -273,78 +273,15 @@ event_def: event_pmu |
event_pmu:
PE_NAME opt_pmu_config
{
- struct parse_events_state *parse_state = _parse_state;
/* List of created evsels. */
struct list_head *list = NULL;
- char *pattern = NULL;
-
-#define CLEANUP \
- do { \
- parse_events_terms__delete($2); \
- free(list); \
- free($1); \
- free(pattern); \
- } while(0)
+ int err = parse_events_multi_pmu_add_or_add_pmu(_parse_state, $1, $2, &list, &@1);
- list = alloc_list();
- if (!list) {
- CLEANUP;
- YYNOMEM;
- }
- /* Attempt to add to list assuming $1 is a PMU name. */
- if (parse_events_add_pmu(parse_state, list, $1, $2, /*auto_merge_stats=*/false, &@1)) {
- struct perf_pmu *pmu = NULL;
- int ok = 0;
-
- /* Failure to add, try wildcard expansion of $1 as a PMU name. */
- if (asprintf(&pattern, "%s*", $1) < 0) {
- CLEANUP;
- YYNOMEM;
- }
-
- while ((pmu = perf_pmus__scan(pmu)) != NULL) {
- const char *name = pmu->name;
-
- if (parse_events__filter_pmu(parse_state, pmu))
- continue;
-
- if (!strncmp(name, "uncore_", 7) &&
- strncmp($1, "uncore_", 7))
- name += 7;
- if (!perf_pmu__match(pattern, name, $1) ||
- !perf_pmu__match(pattern, pmu->alias_name, $1)) {
- bool auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
-
- if (!parse_events_add_pmu(parse_state, list, pmu->name, $2,
- auto_merge_stats, &@1)) {
- ok++;
- parse_state->wild_card_pmus = true;
- }
- }
- }
-
- if (!ok) {
- /* Failure to add, assume $1 is an event name. */
- zfree(&list);
- ok = !parse_events_multi_pmu_add(parse_state, $1, $2, &list, &@1);
- }
- if (!ok) {
- struct parse_events_error *error = parse_state->error;
- char *help;
-
- if (asprintf(&help, "Unable to find PMU or event on a PMU of '%s'", $1) < 0)
- help = NULL;
- parse_events_error__handle(error, @1.first_column,
- strdup("Bad event or PMU"),
- help);
- CLEANUP;
- YYABORT;
- }
- }
+ parse_events_terms__delete($2);
+ free($1);
+ if (err)
+ PE_ABORT(err);
$$ = list;
- list = NULL;
- CLEANUP;
-#undef CLEANUP
}
|
PE_NAME sep_dc
--
2.44.0.683.g7961c838ac-goog
Avoid passing the name of a PMU then finding it again, just directly
pass the PMU. parse_events_multi_pmu_add_or_add_pmu is the only
version that needs to find a PMU, so move the find there. Remove the
error message as parse_events_multi_pmu_add_or_add_pmu will given an
error at the end when a name isn't either a PMU name or event
name. Without the error message being created the location in the
input parameter (loc) can be removed.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 46 +++++++++++++---------------------
1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a6f71165ee1a..2d5a275dd257 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1386,32 +1386,18 @@ static bool config_term_percore(struct list_head *config_terms)
}
static int parse_events_add_pmu(struct parse_events_state *parse_state,
- struct list_head *list, const char *name,
- const struct parse_events_terms *const_parsed_terms,
- bool auto_merge_stats, void *loc_)
+ struct list_head *list, struct perf_pmu *pmu,
+ const struct parse_events_terms *const_parsed_terms,
+ bool auto_merge_stats)
{
struct perf_event_attr attr;
struct perf_pmu_info info;
- struct perf_pmu *pmu;
struct evsel *evsel;
struct parse_events_error *err = parse_state->error;
- YYLTYPE *loc = loc_;
LIST_HEAD(config_terms);
struct parse_events_terms parsed_terms;
bool alias_rewrote_terms = false;
- pmu = parse_state->fake_pmu ?: perf_pmus__find(name);
-
- if (!pmu) {
- char *err_str;
-
- if (asprintf(&err_str,
- "Cannot find PMU `%s'. Missing kernel support?",
- name) >= 0)
- parse_events_error__handle(err, loc->first_column, err_str, NULL);
- return -EINVAL;
- }
-
parse_events_terms__init(&parsed_terms);
if (const_parsed_terms) {
int ret = parse_events_terms__copy(const_parsed_terms, &parsed_terms);
@@ -1425,9 +1411,9 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
strbuf_init(&sb, /*hint=*/ 0);
if (pmu->selectable && list_empty(&parsed_terms.terms)) {
- strbuf_addf(&sb, "%s//", name);
+ strbuf_addf(&sb, "%s//", pmu->name);
} else {
- strbuf_addf(&sb, "%s/", name);
+ strbuf_addf(&sb, "%s/", pmu->name);
parse_events_terms__to_strbuf(&parsed_terms, &sb);
strbuf_addch(&sb, '/');
}
@@ -1469,7 +1455,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
strbuf_init(&sb, /*hint=*/ 0);
parse_events_terms__to_strbuf(&parsed_terms, &sb);
- fprintf(stderr, "..after resolving event: %s/%s/\n", name, sb.buf);
+ fprintf(stderr, "..after resolving event: %s/%s/\n", pmu->name, sb.buf);
strbuf_release(&sb);
}
@@ -1583,8 +1569,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
continue;
auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
- if (!parse_events_add_pmu(parse_state, list, pmu->name,
- &parsed_terms, auto_merge_stats, loc)) {
+ if (!parse_events_add_pmu(parse_state, list, pmu,
+ &parsed_terms, auto_merge_stats)) {
struct strbuf sb;
strbuf_init(&sb, /*hint=*/ 0);
@@ -1596,8 +1582,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
}
if (parse_state->fake_pmu) {
- if (!parse_events_add_pmu(parse_state, list, event_name, &parsed_terms,
- /*auto_merge_stats=*/true, loc)) {
+ if (!parse_events_add_pmu(parse_state, list, parse_state->fake_pmu, &parsed_terms,
+ /*auto_merge_stats=*/true)) {
struct strbuf sb;
strbuf_init(&sb, /*hint=*/ 0);
@@ -1626,7 +1612,7 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
{
char *pattern = NULL;
YYLTYPE *loc = loc_;
- struct perf_pmu *pmu = NULL;
+ struct perf_pmu *pmu;
int ok = 0;
char *help;
@@ -1637,10 +1623,12 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
INIT_LIST_HEAD(*listp);
/* Attempt to add to list assuming event_or_pmu is a PMU name. */
- if (!parse_events_add_pmu(parse_state, *listp, event_or_pmu, const_parsed_terms,
- /*auto_merge_stats=*/false, loc))
+ pmu = parse_state->fake_pmu ?: perf_pmus__find(event_or_pmu);
+ if (pmu && !parse_events_add_pmu(parse_state, *listp, pmu, const_parsed_terms,
+ /*auto_merge_stats=*/false))
return 0;
+ pmu = NULL;
/* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
if (asprintf(&pattern, "%s*", event_or_pmu) < 0) {
zfree(listp);
@@ -1660,9 +1648,9 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
!perf_pmu__match(pattern, pmu->alias_name, event_or_pmu)) {
bool auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
- if (!parse_events_add_pmu(parse_state, *listp, pmu->name,
+ if (!parse_events_add_pmu(parse_state, *listp, pmu,
const_parsed_terms,
- auto_merge_stats, loc)) {
+ auto_merge_stats)) {
ok++;
parse_state->wild_card_pmus = true;
}
--
2.44.0.683.g7961c838ac-goog
In parse_events_add_pmu, delay copying the list of terms until it is
known the list contains terms.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 2d5a275dd257..3b1f767039fa 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1398,29 +1398,21 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
struct parse_events_terms parsed_terms;
bool alias_rewrote_terms = false;
- parse_events_terms__init(&parsed_terms);
- if (const_parsed_terms) {
- int ret = parse_events_terms__copy(const_parsed_terms, &parsed_terms);
-
- if (ret)
- return ret;
- }
-
if (verbose > 1) {
struct strbuf sb;
strbuf_init(&sb, /*hint=*/ 0);
- if (pmu->selectable && list_empty(&parsed_terms.terms)) {
+ if (pmu->selectable && const_parsed_terms &&
+ list_empty(&const_parsed_terms->terms)) {
strbuf_addf(&sb, "%s//", pmu->name);
} else {
strbuf_addf(&sb, "%s/", pmu->name);
- parse_events_terms__to_strbuf(&parsed_terms, &sb);
+ parse_events_terms__to_strbuf(const_parsed_terms, &sb);
strbuf_addch(&sb, '/');
}
fprintf(stderr, "Attempt to add: %s\n", sb.buf);
strbuf_release(&sb);
}
- fix_raw(&parsed_terms, pmu);
memset(&attr, 0, sizeof(attr));
if (pmu->perf_event_attr_init_default)
@@ -1428,7 +1420,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
attr.type = pmu->type;
- if (list_empty(&parsed_terms.terms)) {
+ if (!const_parsed_terms || list_empty(&const_parsed_terms->terms)) {
evsel = __add_event(list, &parse_state->idx, &attr,
/*init_attr=*/true, /*name=*/NULL,
/*metric_id=*/NULL, pmu,
@@ -1437,6 +1429,15 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
return evsel ? 0 : -ENOMEM;
}
+ parse_events_terms__init(&parsed_terms);
+ if (const_parsed_terms) {
+ int ret = parse_events_terms__copy(const_parsed_terms, &parsed_terms);
+
+ if (ret)
+ return ret;
+ }
+ fix_raw(&parsed_terms, pmu);
+
/* Configure attr/terms with a known PMU, this will set hardcoded terms. */
if (config_attr(&attr, &parsed_terms, parse_state->error, config_term_pmu)) {
parse_events_terms__exit(&parsed_terms);
--
2.44.0.683.g7961c838ac-goog
Move all implementation to pmu code. Don't allocate a fnmatch wildcard
pattern, matching ignoring the suffix already handles this, and only
use fnmatch if the given PMU name has a '*' in it.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 19 ++-----------------
tools/perf/util/pmu.c | 27 +++++++++++++++++++--------
tools/perf/util/pmu.h | 2 +-
3 files changed, 22 insertions(+), 26 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3b1f767039fa..39548ec645ec 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1611,7 +1611,6 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
struct list_head **listp,
void *loc_)
{
- char *pattern = NULL;
YYLTYPE *loc = loc_;
struct perf_pmu *pmu;
int ok = 0;
@@ -1631,22 +1630,9 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
pmu = NULL;
/* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
- if (asprintf(&pattern, "%s*", event_or_pmu) < 0) {
- zfree(listp);
- return -ENOMEM;
- }
-
while ((pmu = perf_pmus__scan(pmu)) != NULL) {
- const char *name = pmu->name;
-
- if (parse_events__filter_pmu(parse_state, pmu))
- continue;
-
- if (!strncmp(name, "uncore_", 7) &&
- strncmp(event_or_pmu, "uncore_", 7))
- name += 7;
- if (!perf_pmu__match(pattern, name, event_or_pmu) ||
- !perf_pmu__match(pattern, pmu->alias_name, event_or_pmu)) {
+ if (!parse_events__filter_pmu(parse_state, pmu) &&
+ perf_pmu__match(pmu, event_or_pmu)) {
bool auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
if (!parse_events_add_pmu(parse_state, *listp, pmu,
@@ -1657,7 +1643,6 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
}
}
}
- zfree(&pattern);
if (ok)
return 0;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ce72c99e4f61..d7521d84fe4a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2073,18 +2073,29 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
name ?: "N/A", buf, config_name, config);
}
-int perf_pmu__match(const char *pattern, const char *name, const char *tok)
+bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok)
{
- if (!name)
- return -1;
+ const char *name = pmu->name;
+ bool need_fnmatch = strchr(tok, '*') != NULL;
- if (fnmatch(pattern, name, 0))
- return -1;
+ if (!strncmp(tok, "uncore_", 7))
+ tok += 7;
+ if (!strncmp(name, "uncore_", 7))
+ name += 7;
- if (tok && !perf_pmu__match_ignoring_suffix(name, tok))
- return -1;
+ if (perf_pmu__match_ignoring_suffix(name, tok) ||
+ (need_fnmatch && !fnmatch(tok, name, 0)))
+ return true;
- return 0;
+ name = pmu->alias_name;
+ if (!name)
+ return false;
+
+ if (!strncmp(name, "uncore_", 7))
+ name += 7;
+
+ return perf_pmu__match_ignoring_suffix(name, tok) ||
+ (need_fnmatch && !fnmatch(tok, name, 0));
}
double __weak perf_pmu__cpu_slots_per_cycle(void)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 152700f78455..93d03bd3ecbe 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -263,7 +263,7 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
const char *config_name);
void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
-int perf_pmu__match(const char *pattern, const char *name, const char *tok);
+bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok);
double perf_pmu__cpu_slots_per_cycle(void);
int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size);
--
2.44.0.683.g7961c838ac-goog
Switch from cache-references to branches in test as Intel has a sysfs
event for cache-references and changing the priority for sysfs over
legacy causes the test to fail.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/tests/parse-events.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 0b70451451b3..993e482f094c 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -942,8 +942,8 @@ static int test__group2(struct evlist *evlist)
continue;
}
if (evsel->core.attr.type == PERF_TYPE_HARDWARE &&
- test_config(evsel, PERF_COUNT_HW_CACHE_REFERENCES)) {
- /* cache-references + :u modifier */
+ test_config(evsel, PERF_COUNT_HW_BRANCH_INSTRUCTIONS)) {
+ /* branches + :u modifier */
TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
@@ -2032,7 +2032,7 @@ static const struct evlist_test test__events[] = {
/* 8 */
},
{
- .name = "{faults:k,cache-references}:u,cycles:k",
+ .name = "{faults:k,branches}:u,cycles:k",
.check = test__group2,
/* 9 */
},
--
2.44.0.683.g7961c838ac-goog
Avoid duplicate logic for name_or_raw and PE_TERM_HW by having a rule
to turn PE_TERM_HW into a name_or_raw.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.y | 31 +++++--------------------------
1 file changed, 5 insertions(+), 26 deletions(-)
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 7764e5895210..254f8aeca461 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -603,6 +603,11 @@ event_term
}
name_or_raw: PE_RAW | PE_NAME | PE_LEGACY_CACHE
+|
+PE_TERM_HW
+{
+ $$ = $1.str;
+}
event_term:
PE_RAW
@@ -644,20 +649,6 @@ name_or_raw '=' PE_VALUE
$$ = term;
}
|
-name_or_raw '=' PE_TERM_HW
-{
- struct parse_events_term *term;
- int err = parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, $3.str, &@1, &@3);
-
- if (err) {
- free($1);
- free($3.str);
- PE_ABORT(err);
- }
- $$ = term;
-}
-|
PE_LEGACY_CACHE
{
struct parse_events_term *term;
@@ -710,18 +701,6 @@ PE_TERM '=' name_or_raw
$$ = term;
}
|
-PE_TERM '=' PE_TERM_HW
-{
- struct parse_events_term *term;
- int err = parse_events_term__str(&term, $1, /*config=*/NULL, $3.str, &@1, &@3);
-
- if (err) {
- free($3.str);
- PE_ABORT(err);
- }
- $$ = term;
-}
-|
PE_TERM '=' PE_TERM
{
struct parse_events_term *term;
--
2.44.0.683.g7961c838ac-goog
Allow the term list to be const so that other functions can pass const
term lists. Add const as necessary to called functions.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 20 +++++++++++---------
tools/perf/util/parse-events.h | 2 +-
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1440a3b4b674..1b408e3dccc7 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -34,7 +34,8 @@
#ifdef PARSER_DEBUG
extern int parse_events_debug;
#endif
-static int get_config_terms(struct parse_events_terms *head_config, struct list_head *head_terms);
+static int get_config_terms(const struct parse_events_terms *head_config,
+ struct list_head *head_terms);
static int parse_events_terms__copy(const struct parse_events_terms *src,
struct parse_events_terms *dest);
@@ -154,7 +155,7 @@ const char *event_type(int type)
return "unknown";
}
-static char *get_config_str(struct parse_events_terms *head_terms,
+static char *get_config_str(const struct parse_events_terms *head_terms,
enum parse_events__term_type type_term)
{
struct parse_events_term *term;
@@ -169,12 +170,12 @@ static char *get_config_str(struct parse_events_terms *head_terms,
return NULL;
}
-static char *get_config_metric_id(struct parse_events_terms *head_terms)
+static char *get_config_metric_id(const struct parse_events_terms *head_terms)
{
return get_config_str(head_terms, PARSE_EVENTS__TERM_TYPE_METRIC_ID);
}
-static char *get_config_name(struct parse_events_terms *head_terms)
+static char *get_config_name(const struct parse_events_terms *head_terms)
{
return get_config_str(head_terms, PARSE_EVENTS__TERM_TYPE_NAME);
}
@@ -358,7 +359,7 @@ static int config_term_common(struct perf_event_attr *attr,
struct parse_events_term *term,
struct parse_events_error *err);
static int config_attr(struct perf_event_attr *attr,
- struct parse_events_terms *head,
+ const struct parse_events_terms *head,
struct parse_events_error *err,
config_term_func_t config_term);
@@ -1108,7 +1109,7 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
#endif
static int config_attr(struct perf_event_attr *attr,
- struct parse_events_terms *head,
+ const struct parse_events_terms *head,
struct parse_events_error *err,
config_term_func_t config_term)
{
@@ -1121,7 +1122,8 @@ static int config_attr(struct perf_event_attr *attr,
return 0;
}
-static int get_config_terms(struct parse_events_terms *head_config, struct list_head *head_terms)
+static int get_config_terms(const struct parse_events_terms *head_config,
+ struct list_head *head_terms)
{
#define ADD_CONFIG_TERM(__type, __weak) \
struct evsel_config_term *__t; \
@@ -1325,7 +1327,7 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
static int __parse_events_add_numeric(struct parse_events_state *parse_state,
struct list_head *list,
struct perf_pmu *pmu, u32 type, u32 extended_type,
- u64 config, struct parse_events_terms *head_config)
+ u64 config, const struct parse_events_terms *head_config)
{
struct perf_event_attr attr;
LIST_HEAD(config_terms);
@@ -1361,7 +1363,7 @@ static int __parse_events_add_numeric(struct parse_events_state *parse_state,
int parse_events_add_numeric(struct parse_events_state *parse_state,
struct list_head *list,
u32 type, u64 config,
- struct parse_events_terms *head_config,
+ const struct parse_events_terms *head_config,
bool wildcard)
{
struct perf_pmu *pmu = NULL;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index db47913e54bc..5005782766e9 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -196,7 +196,7 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
int parse_events_add_numeric(struct parse_events_state *parse_state,
struct list_head *list,
u32 type, u64 config,
- struct parse_events_terms *head_config,
+ const struct parse_events_terms *head_config,
bool wildcard);
int parse_events_add_tool(struct parse_events_state *parse_state,
struct list_head *list,
--
2.44.0.683.g7961c838ac-goog
It was requested that RISC-V be able to add events to the perf tool so
the PMU driver didn't need to map legacy events to config encodings:
https://lore.kernel.org/lkml/[email protected]/
This change makes the priority of events specified without a PMU the
same as those specified with a PMU, namely sysfs and json events are
checked first before using the legacy encoding.
The hw_term is made more generic as a hardware_event that encodes a
pair of string and int value, allowing parse_events_multi_pmu_add to
fall back on a known encoding when the sysfs/json adding fails for
core events. As this covers PE_VALUE_SYM_HW, that token is removed and
related code simplified.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 31 ++++++++++----
tools/perf/util/parse-events.h | 2 +-
tools/perf/util/parse-events.l | 76 +++++++++++++++++-----------------
tools/perf/util/parse-events.y | 62 +++++++++++++++++----------
4 files changed, 103 insertions(+), 68 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1b408e3dccc7..805872c90a3e 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1543,7 +1543,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
}
int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
- const char *event_name,
+ const char *event_name, u64 hw_config,
const struct parse_events_terms *const_parsed_terms,
struct list_head **listp, void *loc_)
{
@@ -1551,8 +1551,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
struct list_head *list = NULL;
struct perf_pmu *pmu = NULL;
YYLTYPE *loc = loc_;
- int ok = 0;
- const char *config;
+ int ok = 0, core_ok = 0;
+ const char *tmp;
struct parse_events_terms parsed_terms;
*listp = NULL;
@@ -1565,15 +1565,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
return ret;
}
- config = strdup(event_name);
- if (!config)
+ tmp = strdup(event_name);
+ if (!tmp)
goto out_err;
if (parse_events_term__num(&term,
PARSE_EVENTS__TERM_TYPE_USER,
- config, /*num=*/1, /*novalue=*/true,
+ tmp, /*num=*/1, /*novalue=*/true,
loc, /*loc_val=*/NULL) < 0) {
- zfree(&config);
+ zfree(&tmp);
goto out_err;
}
list_add_tail(&term->list, &parsed_terms.terms);
@@ -1604,6 +1604,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
pr_debug("%s -> %s/%s/\n", event_name, pmu->name, sb.buf);
strbuf_release(&sb);
ok++;
+ if (pmu->is_core)
+ core_ok++;
}
}
@@ -1620,6 +1622,18 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
}
}
+ if (hw_config != PERF_COUNT_HW_MAX && !core_ok) {
+ /*
+ * The event wasn't found on core PMUs but it has a hardware
+ * config version to try.
+ */
+ if (!parse_events_add_numeric(parse_state, list,
+ PERF_TYPE_HARDWARE, hw_config,
+ const_parsed_terms,
+ /*wildcard=*/true))
+ ok++;
+ }
+
out_err:
parse_events_terms__exit(&parsed_terms);
if (ok)
@@ -1673,7 +1687,8 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
/* Failure to add, assume event_or_pmu is an event name. */
zfree(listp);
- if (!parse_events_multi_pmu_add(parse_state, event_or_pmu, const_parsed_terms, listp, loc))
+ if (!parse_events_multi_pmu_add(parse_state, event_or_pmu, PERF_COUNT_HW_MAX,
+ const_parsed_terms, listp, loc))
return 0;
if (asprintf(&help, "Unable to find PMU or event on a PMU of '%s'", event_or_pmu) < 0)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 5005782766e9..7e5afad3feb8 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -215,7 +215,7 @@ struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
struct perf_pmu *pmu);
int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
- const char *event_name,
+ const char *event_name, u64 hw_config,
const struct parse_events_terms *const_parsed_terms,
struct list_head **listp, void *loc);
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index e86c45675e1d..6fe37003ab7b 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -100,12 +100,12 @@ do { \
yyless(0); \
} while (0)
-static int sym(yyscan_t scanner, int type, int config)
+static int sym(yyscan_t scanner, int config)
{
YYSTYPE *yylval = parse_events_get_lval(scanner);
- yylval->num = (type << 16) + config;
- return type == PERF_TYPE_HARDWARE ? PE_VALUE_SYM_HW : PE_VALUE_SYM_SW;
+ yylval->num = config;
+ return PE_VALUE_SYM_SW;
}
static int tool(yyscan_t scanner, enum perf_tool_event event)
@@ -124,13 +124,13 @@ static int term(yyscan_t scanner, enum parse_events__term_type type)
return PE_TERM;
}
-static int hw_term(yyscan_t scanner, int config)
+static int hw(yyscan_t scanner, int config)
{
YYSTYPE *yylval = parse_events_get_lval(scanner);
char *text = parse_events_get_text(scanner);
- yylval->hardware_term.str = strdup(text);
- yylval->hardware_term.num = PERF_TYPE_HARDWARE + config;
+ yylval->hardware_event.str = strdup(text);
+ yylval->hardware_event.num = config;
return PE_TERM_HW;
}
@@ -246,16 +246,16 @@ percore { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_PERCORE); }
aux-output { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT); }
aux-sample-size { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE); }
metric-id { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_METRIC_ID); }
-cpu-cycles|cycles { return hw_term(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
-stalled-cycles-frontend|idle-cycles-frontend { return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
-stalled-cycles-backend|idle-cycles-backend { return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
-instructions { return hw_term(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); }
-cache-references { return hw_term(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); }
-cache-misses { return hw_term(yyscanner, PERF_COUNT_HW_CACHE_MISSES); }
-branch-instructions|branches { return hw_term(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
-branch-misses { return hw_term(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); }
-bus-cycles { return hw_term(yyscanner, PERF_COUNT_HW_BUS_CYCLES); }
-ref-cycles { return hw_term(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); }
+cpu-cycles|cycles { return hw(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
+stalled-cycles-frontend|idle-cycles-frontend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
+stalled-cycles-backend|idle-cycles-backend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
+instructions { return hw(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); }
+cache-references { return hw(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); }
+cache-misses { return hw(yyscanner, PERF_COUNT_HW_CACHE_MISSES); }
+branch-instructions|branches { return hw(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
+branch-misses { return hw(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); }
+bus-cycles { return hw(yyscanner, PERF_COUNT_HW_BUS_CYCLES); }
+ref-cycles { return hw(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); }
r{num_raw_hex} { return str(yyscanner, PE_RAW); }
r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
, { return ','; }
@@ -299,31 +299,31 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
<<EOF>> { BEGIN(INITIAL); }
}
-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); }
-branch-instructions|branches { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
-branch-misses { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_MISSES); }
-bus-cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BUS_CYCLES); }
-ref-cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_REF_CPU_CYCLES); }
-cpu-clock { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_CLOCK); }
-task-clock { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_TASK_CLOCK); }
-page-faults|faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS); }
-minor-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MIN); }
-major-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MAJ); }
-context-switches|cs { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CONTEXT_SWITCHES); }
-cpu-migrations|migrations { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_MIGRATIONS); }
-alignment-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
-emulation-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
-dummy { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
+cpu-cycles|cycles { return hw(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
+stalled-cycles-frontend|idle-cycles-frontend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
+stalled-cycles-backend|idle-cycles-backend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
+instructions { return hw(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); }
+cache-references { return hw(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); }
+cache-misses { return hw(yyscanner, PERF_COUNT_HW_CACHE_MISSES); }
+branch-instructions|branches { return hw(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
+branch-misses { return hw(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); }
+bus-cycles { return hw(yyscanner, PERF_COUNT_HW_BUS_CYCLES); }
+ref-cycles { return hw(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); }
+cpu-clock { return sym(yyscanner, PERF_COUNT_SW_CPU_CLOCK); }
+task-clock { return sym(yyscanner, PERF_COUNT_SW_TASK_CLOCK); }
+page-faults|faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS); }
+minor-faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS_MIN); }
+major-faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS_MAJ); }
+context-switches|cs { return sym(yyscanner, PERF_COUNT_SW_CONTEXT_SWITCHES); }
+cpu-migrations|migrations { return sym(yyscanner, PERF_COUNT_SW_CPU_MIGRATIONS); }
+alignment-faults { return sym(yyscanner, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
+emulation-faults { return sym(yyscanner, PERF_COUNT_SW_EMULATION_FAULTS); }
+dummy { return sym(yyscanner, PERF_COUNT_SW_DUMMY); }
duration_time { return tool(yyscanner, PERF_TOOL_DURATION_TIME); }
user_time { return tool(yyscanner, PERF_TOOL_USER_TIME); }
system_time { return tool(yyscanner, PERF_TOOL_SYSTEM_TIME); }
-bpf-output { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
-cgroup-switches { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CGROUP_SWITCHES); }
+bpf-output { return sym(yyscanner, PERF_COUNT_SW_BPF_OUTPUT); }
+cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); }
{lc_type} { return str(yyscanner, PE_LEGACY_CACHE); }
{lc_type}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); }
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 254f8aeca461..31fe8cf428ff 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -55,7 +55,7 @@ static void free_list_evsel(struct list_head* list_evsel)
%}
%token PE_START_EVENTS PE_START_TERMS
-%token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_TERM
+%token PE_VALUE PE_VALUE_SYM_SW PE_TERM
%token PE_VALUE_SYM_TOOL
%token PE_EVENT_NAME
%token PE_RAW PE_NAME
@@ -66,11 +66,9 @@ static void free_list_evsel(struct list_head* list_evsel)
%token PE_DRV_CFG_TERM
%token PE_TERM_HW
%type <num> PE_VALUE
-%type <num> PE_VALUE_SYM_HW
%type <num> PE_VALUE_SYM_SW
%type <num> PE_VALUE_SYM_TOOL
%type <term_type> PE_TERM
-%type <num> value_sym
%type <str> PE_RAW
%type <str> PE_NAME
%type <str> PE_LEGACY_CACHE
@@ -87,6 +85,7 @@ static void free_list_evsel(struct list_head* list_evsel)
%type <list_terms> opt_pmu_config
%destructor { parse_events_terms__delete ($$); } <list_terms>
%type <list_evsel> event_pmu
+%type <list_evsel> event_legacy_hardware
%type <list_evsel> event_legacy_symbol
%type <list_evsel> event_legacy_cache
%type <list_evsel> event_legacy_mem
@@ -104,8 +103,8 @@ static void free_list_evsel(struct list_head* list_evsel)
%destructor { free_list_evsel ($$); } <list_evsel>
%type <tracepoint_name> tracepoint_name
%destructor { free ($$.sys); free ($$.event); } <tracepoint_name>
-%type <hardware_term> PE_TERM_HW
-%destructor { free ($$.str); } <hardware_term>
+%type <hardware_event> PE_TERM_HW
+%destructor { free ($$.str); } <hardware_event>
%union
{
@@ -119,10 +118,10 @@ static void free_list_evsel(struct list_head* list_evsel)
char *sys;
char *event;
} tracepoint_name;
- struct hardware_term {
+ struct hardware_event {
char *str;
u64 num;
- } hardware_term;
+ } hardware_event;
}
%%
@@ -263,6 +262,7 @@ PE_EVENT_NAME event_def
event_def
event_def: event_pmu |
+ event_legacy_hardware |
event_legacy_symbol |
event_legacy_cache sep_dc |
event_legacy_mem sep_dc |
@@ -289,7 +289,7 @@ PE_NAME sep_dc
struct list_head *list;
int err;
- err = parse_events_multi_pmu_add(_parse_state, $1, NULL, &list, &@1);
+ err = parse_events_multi_pmu_add(_parse_state, $1, PERF_COUNT_HW_MAX, NULL, &list, &@1);
if (err < 0) {
struct parse_events_state *parse_state = _parse_state;
struct parse_events_error *error = parse_state->error;
@@ -305,24 +305,45 @@ PE_NAME sep_dc
$$ = list;
}
-value_sym:
-PE_VALUE_SYM_HW
+event_legacy_hardware:
+PE_TERM_HW opt_pmu_config
+{
+ /* List of created evsels. */
+ struct list_head *list = NULL;
+ int err = parse_events_multi_pmu_add(_parse_state, $1.str, $1.num, $2, &list, &@1);
+
+ free($1.str);
+ parse_events_terms__delete($2);
+ if (err)
+ PE_ABORT(err);
+
+ $$ = list;
+}
|
-PE_VALUE_SYM_SW
+PE_TERM_HW sep_dc
+{
+ struct list_head *list;
+ int err;
+
+ err = parse_events_multi_pmu_add(_parse_state, $1.str, $1.num, NULL, &list, &@1);
+ free($1.str);
+ if (err)
+ PE_ABORT(err);
+ $$ = list;
+}
event_legacy_symbol:
-value_sym '/' event_config '/'
+PE_VALUE_SYM_SW '/' event_config '/'
{
struct list_head *list;
- int type = $1 >> 16;
- int config = $1 & 255;
int err;
- bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE);
list = alloc_list();
if (!list)
YYNOMEM;
- err = parse_events_add_numeric(_parse_state, list, type, config, $3, wildcard);
+ err = parse_events_add_numeric(_parse_state, list,
+ /*type=*/PERF_TYPE_SOFTWARE, /*config=*/$1,
+ $3, /*wildcard=*/false);
parse_events_terms__delete($3);
if (err) {
free_list_evsel(list);
@@ -331,18 +352,17 @@ value_sym '/' event_config '/'
$$ = list;
}
|
-value_sym sep_slash_slash_dc
+PE_VALUE_SYM_SW sep_slash_slash_dc
{
struct list_head *list;
- int type = $1 >> 16;
- int config = $1 & 255;
- bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE);
int err;
list = alloc_list();
if (!list)
YYNOMEM;
- err = parse_events_add_numeric(_parse_state, list, type, config, /*head_config=*/NULL, wildcard);
+ err = parse_events_add_numeric(_parse_state, list,
+ /*type=*/PERF_TYPE_SOFTWARE, /*config=*/$1,
+ /*head_config=*/NULL, /*wildcard=*/false);
if (err)
PE_ABORT(err);
$$ = list;
--
2.44.0.683.g7961c838ac-goog
Use the error handler from the parse_state to give a more informative
error message.
Before:
```
$ perf stat -e 'cycles/period=99999999999999999999/' true
event syntax error: 'cycles/period=99999999999999999999/'
\___ parser error
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
After:
```
$ perf stat -e 'cycles/period=99999999999999999999/' true
event syntax error: 'cycles/period=99999999999999999999/'
\___ parser error
event syntax error: '..les/period=99999999999999999999/'
\___ Bad base 10 number "99999999999999999999"
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.l | 40 ++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 6fe37003ab7b..0cd68c9f0d4f 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -18,26 +18,34 @@
char *parse_events_get_text(yyscan_t yyscanner);
YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
+int parse_events_get_column(yyscan_t yyscanner);
+int parse_events_get_leng(yyscan_t yyscanner);
-static int __value(YYSTYPE *yylval, char *str, int base, int token)
+static int get_column(yyscan_t scanner)
{
- u64 num;
-
- errno = 0;
- num = strtoull(str, NULL, base);
- if (errno)
- return PE_ERROR;
-
- yylval->num = num;
- return token;
+ return parse_events_get_column(scanner) - parse_events_get_leng(scanner);
}
-static int value(yyscan_t scanner, int base)
+static int value(struct parse_events_state *parse_state, yyscan_t scanner, int base)
{
YYSTYPE *yylval = parse_events_get_lval(scanner);
char *text = parse_events_get_text(scanner);
+ u64 num;
- return __value(yylval, text, base, PE_VALUE);
+ errno = 0;
+ num = strtoull(text, NULL, base);
+ if (errno) {
+ struct parse_events_error *error = parse_state->error;
+ char *help = NULL;
+
+ if (asprintf(&help, "Bad base %d number \"%s\"", base, text) > 0)
+ parse_events_error__handle(error, get_column(scanner), help , NULL);
+
+ return PE_ERROR;
+ }
+
+ yylval->num = num;
+ return PE_VALUE;
}
static int str(yyscan_t scanner, int token)
@@ -283,8 +291,8 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
*/
"/"/{digit} { return PE_BP_SLASH; }
"/"/{non_digit} { BEGIN(config); return '/'; }
-{num_dec} { return value(yyscanner, 10); }
-{num_hex} { return value(yyscanner, 16); }
+{num_dec} { return value(_parse_state, yyscanner, 10); }
+{num_hex} { return value(_parse_state, yyscanner, 16); }
/*
* We need to separate 'mem:' scanner part, in order to get specific
* modifier bits parsed out. Otherwise we would need to handle PE_NAME
@@ -330,8 +338,8 @@ cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); }
{lc_type}-{lc_op_result}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); }
mem: { BEGIN(mem); return PE_PREFIX_MEM; }
r{num_raw_hex} { return str(yyscanner, PE_RAW); }
-{num_dec} { return value(yyscanner, 10); }
-{num_hex} { return value(yyscanner, 16); }
+{num_dec} { return value(_parse_state, yyscanner, 10); }
+{num_hex} { return value(_parse_state, yyscanner, 16); }
{modifier_event} { return str(yyscanner, PE_MODIFIER_EVENT); }
{name} { return str(yyscanner, PE_NAME); }
--
2.44.0.683.g7961c838ac-goog
Add comments. Ensure leader->group_name is freed before overwriting
it.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 1 +
tools/perf/util/parse-events.y | 6 +++++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 8d3d692d219d..1c1b1bcb78e8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1711,6 +1711,7 @@ void parse_events__set_leader(char *name, struct list_head *list)
leader = list_first_entry(list, struct evsel, core.node);
__perf_evlist__set_leader(list, &leader->core);
+ zfree(&leader->group_name);
leader->group_name = name;
}
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 79f254189be6..6f1042272dda 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -193,7 +193,10 @@ PE_NAME '{' events '}'
{
struct list_head *list = $3;
- /* Takes ownership of $1. */
+ /*
+ * Set the first entry of list to be the leader. Set the group name on
+ * the leader to $1 taking ownership.
+ */
parse_events__set_leader($1, list);
$$ = list;
}
@@ -202,6 +205,7 @@ PE_NAME '{' events '}'
{
struct list_head *list = $2;
+ /* Set the first entry of list to be the leader clearing the group name. */
parse_events__set_leader(NULL, list);
$$ = list;
}
--
2.44.0.683.g7961c838ac-goog
Prior behavior is to not look for legacy cache names in sysfs/json and
to create events on all core PMUs. New behavior is to look for
sysfs/json events first on all PMUs, for core PMUs add a legacy event
if the sysfs/json event isn't present.
This is done so that there is consistency with how event names in
terms are handled and their prioritization of sysfs/json over
legacy. It may make sense to use a legacy cache event name as an event
name on a non-core PMU so we should allow it.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 39 +++++++++++++++++++++++++++-------
tools/perf/util/parse-events.h | 2 +-
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 39548ec645ec..1440a3b4b674 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -442,17 +442,21 @@ bool parse_events__filter_pmu(const struct parse_events_state *parse_state,
return strcmp(parse_state->pmu_filter, pmu->name) != 0;
}
+static int parse_events_add_pmu(struct parse_events_state *parse_state,
+ struct list_head *list, struct perf_pmu *pmu,
+ const struct parse_events_terms *const_parsed_terms,
+ bool auto_merge_stats);
+
int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
struct parse_events_state *parse_state,
- struct parse_events_terms *head_config)
+ struct parse_events_terms *parsed_terms)
{
struct perf_pmu *pmu = NULL;
bool found_supported = false;
- const char *config_name = get_config_name(head_config);
- const char *metric_id = get_config_metric_id(head_config);
+ const char *config_name = get_config_name(parsed_terms);
+ const char *metric_id = get_config_metric_id(parsed_terms);
- /* Legacy cache events are only supported by core PMUs. */
- while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
+ while ((pmu = perf_pmus__scan(pmu)) != NULL) {
LIST_HEAD(config_terms);
struct perf_event_attr attr;
int ret;
@@ -460,6 +464,24 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
if (parse_events__filter_pmu(parse_state, pmu))
continue;
+ if (perf_pmu__have_event(pmu, name)) {
+ /*
+ * The PMU has the event so add as not a legacy cache
+ * event.
+ */
+ ret = parse_events_add_pmu(parse_state, list, pmu,
+ parsed_terms,
+ perf_pmu__auto_merge_stats(pmu));
+ if (ret)
+ return ret;
+ continue;
+ }
+
+ if (!pmu->is_core) {
+ /* Legacy cache events are only supported by core PMUs. */
+ continue;
+ }
+
memset(&attr, 0, sizeof(attr));
attr.type = PERF_TYPE_HW_CACHE;
@@ -469,11 +491,12 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
found_supported = true;
- if (head_config) {
- if (config_attr(&attr, head_config, parse_state->error, config_term_common))
+ if (parsed_terms) {
+ if (config_attr(&attr, parsed_terms, parse_state->error,
+ config_term_common))
return -EINVAL;
- if (get_config_terms(head_config, &config_terms))
+ if (get_config_terms(parsed_terms, &config_terms))
return -ENOMEM;
}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index a331b9f0da2b..db47913e54bc 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -203,7 +203,7 @@ int parse_events_add_tool(struct parse_events_state *parse_state,
int tool_event);
int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
struct parse_events_state *parse_state,
- struct parse_events_terms *head_config);
+ struct parse_events_terms *parsed_terms);
int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config);
int parse_events_add_breakpoint(struct parse_events_state *parse_state,
struct list_head *list,
--
2.44.0.683.g7961c838ac-goog
Add comments. Pass ownership of the event name to save on a strdup.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 9 ++++++---
tools/perf/util/parse-events.h | 2 +-
tools/perf/util/parse-events.l | 5 +++++
tools/perf/util/parse-events.y | 10 +++++++---
4 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1c1b1bcb78e8..0f308b4db2b9 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1836,18 +1836,21 @@ int parse_events__modifier_event(struct parse_events_state *parse_state, void *l
return parse_events__modifier_list(parse_state, loc, list, mod, /*group=*/false);
}
-int parse_events_name(struct list_head *list, const char *name)
+int parse_events__set_default_name(struct list_head *list, char *name)
{
struct evsel *evsel;
+ bool used_name = false;
__evlist__for_each_entry(list, evsel) {
if (!evsel->name) {
- evsel->name = strdup(name);
+ evsel->name = used_name ? strdup(name) : name;
+ used_name = true;
if (!evsel->name)
return -ENOMEM;
}
}
-
+ if (!used_name)
+ free(name);
return 0;
}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 0bb5f0c80a5e..5695308efab9 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -207,7 +207,7 @@ int parse_events__modifier_event(struct parse_events_state *parse_state, void *l
struct list_head *list, struct parse_events_modifier mod);
int parse_events__modifier_group(struct parse_events_state *parse_state, void *loc,
struct list_head *list, struct parse_events_modifier mod);
-int parse_events_name(struct list_head *list, const char *name);
+int parse_events__set_default_name(struct list_head *list, char *name);
int parse_events_add_tracepoint(struct list_head *list, int *idx,
const char *sys, const char *event,
struct parse_events_error *error,
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4aaf0c53d9b6..08ea2d845dc3 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -96,6 +96,11 @@ static int drv_str(yyscan_t scanner, int token)
return token;
}
+/*
+ * Use yyless to return all the characaters to the input. Update the column for
+ * location debugging. If __alloc is non-zero set yylval to the text for the
+ * returned token's value.
+ */
#define REWIND(__alloc) \
do { \
YYSTYPE *__yylval = parse_events_get_lval(yyscanner); \
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 6f1042272dda..68b3b06c7ff0 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -247,10 +247,14 @@ event_name
event_name:
PE_EVENT_NAME event_def
{
- int err;
+ /*
+ * When an event is parsed the text is rewound and the entire text of
+ * the event is set to the str of PE_EVENT_NAME token matched here. If
+ * no name was on an event via a term, set the name to the entire text
+ * taking ownership of the allocation.
+ */
+ int err = parse_events__set_default_name($2, $1);
- err = parse_events_name($2, $1);
- free($1);
if (err) {
free_list_evsel($2);
YYNOMEM;
--
2.44.0.683.g7961c838ac-goog
The helper function just wraps a splice and free. Making the free
inline removes a comment, so then it just wraps a splice which we can
make inline too.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 13 -----------
tools/perf/util/parse-events.h | 2 --
tools/perf/util/parse-events.y | 41 +++++++++++++++++++++-------------
3 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 805872c90a3e..7eba714f0d73 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1720,19 +1720,6 @@ void parse_events__set_leader(char *name, struct list_head *list)
leader->group_name = name;
}
-/* list_event is assumed to point to malloc'ed memory */
-void parse_events_update_lists(struct list_head *list_event,
- struct list_head *list_all)
-{
- /*
- * Called for single event definition. Update the
- * 'all event' list, and reinit the 'single event'
- * list, for next event definition.
- */
- list_splice_tail(list_event, list_all);
- free(list_event);
-}
-
struct event_modifier {
int eu;
int ek;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 7e5afad3feb8..e8f2aebea10f 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -226,8 +226,6 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
void *loc_);
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);
void parse_events_evlist_error(struct parse_events_state *parse_state,
int idx, const char *str);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 31fe8cf428ff..51490f0f8c50 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -125,6 +125,10 @@ static void free_list_evsel(struct list_head* list_evsel)
}
%%
+ /*
+ * Entry points. We are either parsing events or terminals. Just terminal
+ * parsing is used for parsing events in sysfs.
+ */
start:
PE_START_EVENTS start_events
|
@@ -132,31 +136,36 @@ PE_START_TERMS start_terms
start_events: groups
{
+ /* Take the parsed events, groups.. and place into parse_state. */
+ struct list_head *groups = $1;
struct parse_events_state *parse_state = _parse_state;
- /* frees $1 */
- parse_events_update_lists($1, &parse_state->list);
+ list_splice_tail(groups, &parse_state->list);
+ free(groups);
}
-groups:
+groups: /* A list of groups or events. */
groups ',' group
{
- struct list_head *list = $1;
- struct list_head *group = $3;
+ /* Merge group into the list of events/groups. */
+ struct list_head *groups = $1;
+ struct list_head *group = $3;
- /* frees $3 */
- parse_events_update_lists(group, list);
- $$ = list;
+ list_splice_tail(group, groups);
+ free(group);
+ $$ = groups;
}
|
groups ',' event
{
- struct list_head *list = $1;
+ /* Merge event into the list of events/groups. */
+ struct list_head *groups = $1;
struct list_head *event = $3;
- /* frees $3 */
- parse_events_update_lists(event, list);
- $$ = list;
+
+ list_splice_tail(event, groups);
+ free(event);
+ $$ = groups;
}
|
group
@@ -206,12 +215,12 @@ PE_NAME '{' events '}'
events:
events ',' event
{
+ struct list_head *events = $1;
struct list_head *event = $3;
- struct list_head *list = $1;
- /* frees $3 */
- parse_events_update_lists(event, list);
- $$ = list;
+ list_splice_tail(event, events);
+ free(event);
+ $$ = events;
}
|
event
--
2.44.0.683.g7961c838ac-goog
Inline parse_events_evlist_error that is only used in
parse_events_error. Modify parse_events_error to not report a parser
error unless errors haven't already been reported. Make it clearer
that the latter case only happens for unrecognized input.
Before:
```
$ perf stat -e 'cycles/period=99999999999999999999/' true
event syntax error: 'cycles/period=99999999999999999999/'
\___ parser error
event syntax error: '..les/period=99999999999999999999/'
\___ Bad base 10 number "99999999999999999999"
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
$ perf stat -e 'cycles:xyz' true
event syntax error: 'cycles:xyz'
\___ parser error
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
After:
```
$ perf stat -e 'cycles/period=99999999999999999999/xyz' true
event syntax error: '..les/period=99999999999999999999/xyz'
\___ Bad base 10 number "99999999999999999999"
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
$ perf stat -e 'cycles:xyz' true
event syntax error: 'cycles:xyz'
\___ Unrecognized input
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 9 ---------
tools/perf/util/parse-events.h | 2 --
tools/perf/util/parse-events.y | 10 ++++++++--
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7eba714f0d73..ebada37ef98a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2760,15 +2760,6 @@ int parse_events_terms__to_strbuf(const struct parse_events_terms *terms, struct
return 0;
}
-void parse_events_evlist_error(struct parse_events_state *parse_state,
- int idx, const char *str)
-{
- if (!parse_state->error)
- return;
-
- parse_events_error__handle(parse_state->error, idx, strdup(str), NULL);
-}
-
static void config_terms_list(char *buf, size_t buf_sz)
{
int i;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index e8f2aebea10f..290ae6c72ec5 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -226,8 +226,6 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
void *loc_);
void parse_events__set_leader(char *name, struct list_head *list);
-void parse_events_evlist_error(struct parse_events_state *parse_state,
- int idx, const char *str);
struct event_symbol {
const char *symbol;
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 51490f0f8c50..2c4817e126c1 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -790,9 +790,15 @@ sep_slash_slash_dc: '/' '/' | ':' |
%%
-void parse_events_error(YYLTYPE *loc, void *parse_state,
+void parse_events_error(YYLTYPE *loc, void *_parse_state,
void *scanner __maybe_unused,
char const *msg __maybe_unused)
{
- parse_events_evlist_error(parse_state, loc->last_column, "parser error");
+ struct parse_events_state *parse_state = _parse_state;
+
+ if (!parse_state->error || !list_empty(&parse_state->error->list))
+ return;
+
+ parse_events_error__handle(parse_state->error, loc->last_column,
+ strdup("Unrecognized input"), NULL);
}
--
2.44.0.683.g7961c838ac-goog
Moves 352 bytes from .data to .data.rel.ro.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 4 ++--
tools/perf/util/parse-events.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3ab533d0e653..8d3d692d219d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -39,7 +39,7 @@ static int get_config_terms(const struct parse_events_terms *head_config,
static int parse_events_terms__copy(const struct parse_events_terms *src,
struct parse_events_terms *dest);
-struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
+const struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
[PERF_COUNT_HW_CPU_CYCLES] = {
.symbol = "cpu-cycles",
.alias = "cycles",
@@ -82,7 +82,7 @@ struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
},
};
-struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
+const struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
[PERF_COUNT_SW_CPU_CLOCK] = {
.symbol = "cpu-clock",
.alias = "",
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index f104faef1a78..0bb5f0c80a5e 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -250,8 +250,8 @@ struct event_symbol {
const char *symbol;
const char *alias;
};
-extern struct event_symbol event_symbols_hw[];
-extern struct event_symbol event_symbols_sw[];
+extern const struct event_symbol event_symbols_hw[];
+extern const struct event_symbol event_symbols_sw[];
char *parse_events_formats_error_string(char *additional_terms);
--
2.44.0.683.g7961c838ac-goog
On 2024-04-16 2:15 a.m., Ian Rogers wrote:
> Use the error handler from the parse_state to give a more informative
> error message.
>
> Before:
> ```
> $ perf stat -e 'cycles/period=99999999999999999999/' true
> event syntax error: 'cycles/period=99999999999999999999/'
> \___ parser error
> Run 'perf list' for a list of valid events
>
> Usage: perf stat [<options>] [<command>]
>
> -e, --event <event> event selector. use 'perf list' to list available events
> ```
>
> After:
> ```
> $ perf stat -e 'cycles/period=99999999999999999999/' true
> event syntax error: 'cycles/period=99999999999999999999/'
> \___ parser error
>
> event syntax error: '..les/period=99999999999999999999/'
> \___ Bad base 10 number "99999999999999999999"
It seems the patch only works for decimal?
/perf stat -e 'cycles/period=0xaaaaaaaaaaaaaaaaaaaaaa/' true
event syntax error: '..les/period=0xaaaaaaaaaaaaaaaaaaaaaa/'
\___ parser error
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list
available events
Thanks,
Kan
> Run 'perf list' for a list of valid events
>
> Usage: perf stat [<options>] [<command>]
>
> -e, --event <event> event selector. use 'perf list' to list available events
> ```
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/util/parse-events.l | 40 ++++++++++++++++++++--------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 6fe37003ab7b..0cd68c9f0d4f 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -18,26 +18,34 @@
>
> char *parse_events_get_text(yyscan_t yyscanner);
> YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
> +int parse_events_get_column(yyscan_t yyscanner);
> +int parse_events_get_leng(yyscan_t yyscanner);
>
> -static int __value(YYSTYPE *yylval, char *str, int base, int token)
> +static int get_column(yyscan_t scanner)
> {
> - u64 num;
> -
> - errno = 0;
> - num = strtoull(str, NULL, base);
> - if (errno)
> - return PE_ERROR;
> -
> - yylval->num = num;
> - return token;
> + return parse_events_get_column(scanner) - parse_events_get_leng(scanner);
> }
>
> -static int value(yyscan_t scanner, int base)
> +static int value(struct parse_events_state *parse_state, yyscan_t scanner, int base)
> {
> YYSTYPE *yylval = parse_events_get_lval(scanner);
> char *text = parse_events_get_text(scanner);
> + u64 num;
>
> - return __value(yylval, text, base, PE_VALUE);
> + errno = 0;
> + num = strtoull(text, NULL, base);
> + if (errno) {
> + struct parse_events_error *error = parse_state->error;
> + char *help = NULL;
> +
> + if (asprintf(&help, "Bad base %d number \"%s\"", base, text) > 0)
> + parse_events_error__handle(error, get_column(scanner), help , NULL);
> +
> + return PE_ERROR;
> + }
> +
> + yylval->num = num;
> + return PE_VALUE;
> }
>
> static int str(yyscan_t scanner, int token)
> @@ -283,8 +291,8 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
> */
> "/"/{digit} { return PE_BP_SLASH; }
> "/"/{non_digit} { BEGIN(config); return '/'; }
> -{num_dec} { return value(yyscanner, 10); }
> -{num_hex} { return value(yyscanner, 16); }
> +{num_dec} { return value(_parse_state, yyscanner, 10); }
> +{num_hex} { return value(_parse_state, yyscanner, 16); }
> /*
> * We need to separate 'mem:' scanner part, in order to get specific
> * modifier bits parsed out. Otherwise we would need to handle PE_NAME
> @@ -330,8 +338,8 @@ cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); }
> {lc_type}-{lc_op_result}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); }
> mem: { BEGIN(mem); return PE_PREFIX_MEM; }
> r{num_raw_hex} { return str(yyscanner, PE_RAW); }
> -{num_dec} { return value(yyscanner, 10); }
> -{num_hex} { return value(yyscanner, 16); }
> +{num_dec} { return value(_parse_state, yyscanner, 10); }
> +{num_hex} { return value(_parse_state, yyscanner, 16); }
>
> {modifier_event} { return str(yyscanner, PE_MODIFIER_EVENT); }
> {name} { return str(yyscanner, PE_NAME); }
On Thu, Apr 18, 2024 at 1:27 PM Liang, Kan <[email protected]> wrote:
>
>
>
> On 2024-04-16 2:15 a.m., Ian Rogers wrote:
> > Use the error handler from the parse_state to give a more informative
> > error message.
> >
> > Before:
> > ```
> > $ perf stat -e 'cycles/period=99999999999999999999/' true
> > event syntax error: 'cycles/period=99999999999999999999/'
> > \___ parser error
> > Run 'perf list' for a list of valid events
> >
> > Usage: perf stat [<options>] [<command>]
> >
> > -e, --event <event> event selector. use 'perf list' to list available events
> > ```
> >
> > After:
> > ```
> > $ perf stat -e 'cycles/period=99999999999999999999/' true
> > event syntax error: 'cycles/period=99999999999999999999/'
> > \___ parser error
> >
> > event syntax error: '..les/period=99999999999999999999/'
> > \___ Bad base 10 number "99999999999999999999"
>
>
> It seems the patch only works for decimal?
>
> ./perf stat -e 'cycles/period=0xaaaaaaaaaaaaaaaaaaaaaa/' true
> event syntax error: '..les/period=0xaaaaaaaaaaaaaaaaaaaaaa/'
> \___ parser error
> Run 'perf list' for a list of valid events
>
> Usage: perf stat [<options>] [<command>]
>
> -e, --event <event> event selector. use 'perf list' to list
> available events
>
> Thanks,
> Kan
Right, for hexadecimal we say the number of digits is at most 16, so
when you exceed this the token is no longer recognized. It just
becomes input that can't be parsed, hence parser error. Doing this
means we can simplify other strtoull checks, but I agree having a
better error message for hexadecimal would be good. Let's do it as
follow up.
Thanks,
Ian
> > Run 'perf list' for a list of valid events
> >
> > Usage: perf stat [<options>] [<command>]
> >
> > -e, --event <event> event selector. use 'perf list' to list available events
> > ```
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > tools/perf/util/parse-events.l | 40 ++++++++++++++++++++--------------
> > 1 file changed, 24 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index 6fe37003ab7b..0cd68c9f0d4f 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -18,26 +18,34 @@
> >
> > char *parse_events_get_text(yyscan_t yyscanner);
> > YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
> > +int parse_events_get_column(yyscan_t yyscanner);
> > +int parse_events_get_leng(yyscan_t yyscanner);
> >
> > -static int __value(YYSTYPE *yylval, char *str, int base, int token)
> > +static int get_column(yyscan_t scanner)
> > {
> > - u64 num;
> > -
> > - errno = 0;
> > - num = strtoull(str, NULL, base);
> > - if (errno)
> > - return PE_ERROR;
> > -
> > - yylval->num = num;
> > - return token;
> > + return parse_events_get_column(scanner) - parse_events_get_leng(scanner);
> > }
> >
> > -static int value(yyscan_t scanner, int base)
> > +static int value(struct parse_events_state *parse_state, yyscan_t scanner, int base)
> > {
> > YYSTYPE *yylval = parse_events_get_lval(scanner);
> > char *text = parse_events_get_text(scanner);
> > + u64 num;
> >
> > - return __value(yylval, text, base, PE_VALUE);
> > + errno = 0;
> > + num = strtoull(text, NULL, base);
> > + if (errno) {
> > + struct parse_events_error *error = parse_state->error;
> > + char *help = NULL;
> > +
> > + if (asprintf(&help, "Bad base %d number \"%s\"", base, text) > 0)
> > + parse_events_error__handle(error, get_column(scanner), help , NULL);
> > +
> > + return PE_ERROR;
> > + }
> > +
> > + yylval->num = num;
> > + return PE_VALUE;
> > }
> >
> > static int str(yyscan_t scanner, int token)
> > @@ -283,8 +291,8 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
> > */
> > "/"/{digit} { return PE_BP_SLASH; }
> > "/"/{non_digit} { BEGIN(config); return '/'; }
> > -{num_dec} { return value(yyscanner, 10); }
> > -{num_hex} { return value(yyscanner, 16); }
> > +{num_dec} { return value(_parse_state, yyscanner, 10); }
> > +{num_hex} { return value(_parse_state, yyscanner, 16); }
> > /*
> > * We need to separate 'mem:' scanner part, in order to get specific
> > * modifier bits parsed out. Otherwise we would need to handle PE_NAME
> > @@ -330,8 +338,8 @@ cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); }
> > {lc_type}-{lc_op_result}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); }
> > mem: { BEGIN(mem); return PE_PREFIX_MEM; }
> > r{num_raw_hex} { return str(yyscanner, PE_RAW); }
> > -{num_dec} { return value(yyscanner, 10); }
> > -{num_hex} { return value(yyscanner, 16); }
> > +{num_dec} { return value(_parse_state, yyscanner, 10); }
> > +{num_hex} { return value(_parse_state, yyscanner, 16); }
> >
> > {modifier_event} { return str(yyscanner, PE_MODIFIER_EVENT); }
> > {name} { return str(yyscanner, PE_NAME); }
On 2024-04-18 5:07 p.m., Ian Rogers wrote:
> On Thu, Apr 18, 2024 at 1:27 PM Liang, Kan <[email protected]> wrote:
>>
>>
>>
>> On 2024-04-16 2:15 a.m., Ian Rogers wrote:
>>> Use the error handler from the parse_state to give a more informative
>>> error message.
>>>
>>> Before:
>>> ```
>>> $ perf stat -e 'cycles/period=99999999999999999999/' true
>>> event syntax error: 'cycles/period=99999999999999999999/'
>>> \___ parser error
>>> Run 'perf list' for a list of valid events
>>>
>>> Usage: perf stat [<options>] [<command>]
>>>
>>> -e, --event <event> event selector. use 'perf list' to list available events
>>> ```
>>>
>>> After:
>>> ```
>>> $ perf stat -e 'cycles/period=99999999999999999999/' true
>>> event syntax error: 'cycles/period=99999999999999999999/'
>>> \___ parser error
>>>
>>> event syntax error: '..les/period=99999999999999999999/'
>>> \___ Bad base 10 number "99999999999999999999"
>>
>>
>> It seems the patch only works for decimal?
>>
>> ./perf stat -e 'cycles/period=0xaaaaaaaaaaaaaaaaaaaaaa/' true
>> event syntax error: '..les/period=0xaaaaaaaaaaaaaaaaaaaaaa/'
>> \___ parser error
>> Run 'perf list' for a list of valid events
>>
>> Usage: perf stat [<options>] [<command>]
>>
>> -e, --event <event> event selector. use 'perf list' to list
>> available events
>>
>> Thanks,
>> Kan
>
> Right, for hexadecimal we say the number of digits is at most 16, so
> when you exceed this the token is no longer recognized. It just
> becomes input that can't be parsed, hence parser error. Doing this
> means we can simplify other strtoull checks, but I agree having a
> better error message for hexadecimal would be good. Let's do it as
> follow up.
OK. There is already a warning. It's fine to provide a follow-up for a
better error message later.
Thanks,
Kan
>
> Thanks,
> Ian
>
>>> Run 'perf list' for a list of valid events
>>>
>>> Usage: perf stat [<options>] [<command>]
>>>
>>> -e, --event <event> event selector. use 'perf list' to list available events
>>> ```
>>>
>>> Signed-off-by: Ian Rogers <[email protected]>
>>> ---
>>> tools/perf/util/parse-events.l | 40 ++++++++++++++++++++--------------
>>> 1 file changed, 24 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
>>> index 6fe37003ab7b..0cd68c9f0d4f 100644
>>> --- a/tools/perf/util/parse-events.l
>>> +++ b/tools/perf/util/parse-events.l
>>> @@ -18,26 +18,34 @@
>>>
>>> char *parse_events_get_text(yyscan_t yyscanner);
>>> YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
>>> +int parse_events_get_column(yyscan_t yyscanner);
>>> +int parse_events_get_leng(yyscan_t yyscanner);
>>>
>>> -static int __value(YYSTYPE *yylval, char *str, int base, int token)
>>> +static int get_column(yyscan_t scanner)
>>> {
>>> - u64 num;
>>> -
>>> - errno = 0;
>>> - num = strtoull(str, NULL, base);
>>> - if (errno)
>>> - return PE_ERROR;
>>> -
>>> - yylval->num = num;
>>> - return token;
>>> + return parse_events_get_column(scanner) - parse_events_get_leng(scanner);
>>> }
>>>
>>> -static int value(yyscan_t scanner, int base)
>>> +static int value(struct parse_events_state *parse_state, yyscan_t scanner, int base)
>>> {
>>> YYSTYPE *yylval = parse_events_get_lval(scanner);
>>> char *text = parse_events_get_text(scanner);
>>> + u64 num;
>>>
>>> - return __value(yylval, text, base, PE_VALUE);
>>> + errno = 0;
>>> + num = strtoull(text, NULL, base);
>>> + if (errno) {
>>> + struct parse_events_error *error = parse_state->error;
>>> + char *help = NULL;
>>> +
>>> + if (asprintf(&help, "Bad base %d number \"%s\"", base, text) > 0)
>>> + parse_events_error__handle(error, get_column(scanner), help , NULL);
>>> +
>>> + return PE_ERROR;
>>> + }
>>> +
>>> + yylval->num = num;
>>> + return PE_VALUE;
>>> }
>>>
>>> static int str(yyscan_t scanner, int token)
>>> @@ -283,8 +291,8 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
>>> */
>>> "/"/{digit} { return PE_BP_SLASH; }
>>> "/"/{non_digit} { BEGIN(config); return '/'; }
>>> -{num_dec} { return value(yyscanner, 10); }
>>> -{num_hex} { return value(yyscanner, 16); }
>>> +{num_dec} { return value(_parse_state, yyscanner, 10); }
>>> +{num_hex} { return value(_parse_state, yyscanner, 16); }
>>> /*
>>> * We need to separate 'mem:' scanner part, in order to get specific
>>> * modifier bits parsed out. Otherwise we would need to handle PE_NAME
>>> @@ -330,8 +338,8 @@ cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); }
>>> {lc_type}-{lc_op_result}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); }
>>> mem: { BEGIN(mem); return PE_PREFIX_MEM; }
>>> r{num_raw_hex} { return str(yyscanner, PE_RAW); }
>>> -{num_dec} { return value(yyscanner, 10); }
>>> -{num_hex} { return value(yyscanner, 16); }
>>> +{num_dec} { return value(_parse_state, yyscanner, 10); }
>>> +{num_hex} { return value(_parse_state, yyscanner, 16); }
>>>
>>> {modifier_event} { return str(yyscanner, PE_MODIFIER_EVENT); }
>>> {name} { return str(yyscanner, PE_NAME); }
>
On Mon, Apr 15, 2024 at 11:15 PM Ian Rogers <[email protected]> wrote:
>
> As discussed in:
> https://lore.kernel.org/lkml/[email protected]/
> preferring sysfs/json events consistently (with or without a given
> PMU) will enable RISC-V's hope to customize legacy events in the perf
> tool.
>
Thanks for remapping legacy events in a generic way. This looks great
and got rid of my
ugly arch specific way of remapping. Is there a good way for the
driver (e.g via sysfs) to tell the perf tool
whether to remap the legacy event or not ?
In RISC-V the legacy systems without the new ISA extension may not
want to remap if running
the latest kernel.
I described the problem in detail in the original thread as well.
https://lore.kernel.org/lkml/[email protected]/
FWIW, for the entire series.
Tested-by: Atish Patra <[email protected]>
> Some minor clean-up is performed on the way.
>
> v2. Additional cleanup particularly adding better error messages. Fix
> some line length issues on the earlier patches.
>
> Ian Rogers (16):
> perf parse-events: Factor out '<event_or_pmu>/.../' parsing
> perf parse-events: Directly pass PMU to parse_events_add_pmu
> perf parse-events: Avoid copying an empty list
> perf pmu: Refactor perf_pmu__match
> perf tests parse-events: Use branches rather than cache-references
> perf parse-events: Legacy cache names on all PMUs and lower priority
> perf parse-events: Handle PE_TERM_HW in name_or_raw
> perf parse-events: Constify parse_events_add_numeric
> perf parse-events: Prefer sysfs/json hardware events over legacy
> perf parse-events: Inline parse_events_update_lists
> perf parse-events: Improve error message for bad numbers
> perf parse-events: Inline parse_events_evlist_error
> perf parse-events: Improvements to modifier parsing
> perf parse-event: Constify event_symbol arrays
> perf parse-events: Minor grouping tidy up
> perf parse-events: Tidy the setting of the default event name
>
> tools/perf/tests/parse-events.c | 6 +-
> tools/perf/util/parse-events.c | 482 ++++++++++++++++----------------
> tools/perf/util/parse-events.h | 49 ++--
> tools/perf/util/parse-events.l | 196 +++++++++----
> tools/perf/util/parse-events.y | 261 +++++++----------
> tools/perf/util/pmu.c | 27 +-
> tools/perf/util/pmu.h | 2 +-
> 7 files changed, 540 insertions(+), 483 deletions(-)
>
> --
> 2.44.0.683.g7961c838ac-goog
>
On Tue, Apr 23, 2024 at 5:28 PM Atish Kumar Patra <[email protected]> wrote:
>
> On Mon, Apr 15, 2024 at 11:15 PM Ian Rogers <[email protected]> wrote:
> >
> > As discussed in:
> > https://lore.kernel.org/lkml/[email protected]/
> > preferring sysfs/json events consistently (with or without a given
> > PMU) will enable RISC-V's hope to customize legacy events in the perf
> > tool.
> >
>
> Thanks for remapping legacy events in a generic way. This looks great
> and got rid of my
> ugly arch specific way of remapping. Is there a good way for the
> driver (e.g via sysfs) to tell the perf tool
> whether to remap the legacy event or not ?
>
> In RISC-V the legacy systems without the new ISA extension may not
> want to remap if running
> the latest kernel.
>
> I described the problem in detail in the original thread as well.
> https://lore.kernel.org/lkml/[email protected]/
So the sysfs/json events have priority over the legacy hardware events
with this patch series. I'm not clear on your question but here are
some scenarios:
1) for a vendor/model with a CPUID json files want to be used:
1.1) the driver shouldn't advertise the events /sys/devices/<pmu name>/events
1.2) the json in the perf tool needs to have a mapfile.csv entry for
the cpuid to a model directory containing the event json. In the
directory the legacy events should be defined.
2) for a vendor/model with a CPUID the driver files should be used:
2.1) the driver should advertise the events in /sys/devices/<pmu name>/events
2.2) in the json for the CPUID avoid redefining the events
3) for a vendor/model with a CPUID the legacy events should be used:
3.1) the driver shouldn't advertise the events in /sys/devices/<pmu name>/events
3.2) in the json for the CPUID avoid defining the events
Are you asking to have both sysfs and json events for a model? In this
case, which have priority over the other? It's possible in the pmu.c
code to have a prioritized lookup either from json then sysfs or
vice-versa, at the moment it is first come first served. To some
extent this can be seen on Intel uncore events where there are both
sysfs and json events with the same config, when we reverse map if the
sysfs name is loaded then it is reverse mapped in verbose log or by
perf trace, whilst typically I think the json name is reverse mapped.
Are you asking for the search order to be configurable by the driver?
In the past I've considered that the search order may be configured in
the tool and the user may want to provide their own directory
containing additional events and metrics.
> FWIW, for the entire series.
> Tested-by: Atish Patra <[email protected]>
Thanks, I think we can go ahead to land this. Kan's comment was to ask
for a follow up changing max_precise behavior and I'm hesitant to do
two behavior changes in 1 patch series.
Ian
> > Some minor clean-up is performed on the way.
> >
> > v2. Additional cleanup particularly adding better error messages. Fix
> > some line length issues on the earlier patches.
> >
> > Ian Rogers (16):
> > perf parse-events: Factor out '<event_or_pmu>/.../' parsing
> > perf parse-events: Directly pass PMU to parse_events_add_pmu
> > perf parse-events: Avoid copying an empty list
> > perf pmu: Refactor perf_pmu__match
> > perf tests parse-events: Use branches rather than cache-references
> > perf parse-events: Legacy cache names on all PMUs and lower priority
> > perf parse-events: Handle PE_TERM_HW in name_or_raw
> > perf parse-events: Constify parse_events_add_numeric
> > perf parse-events: Prefer sysfs/json hardware events over legacy
> > perf parse-events: Inline parse_events_update_lists
> > perf parse-events: Improve error message for bad numbers
> > perf parse-events: Inline parse_events_evlist_error
> > perf parse-events: Improvements to modifier parsing
> > perf parse-event: Constify event_symbol arrays
> > perf parse-events: Minor grouping tidy up
> > perf parse-events: Tidy the setting of the default event name
> >
> > tools/perf/tests/parse-events.c | 6 +-
> > tools/perf/util/parse-events.c | 482 ++++++++++++++++----------------
> > tools/perf/util/parse-events.h | 49 ++--
> > tools/perf/util/parse-events.l | 196 +++++++++----
> > tools/perf/util/parse-events.y | 261 +++++++----------
> > tools/perf/util/pmu.c | 27 +-
> > tools/perf/util/pmu.h | 2 +-
> > 7 files changed, 540 insertions(+), 483 deletions(-)
> >
> > --
> > 2.44.0.683.g7961c838ac-goog
> >
On 2024-04-16 2:15 a.m., Ian Rogers wrote:
> As discussed in:
> https://lore.kernel.org/lkml/[email protected]/
> preferring sysfs/json events consistently (with or without a given
> PMU) will enable RISC-V's hope to customize legacy events in the perf
> tool.
>
> Some minor clean-up is performed on the way.
>
> v2. Additional cleanup particularly adding better error messages. Fix
> some line length issues on the earlier patches.
>
> Ian Rogers (16):
> perf parse-events: Factor out '<event_or_pmu>/.../' parsing
> perf parse-events: Directly pass PMU to parse_events_add_pmu
> perf parse-events: Avoid copying an empty list
> perf pmu: Refactor perf_pmu__match
> perf tests parse-events: Use branches rather than cache-references
> perf parse-events: Legacy cache names on all PMUs and lower priority
> perf parse-events: Handle PE_TERM_HW in name_or_raw
> perf parse-events: Constify parse_events_add_numeric
> perf parse-events: Prefer sysfs/json hardware events over legacy
> perf parse-events: Inline parse_events_update_lists
> perf parse-events: Improve error message for bad numbers
> perf parse-events: Inline parse_events_evlist_error
> perf parse-events: Improvements to modifier parsing
> perf parse-event: Constify event_symbol arrays
> perf parse-events: Minor grouping tidy up
> perf parse-events: Tidy the setting of the default event name
>
> tools/perf/tests/parse-events.c | 6 +-
> tools/perf/util/parse-events.c | 482 ++++++++++++++++----------------
> tools/perf/util/parse-events.h | 49 ++--
> tools/perf/util/parse-events.l | 196 +++++++++----
> tools/perf/util/parse-events.y | 261 +++++++----------
> tools/perf/util/pmu.c | 27 +-
> tools/perf/util/pmu.h | 2 +-
> 7 files changed, 540 insertions(+), 483 deletions(-)
>
The series looks good to me.
Reviewed-by: Kan Liang <[email protected]>
Thanks,
Kan
On Mon, Apr 15, 2024 at 11:15:27PM -0700, Ian Rogers wrote:
> Use the error handler from the parse_state to give a more informative
> error message.
>
> Before:
> ```
> $ perf stat -e 'cycles/period=99999999999999999999/' true
> event syntax error: 'cycles/period=99999999999999999999/'
> \___ parser error
> Run 'perf list' for a list of valid events
>
> Usage: perf stat [<options>] [<command>]
>
> -e, --event <event> event selector. use 'perf list' to list available events
> ```
>
> After:
> ```
> $ perf stat -e 'cycles/period=99999999999999999999/' true
> event syntax error: 'cycles/period=99999999999999999999/'
> \___ parser error
>
This ended up in perf-tools-next, will have to look at what this problem
is:
9 11.46 amazonlinux:2 : FAIL gcc version 7.3.1 20180712 (Red Hat 7.3.1-17) (GCC)
yy_size_t parse_events_get_leng (yyscan_t yyscanner );
^~~~~~~~~~~~~~~~~~~~~
util/parse-events.l:22:5: note: previous declaration of 'parse_events_get_leng' was here
int parse_events_get_leng(yyscan_t yyscanner);
^~~~~~~~~~~~~~~~~~~~~
yy_size_t parse_events_get_leng (yyscan_t yyscanner)
^~~~~~~~~~~~~~~~~~~~~
util/parse-events.l:22:5: note: previous declaration of 'parse_events_get_leng' was here
int parse_events_get_leng(yyscan_t yyscanner);
^~~~~~~~~~~~~~~~~~~~~
make[3]: *** [util] Error 2
Unsure if this will appear on the radar on other distros, maybe this is
just something that pops up with older distros...
Ran out of time today...
- Arnaldo
On Fri, Apr 26, 2024 at 10:36:02PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Apr 15, 2024 at 11:15:27PM -0700, Ian Rogers wrote:
> > Use the error handler from the parse_state to give a more informative
> > error message.
> >
> > Before:
> > ```
> > $ perf stat -e 'cycles/period=99999999999999999999/' true
> > event syntax error: 'cycles/period=99999999999999999999/'
> > \___ parser error
> > Run 'perf list' for a list of valid events
> >
> > Usage: perf stat [<options>] [<command>]
> >
> > -e, --event <event> event selector. use 'perf list' to list available events
> > ```
> >
> > After:
> > ```
> > $ perf stat -e 'cycles/period=99999999999999999999/' true
> > event syntax error: 'cycles/period=99999999999999999999/'
> > \___ parser error
> >
>
> This ended up in perf-tools-next, will have to look at what this problem
> is:
>
> 9 11.46 amazonlinux:2 : FAIL gcc version 7.3.1 20180712 (Red Hat 7.3.1-17) (GCC)
> yy_size_t parse_events_get_leng (yyscan_t yyscanner );
> ^~~~~~~~~~~~~~~~~~~~~
> util/parse-events.l:22:5: note: previous declaration of 'parse_events_get_leng' was here
> int parse_events_get_leng(yyscan_t yyscanner);
> ^~~~~~~~~~~~~~~~~~~~~
> yy_size_t parse_events_get_leng (yyscan_t yyscanner)
> ^~~~~~~~~~~~~~~~~~~~~
> util/parse-events.l:22:5: note: previous declaration of 'parse_events_get_leng' was here
> int parse_events_get_leng(yyscan_t yyscanner);
> ^~~~~~~~~~~~~~~~~~~~~
> make[3]: *** [util] Error 2
>
>
> Unsure if this will appear on the radar on other distros, maybe this is
> just something that pops up with older distros...
>
> Ran out of time today...
Context:
perfbuilder@number:~$ export BUILD_TARBALL=http://192.168.86.42/perf/perf-6.9.0-rc5.tar.xz
perfbuilder@number:~$ time dm
1 102.33 almalinux:8 : Ok gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-20) , clang version 16.0.6 (Red Hat 16.0.6-2.module_el8.9.0+3621+df7f7146) flex 2.6.1
2 102.44 almalinux:9 : Ok gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2) , clang version 16.0.6 (Red Hat 16.0.6-1.el9) flex 2.6.4
3 124.34 alpine:3.15 : Ok gcc (Alpine 10.3.1_git20211027) 10.3.1 20211027 , Alpine clang version 12.0.1 flex 2.6.4
4 109.42 alpine:3.16 : Ok gcc (Alpine 11.2.1_git20220219) 11.2.1 20220219 , Alpine clang version 13.0.1 flex 2.6.4
5 90.08 alpine:3.17 : Ok gcc (Alpine 12.2.1_git20220924-r4) 12.2.1 20220924 , Alpine clang version 15.0.7 flex 2.6.4
6 84.85 alpine:3.18 : Ok gcc (Alpine 12.2.1_git20220924-r10) 12.2.1 20220924 , Alpine clang version 16.0.6 flex 2.6.4
7 94.18 alpine:3.19 : Ok gcc (Alpine 13.2.1_git20231014) 13.2.1 20231014 , Alpine clang version 17.0.5 flex 2.6.4
8 95.45 alpine:edge : Ok gcc (Alpine 13.2.1_git20240309) 13.2.1 20240309 , Alpine clang version 17.0.6 flex 2.6.4
9 11.46 amazonlinux:2 : FAIL gcc version 7.3.1 20180712 (Red Hat 7.3.1-17) (GCC)
yy_size_t parse_events_get_leng (yyscan_t yyscanner );
^~~~~~~~~~~~~~~~~~~~~
util/parse-events.l:22:5: note: previous declaration of 'parse_events_get_leng' was here
int parse_events_get_leng(yyscan_t yyscanner);
^~~~~~~~~~~~~~~~~~~~~
yy_size_t parse_events_get_leng (yyscan_t yyscanner)
^~~~~~~~~~~~~~~~~~~~~
util/parse-events.l:22:5: note: previous declaration of 'parse_events_get_leng' was here
int parse_events_get_leng(yyscan_t yyscanner);
^~~~~~~~~~~~~~~~~~~~~
make[3]: *** [util] Error 2
10 88.41 amazonlinux:2023 : Ok gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2) , clang version 15.0.7 (Amazon Linux 15.0.7-3.amzn2023.0.1) flex 2.6.4
11 89.72 amazonlinux:devel : Ok gcc (GCC) 11.3.1 20221121 (Red Hat 11.3.1-4) , clang version 15.0.6 (Amazon Linux 15.0.6-3.amzn2023.0.2) flex 2.6.4
12 115.65 archlinux:base : Ok gcc (GCC) 13.2.1 20230801 , clang version 17.0.6 flex 2.6.4
13 93.87 centos:stream : Ok gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-21) , clang version 17.0.6 (Red Hat 17.0.6-1.module_el8+767+9fa966b8) flex 2.6.1