From 70c90e4a6b2fbe775b662eafefae51f64d627790 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:25 -0700 Subject: perf parse-events: Avoid scanning PMUs before parsing The event parser needs to handle two special cases: 1) legacy events like L1-dcache-load-miss. These event names don't appear in JSON or sysfs, and lookup tables are used for the config value. 2) raw events where 'r0xead' is the same as 'read' unless the PMU has an event called 'read' in which case the event has priority. The previous parser to handle these cases would scan all PMUs for components of event names. These components would then be used to classify in the lexer whether the token should be part of a legacy event, a raw event or an event. The grammar would handle legacy event tokens or recombining the tokens back into a regular event name. The code wasn't PMU specific and had issues around events like AMD's branch-brs that would fail to parse as it expects brs to be a suffix on a legacy event style name: $ perf stat -e branch-brs true event syntax error: 'branch-brs' \___ parser error This change removes processing all PMUs by using the lexer in the form of a regular expression matcher. The lexer will return the token for the longest matched sequence of characters, and in the event of a tie the first. The legacy events are a fixed number of regular expressions, and by matching these before a name token its possible to generate an accurate legacy event token with everything else matching as a name. Because of the lexer change the handling of hyphens in the grammar can be removed as hyphens just become a part of the name. To handle raw events and terms the parser is changed to defer trying to evaluate whether something is a raw event until the PMU is known in the grammar. Once the PMU is known, the events of the PMU can be scanned for the 'read' style problem. A new term type is added for these raw terms, used to enable deferring the evaluation. While this change is large, it has stats of: 170 insertions(+), 436 deletions(-) the bulk of the change is deleting the old approach. It isn't possible to break apart the code added due to the dependencies on how the parts of the parsing work. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-19-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.h | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 86ad4438a2aa..f638542c8638 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -41,14 +41,6 @@ int parse_events_terms(struct list_head *terms, const char *str); int parse_filter(const struct option *opt, const char *str, int unset); int exclude_perf(const struct option *opt, const char *arg, int unset); -enum perf_pmu_event_symbol_type { - PMU_EVENT_SYMBOL_ERR, /* not a PMU EVENT */ - PMU_EVENT_SYMBOL, /* normal style PMU event */ - PMU_EVENT_SYMBOL_PREFIX, /* prefix of pre-suf style event */ - PMU_EVENT_SYMBOL_SUFFIX, /* suffix of pre-suf style event */ - PMU_EVENT_SYMBOL_SUFFIX2, /* suffix of pre-suf2 style event */ -}; - enum { PARSE_EVENTS__TERM_TYPE_NUM, PARSE_EVENTS__TERM_TYPE_STR, @@ -78,6 +70,7 @@ enum { PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT, PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE, PARSE_EVENTS__TERM_TYPE_METRIC_ID, + PARSE_EVENTS__TERM_TYPE_RAW, __PARSE_EVENTS__TERM_TYPE_NR, }; @@ -174,8 +167,7 @@ int parse_events_add_numeric(struct parse_events_state *parse_state, int parse_events_add_tool(struct parse_events_state *parse_state, struct list_head *list, int tool_event); -int parse_events_add_cache(struct list_head *list, int *idx, - char *type, char *op_result1, char *op_result2, +int parse_events_add_cache(struct list_head *list, int *idx, const char *name, struct parse_events_error *error, struct list_head *head_config, struct parse_events_state *parse_state); @@ -198,8 +190,6 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, int parse_events_copy_term_list(struct list_head *old, struct list_head **new); -enum perf_pmu_event_symbol_type -perf_pmu__parse_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); @@ -241,8 +231,6 @@ static inline bool is_sdt_event(char *str __maybe_unused) } #endif /* HAVE_LIBELF_SUPPORT */ -int perf_pmu__test_parse_init(void); - struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx, struct perf_event_attr *attr, const char *name, -- cgit v1.2.3 From 6fd1e5191591f9d55afe4d23fa35af2a5cf8f81f Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:30 -0700 Subject: perf parse-events: Support PMUs for legacy cache events Allow a legacy cache event to be both, for example, "L1-dcache-load-miss" and "cpu/L1-dcache-load-miss/" by introducing a new legacy cache term type. The term type is processed in config_term_pmu, setting both the type in perf_event_attr and the config. The code to determine the config is factored out of parse_events_add_cache and shared. If the PMU doesn't support legacy events, currently just core/hybrid PMUs do, then the term is treated like a PE_NAME term - as before. If only terms are being parsed, such as for perf_pmu__new_alias, then the PE_LEGACY_CACHE token is always parsed as PE_NAME. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-24-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 18 +++++++++++ tools/perf/util/parse-events.c | 70 ++++++++++++++++++++++++++++------------- tools/perf/util/parse-events.h | 3 ++ tools/perf/util/parse-events.l | 9 +++++- tools/perf/util/parse-events.y | 14 ++++++++- tools/perf/util/pmu.c | 5 +++ tools/perf/util/pmu.h | 1 + 7 files changed, 96 insertions(+), 24 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index 43c0778983e5..c3afd0b129bb 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1875,6 +1875,24 @@ static const struct evlist_test test__events_pmu[] = { .check = test__checkevent_raw_pmu, /* 5 */ }, + { + .name = "cpu/L1-dcache-load-miss/", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_genhw, + /* 6 */ + }, + { + .name = "cpu/L1-dcache-load-miss/kp", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_genhw_modifier, + /* 7 */ + }, + { + .name = "cpu/L1-dcache-misses,name=cachepmu/", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_config_cache, + /* 8 */ + }, }; struct terms_test { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index b5d95fce520c..f692dd953593 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -404,33 +404,27 @@ static int config_attr(struct perf_event_attr *attr, struct parse_events_error *err, config_term_func_t config_term); -int parse_events_add_cache(struct list_head *list, int *idx, const char *name, - struct parse_events_error *err, - struct list_head *head_config, - struct parse_events_state *parse_state) +/** + * parse_events__decode_legacy_cache - Search name for the legacy cache event + * name composed of 1, 2 or 3 hyphen + * separated sections. The first section is + * the cache type while the others are the + * optional op and optional result. To make + * life hard the names in the table also + * contain hyphens and the longest name + * should always be selected. + */ +static int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config) { - struct perf_event_attr attr; - LIST_HEAD(config_terms); - const char *config_name, *metric_id; - int cache_type = -1, cache_op = -1, cache_result = -1; - int ret, len; + int len, cache_type = -1, cache_op = -1, cache_result = -1; const char *name_end = &name[strlen(name) + 1]; - bool hybrid; const char *str = name; - /* - * Search str for the legacy cache event name composed of 1, 2 or 3 - * hyphen separated sections. The first section is the cache type while - * the others are the optional op and optional result. To make life hard - * the names in the table also contain hyphens and the longest name - * should always be selected. - */ cache_type = parse_aliases(str, evsel__hw_cache, PERF_COUNT_HW_CACHE_MAX, &len); if (cache_type == -1) return -EINVAL; str += len + 1; - config_name = get_config_name(head_config); if (str < name_end) { cache_op = parse_aliases(str, evsel__hw_cache_op, PERF_COUNT_HW_CACHE_OP_MAX, &len); @@ -471,9 +465,28 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name, if (cache_result == -1) cache_result = PERF_COUNT_HW_CACHE_RESULT_ACCESS; + *config = ((__u64)pmu_type << PERF_PMU_TYPE_SHIFT) | + cache_type | (cache_op << 8) | (cache_result << 16); + return 0; +} + +int parse_events_add_cache(struct list_head *list, int *idx, const char *name, + struct parse_events_error *err, + struct list_head *head_config, + struct parse_events_state *parse_state) +{ + struct perf_event_attr attr; + LIST_HEAD(config_terms); + const char *config_name, *metric_id; + int ret; + bool hybrid; + + memset(&attr, 0, sizeof(attr)); - attr.config = cache_type | (cache_op << 8) | (cache_result << 16); attr.type = PERF_TYPE_HW_CACHE; + ret = parse_events__decode_legacy_cache(name, /*pmu_type=*/0, &attr.config); + if (ret) + return ret; if (head_config) { if (config_attr(&attr, head_config, err, @@ -484,6 +497,7 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name, return -ENOMEM; } + config_name = get_config_name(head_config); metric_id = get_config_metric_id(head_config); ret = parse_events__add_cache_hybrid(list, idx, &attr, config_name ? : name, @@ -1022,6 +1036,7 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = { [PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE] = "aux-sample-size", [PARSE_EVENTS__TERM_TYPE_METRIC_ID] = "metric-id", [PARSE_EVENTS__TERM_TYPE_RAW] = "raw", + [PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE] = "legacy-cache", }; static bool config_term_shrinked; @@ -1199,15 +1214,25 @@ static int config_term_pmu(struct perf_event_attr *attr, struct parse_events_term *term, struct parse_events_error *err) { + if (term->type_term == PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE) { + const struct perf_pmu *pmu = perf_pmu__find_by_type(attr->type); + + if (perf_pmu__supports_legacy_cache(pmu)) { + attr->type = PERF_TYPE_HW_CACHE; + return parse_events__decode_legacy_cache(term->config, pmu->type, + &attr->config); + } else + term->type_term = PARSE_EVENTS__TERM_TYPE_USER; + } if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER || - term->type_term == PARSE_EVENTS__TERM_TYPE_DRV_CFG) + term->type_term == PARSE_EVENTS__TERM_TYPE_DRV_CFG) { /* * Always succeed for sysfs terms, as we dont know * at this point what type they need to have. */ return 0; - else - return config_term_common(attr, term, err); + } + return config_term_common(attr, term, err); } #ifdef HAVE_LIBTRACEEVENT @@ -2147,6 +2172,7 @@ int __parse_events(struct evlist *evlist, const char *str, .evlist = evlist, .stoken = PE_START_EVENTS, .fake_pmu = fake_pmu, + .match_legacy_cache_terms = true, }; int ret; diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index f638542c8638..5acb62c2e00a 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -71,6 +71,7 @@ enum { PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE, PARSE_EVENTS__TERM_TYPE_METRIC_ID, PARSE_EVENTS__TERM_TYPE_RAW, + PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE, __PARSE_EVENTS__TERM_TYPE_NR, }; @@ -122,6 +123,8 @@ struct parse_events_state { int stoken; struct perf_pmu *fake_pmu; char *hybrid_pmu_name; + /* Should PE_LEGACY_NAME tokens be generated for config terms? */ + bool match_legacy_cache_terms; bool wild_card_pmus; }; diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 4b35c099189a..abe0ce681d29 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -63,6 +63,11 @@ static int str(yyscan_t scanner, int token) return token; } +static int lc_str(yyscan_t scanner, const struct parse_events_state *state) +{ + return str(scanner, state->match_legacy_cache_terms ? PE_LEGACY_CACHE : PE_NAME); +} + static bool isbpf_suffix(char *text) { int len = strlen(text); @@ -185,7 +190,6 @@ lc_op_result (load|loads|read|store|stores|write|prefetch|prefetches|speculative %{ struct parse_events_state *_parse_state = parse_events_get_extra(yyscanner); - { int start_token = _parse_state->stoken; @@ -269,6 +273,9 @@ r{num_raw_hex} { return str(yyscanner, PE_RAW); } r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } , { return ','; } "/" { BEGIN(INITIAL); return '/'; } +{lc_type} { return lc_str(yyscanner, _parse_state); } +{lc_type}-{lc_op_result} { return lc_str(yyscanner, _parse_state); } +{lc_type}-{lc_op_result}-{lc_op_result} { return lc_str(yyscanner, _parse_state); } {name_minus} { return str(yyscanner, PE_NAME); } \[all\] { return PE_ARRAY_ALL; } "[" { BEGIN(array); return '['; } diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index e7072b5601c5..f84fa1b132b3 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -723,7 +723,7 @@ event_term $$ = head; } -name_or_raw: PE_RAW | PE_NAME +name_or_raw: PE_RAW | PE_NAME | PE_LEGACY_CACHE event_term: PE_RAW @@ -775,6 +775,18 @@ name_or_raw '=' PE_VALUE_SYM_HW $$ = term; } | +PE_LEGACY_CACHE +{ + struct parse_events_term *term; + + if (parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE, + $1, 1, true, &@1, NULL)) { + free($1); + YYABORT; + } + $$ = term; +} +| PE_NAME { struct parse_events_term *term; diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index cb33d869f1ed..63071d876190 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1650,6 +1650,11 @@ bool is_pmu_core(const char *name) return !strcmp(name, "cpu") || is_arm_pmu_core(name); } +bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu) +{ + return is_pmu_core(pmu->name) || perf_pmu__is_hybrid(pmu->name); +} + static bool pmu_alias_is_duplicate(struct sevent *alias_a, struct sevent *alias_b) { diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h index b9a02dedd473..05702bc4bcf8 100644 --- a/tools/perf/util/pmu.h +++ b/tools/perf/util/pmu.h @@ -220,6 +220,7 @@ void perf_pmu__del_formats(struct list_head *formats); struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu); bool is_pmu_core(const char *name); +bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu); void print_pmu_events(const struct print_callbacks *print_cb, void *print_state); bool pmu_have_event(const char *pname, const char *name); -- cgit v1.2.3 From 2bdf4d7ea9b66e54948297194d564a71504a5bda Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:31 -0700 Subject: perf parse-events: Wildcard legacy cache events It is inconsistent that "perf stat -e instructions-retired" wildcard opens on all PMUs while legacy cache events like "perf stat -e L1-dcache-load-miss" do not. A behavior introduced by hybrid is that a legacy cache event like L1-dcache-load-miss should wildcard open on all hybrid PMUs. Previously hybrid would call to is_event_supported for each PMU, a failure of which results in the event not being added. This isn't done in this case as the parser should just create perf_event_attr and the later open should fail, or the counter give "". If this wants to be avoided then the PMU can be named with the event. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-25-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events-hybrid.c | 33 ----------------- tools/perf/util/parse-events-hybrid.h | 7 ---- tools/perf/util/parse-events.c | 68 ++++++++++++++++++----------------- tools/perf/util/parse-events.h | 3 +- tools/perf/util/parse-events.y | 2 +- 5 files changed, 37 insertions(+), 76 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c index 7c9f9150bad5..d2c0be051d46 100644 --- a/tools/perf/util/parse-events-hybrid.c +++ b/tools/perf/util/parse-events-hybrid.c @@ -179,36 +179,3 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state, return add_raw_hybrid(parse_state, list, attr, name, metric_id, config_terms); } - -int parse_events__add_cache_hybrid(struct list_head *list, int *idx, - struct perf_event_attr *attr, - const char *name, - const char *metric_id, - struct list_head *config_terms, - bool *hybrid, - struct parse_events_state *parse_state) -{ - struct perf_pmu *pmu; - int ret; - - *hybrid = false; - if (!perf_pmu__has_hybrid()) - return 0; - - *hybrid = true; - perf_pmu__for_each_hybrid_pmu(pmu) { - LIST_HEAD(terms); - - if (pmu_cmp(parse_state, pmu)) - continue; - - copy_config_terms(&terms, config_terms); - ret = create_event_hybrid(PERF_TYPE_HW_CACHE, idx, list, - attr, name, metric_id, &terms, pmu); - free_config_terms(&terms); - if (ret) - return ret; - } - - return 0; -} diff --git a/tools/perf/util/parse-events-hybrid.h b/tools/perf/util/parse-events-hybrid.h index cbc05fec02a2..bc2966e73897 100644 --- a/tools/perf/util/parse-events-hybrid.h +++ b/tools/perf/util/parse-events-hybrid.h @@ -15,11 +15,4 @@ int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state, struct list_head *config_terms, bool *hybrid); -int parse_events__add_cache_hybrid(struct list_head *list, int *idx, - struct perf_event_attr *attr, - const char *name, const char *metric_id, - struct list_head *config_terms, - bool *hybrid, - struct parse_events_state *parse_state); - #endif /* __PERF_PARSE_EVENTS_HYBRID_H */ diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index f692dd953593..9f2bbf8f3a81 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -472,46 +472,48 @@ static int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u int parse_events_add_cache(struct list_head *list, int *idx, const char *name, struct parse_events_error *err, - struct list_head *head_config, - struct parse_events_state *parse_state) + struct list_head *head_config) { - struct perf_event_attr attr; - LIST_HEAD(config_terms); - const char *config_name, *metric_id; - int ret; - bool hybrid; + 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); + while ((pmu = perf_pmu__scan(pmu)) != NULL) { + LIST_HEAD(config_terms); + struct perf_event_attr attr; + int ret; - memset(&attr, 0, sizeof(attr)); - attr.type = PERF_TYPE_HW_CACHE; - ret = parse_events__decode_legacy_cache(name, /*pmu_type=*/0, &attr.config); - if (ret) - return ret; + /* Skip unsupported PMUs. */ + if (!perf_pmu__supports_legacy_cache(pmu)) + continue; - if (head_config) { - if (config_attr(&attr, head_config, err, - config_term_common)) - return -EINVAL; + memset(&attr, 0, sizeof(attr)); + attr.type = PERF_TYPE_HW_CACHE; - if (get_config_terms(head_config, &config_terms)) - return -ENOMEM; - } + ret = parse_events__decode_legacy_cache(name, pmu->type, &attr.config); + if (ret) + return ret; - config_name = get_config_name(head_config); - metric_id = get_config_metric_id(head_config); - ret = parse_events__add_cache_hybrid(list, idx, &attr, - config_name ? : name, - metric_id, - &config_terms, - &hybrid, parse_state); - if (hybrid) - goto out_free_terms; + found_supported = true; - ret = add_event(list, idx, &attr, config_name ? : name, metric_id, - &config_terms); -out_free_terms: - free_config_terms(&config_terms); - return ret; + if (head_config) { + if (config_attr(&attr, head_config, err, + config_term_common)) + return -EINVAL; + + if (get_config_terms(head_config, &config_terms)) + return -ENOMEM; + } + + if (__add_event(list, idx, &attr, /*init_attr*/true, config_name ?: name, + metric_id, pmu, &config_terms, /*auto_merge_stats=*/false, + /*cpu_list=*/NULL) == NULL) + return -ENOMEM; + + free_config_terms(&config_terms); + } + return found_supported ? 0 : -EINVAL; } #ifdef HAVE_LIBTRACEEVENT diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 5acb62c2e00a..0c26303f7f63 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -172,8 +172,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_error *error, - struct list_head *head_config, - struct parse_events_state *parse_state); + struct list_head *head_config); int parse_events_add_breakpoint(struct list_head *list, int *idx, u64 addr, char *type, u64 len); int parse_events_add_pmu(struct parse_events_state *parse_state, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index f84fa1b132b3..cc7528558845 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -476,7 +476,7 @@ PE_LEGACY_CACHE opt_event_config list = alloc_list(); ABORT_ON(!list); - err = parse_events_add_cache(list, &parse_state->idx, $1, error, $2, parse_state); + err = parse_events_add_cache(list, &parse_state->idx, $1, error, $2); parse_events_terms__delete($2); free($1); -- cgit v1.2.3 From d7f21df0c991f0909a992c0c7e2d31d4c46d40b4 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:32 -0700 Subject: perf print-events: Print legacy cache events for each PMU Mirroring parse_events_add_cache, list the legacy name alongside its alias with the PMU. Remove the now unnecessary hybrid logic. Note, the alias output removes the event type descriptor, so: L1-dcache-loads [Hardware cache event] becomes: L1-dcache-loads OR cpu/L1-dcache-loads/ Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-26-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 2 +- tools/perf/util/parse-events.h | 1 + tools/perf/util/print-events.c | 88 ++++++++++++++++++++---------------------- 3 files changed, 43 insertions(+), 48 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 9f2bbf8f3a81..ec72f11fb37f 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -414,7 +414,7 @@ static int config_attr(struct perf_event_attr *attr, * contain hyphens and the longest name * should always be selected. */ -static int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config) +int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config) { int len, cache_type = -1, cache_op = -1, cache_result = -1; const char *name_end = &name[strlen(name) + 1]; diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 0c26303f7f63..4e49be290209 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -173,6 +173,7 @@ int parse_events_add_tool(struct parse_events_state *parse_state, int parse_events_add_cache(struct list_head *list, int *idx, const char *name, struct parse_events_error *error, struct list_head *head_config); +int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config); int parse_events_add_breakpoint(struct list_head *list, int *idx, u64 addr, char *type, u64 len); int parse_events_add_pmu(struct parse_events_state *parse_state, diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c index 89ac34a922c9..d148842b205a 100644 --- a/tools/perf/util/print-events.c +++ b/tools/perf/util/print-events.c @@ -230,56 +230,50 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state) int print_hwcache_events(const struct print_callbacks *print_cb, void *print_state) { + struct perf_pmu *pmu = NULL; const char *event_type_descriptor = event_type_descriptors[PERF_TYPE_HW_CACHE]; - for (int type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) { - for (int op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) { - /* skip invalid cache type */ - if (!evsel__is_cache_op_valid(type, op)) - continue; - - for (int res = 0; res < PERF_COUNT_HW_CACHE_RESULT_MAX; res++) { - struct perf_pmu *pmu = NULL; - char name[64]; - - __evsel__hw_cache_type_op_res_name(type, op, res, - name, sizeof(name)); - if (!perf_pmu__has_hybrid()) { - if (is_event_supported(PERF_TYPE_HW_CACHE, - type | (op << 8) | (res << 16))) { - print_cb->print_event(print_state, - "cache", - /*pmu_name=*/NULL, - name, - /*event_alias=*/NULL, - /*scale_unit=*/NULL, - /*deprecated=*/false, - event_type_descriptor, - /*desc=*/NULL, - /*long_desc=*/NULL, - /*encoding_desc=*/NULL); - } + while ((pmu = perf_pmu__scan(pmu)) != NULL) { + /* + * Skip uncore PMUs for performance. PERF_TYPE_HW_CACHE type + * attributes can accept software PMUs in the extended type, so + * also skip. + */ + if (pmu->is_uncore || pmu->type == PERF_TYPE_SOFTWARE) + continue; + + for (int type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) { + for (int op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) { + /* skip invalid cache type */ + if (!evsel__is_cache_op_valid(type, op)) continue; - } - perf_pmu__for_each_hybrid_pmu(pmu) { - if (is_event_supported(PERF_TYPE_HW_CACHE, - type | (op << 8) | (res << 16) | - ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT))) { - char new_name[128]; - snprintf(new_name, sizeof(new_name), - "%s/%s/", pmu->name, name); - print_cb->print_event(print_state, - "cache", - pmu->name, - name, - new_name, - /*scale_unit=*/NULL, - /*deprecated=*/false, - event_type_descriptor, - /*desc=*/NULL, - /*long_desc=*/NULL, - /*encoding_desc=*/NULL); - } + + for (int res = 0; res < PERF_COUNT_HW_CACHE_RESULT_MAX; res++) { + char name[64]; + char alias_name[128]; + __u64 config; + int ret; + + __evsel__hw_cache_type_op_res_name(type, op, res, + name, sizeof(name)); + + ret = parse_events__decode_legacy_cache(name, pmu->type, + &config); + if (ret || !is_event_supported(PERF_TYPE_HW_CACHE, config)) + continue; + snprintf(alias_name, sizeof(alias_name), "%s/%s/", + pmu->name, name); + print_cb->print_event(print_state, + "cache", + pmu->name, + name, + alias_name, + /*scale_unit=*/NULL, + /*deprecated=*/false, + event_type_descriptor, + /*desc=*/NULL, + /*long_desc=*/NULL, + /*encoding_desc=*/NULL); } } } -- cgit v1.2.3 From 8bc75f699c14142021d9ecbf5556ded13a403b64 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:33 -0700 Subject: perf parse-events: Support wildcards on raw events Legacy raw events like r1a open as PERF_TYPE_RAW on non-hybrid systems and on each hybrid PMU on hybrid systems. Rather than iterate hybrid PMUs add a perf_pmu__supports_wildcard_numeric function that says when a numeric event should be opened upon it. If the parsed event specifies the type of the PMU then don't wildcard match PMUs, use the specified PMU type. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-27-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 50 +++++++++++++++++++++++++++++------------- tools/perf/util/parse-events.h | 3 ++- tools/perf/util/parse-events.y | 13 +++++++---- tools/perf/util/pmu.c | 5 +++++ tools/perf/util/pmu.h | 1 + 5 files changed, 52 insertions(+), 20 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index ec72f11fb37f..c8b4ec076825 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -25,7 +25,6 @@ #include "util/parse-branch-options.h" #include "util/evsel_config.h" #include "util/event.h" -#include "util/parse-events-hybrid.h" #include "util/pmu-hybrid.h" #include "util/bpf-filter.h" #include "util/util.h" @@ -1448,15 +1447,14 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx, #endif } -int parse_events_add_numeric(struct parse_events_state *parse_state, - struct list_head *list, - u32 type, u64 config, - struct list_head *head_config) +static int __parse_events_add_numeric(struct parse_events_state *parse_state, + struct list_head *list, + struct perf_pmu *pmu, u32 type, u64 config, + struct list_head *head_config) { struct perf_event_attr attr; LIST_HEAD(config_terms); const char *name, *metric_id; - bool hybrid; int ret; memset(&attr, 0, sizeof(attr)); @@ -1474,19 +1472,41 @@ int parse_events_add_numeric(struct parse_events_state *parse_state, name = get_config_name(head_config); metric_id = get_config_metric_id(head_config); - ret = parse_events__add_numeric_hybrid(parse_state, list, &attr, - name, metric_id, - &config_terms, &hybrid); - if (hybrid) - goto out_free_terms; - - ret = add_event(list, &parse_state->idx, &attr, name, metric_id, - &config_terms); -out_free_terms: + ret = __add_event(list, &parse_state->idx, &attr, /*init_attr*/true, name, + metric_id, pmu, &config_terms, /*auto_merge_stats=*/false, + /*cpu_list=*/NULL) ? 0 : -ENOMEM; free_config_terms(&config_terms); return ret; } +int parse_events_add_numeric(struct parse_events_state *parse_state, + struct list_head *list, + u32 type, u64 config, + struct list_head *head_config, + bool wildcard) +{ + struct perf_pmu *pmu = NULL; + bool found_supported = false; + + if (!wildcard) + return __parse_events_add_numeric(parse_state, list, /*pmu=*/NULL, + type, config, head_config); + + while ((pmu = perf_pmu__scan(pmu)) != NULL) { + int ret; + + if (!perf_pmu__supports_wildcard_numeric(pmu)) + continue; + + found_supported = true; + ret = __parse_events_add_numeric(parse_state, list, pmu, pmu->type, + config, head_config); + if (ret) + return ret; + } + return found_supported ? 0 : -EINVAL; +} + int parse_events_add_tool(struct parse_events_state *parse_state, struct list_head *list, int tool_event) diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 4e49be290209..831cd1ff4702 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -166,7 +166,8 @@ int parse_events_load_bpf_obj(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 list_head *head_config); + struct list_head *head_config, + bool wildcard); int parse_events_add_tool(struct parse_events_state *parse_state, struct list_head *list, int tool_event); diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index cc7528558845..5055a29a448f 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -435,7 +435,8 @@ value_sym '/' event_config '/' list = alloc_list(); ABORT_ON(!list); - err = parse_events_add_numeric(_parse_state, list, type, config, $3); + err = parse_events_add_numeric(_parse_state, list, type, config, $3, + /*wildcard=*/false); parse_events_terms__delete($3); if (err) { free_list_evsel(list); @@ -452,7 +453,9 @@ value_sym sep_slash_slash_dc list = alloc_list(); ABORT_ON(!list); - ABORT_ON(parse_events_add_numeric(_parse_state, list, type, config, NULL)); + ABORT_ON(parse_events_add_numeric(_parse_state, list, type, config, + /*head_config=*/NULL, + /*wildcard=*/false)); $$ = list; } | @@ -596,7 +599,8 @@ PE_VALUE ':' PE_VALUE opt_event_config list = alloc_list(); ABORT_ON(!list); - err = parse_events_add_numeric(_parse_state, list, (u32)$1, $3, $4); + err = parse_events_add_numeric(_parse_state, list, (u32)$1, $3, $4, + /*wildcard=*/false); parse_events_terms__delete($4); if (err) { free(list); @@ -618,7 +622,8 @@ PE_RAW opt_event_config num = strtoull($1 + 1, NULL, 16); ABORT_ON(errno); free($1); - err = parse_events_add_numeric(_parse_state, list, PERF_TYPE_RAW, num, $2); + err = parse_events_add_numeric(_parse_state, list, PERF_TYPE_RAW, num, $2, + /*wildcard=*/true); parse_events_terms__delete($2); if (err) { free(list); diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 63071d876190..cd4247a379d4 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1655,6 +1655,11 @@ bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu) return is_pmu_core(pmu->name) || perf_pmu__is_hybrid(pmu->name); } +bool perf_pmu__supports_wildcard_numeric(const struct perf_pmu *pmu) +{ + return is_pmu_core(pmu->name) || perf_pmu__is_hybrid(pmu->name); +} + static bool pmu_alias_is_duplicate(struct sevent *alias_a, struct sevent *alias_b) { diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h index 05702bc4bcf8..5a19536a5449 100644 --- a/tools/perf/util/pmu.h +++ b/tools/perf/util/pmu.h @@ -221,6 +221,7 @@ struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu); bool is_pmu_core(const char *name); bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu); +bool perf_pmu__supports_wildcard_numeric(const struct perf_pmu *pmu); void print_pmu_events(const struct print_callbacks *print_cb, void *print_state); bool pmu_have_event(const char *pname, const char *name); -- cgit v1.2.3 From 996e54bbee825d25706796672926dbac826a0818 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:34 -0700 Subject: perf parse-events: Remove now unused hybrid logic The event parser no longer needs to recurse in case of a legacy cache event in a PMU, the necessary wild card logic has moved to perf_pmu__supports_legacy_cache and perf_pmu__supports_wildcard_numeric. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-28-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/Build | 1 - tools/perf/util/parse-events-hybrid.c | 181 ---------------------------------- tools/perf/util/parse-events-hybrid.h | 18 ---- tools/perf/util/parse-events.c | 74 -------------- tools/perf/util/parse-events.h | 8 -- 5 files changed, 282 deletions(-) delete mode 100644 tools/perf/util/parse-events-hybrid.c delete mode 100644 tools/perf/util/parse-events-hybrid.h (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/Build b/tools/perf/util/Build index bd18fe5f2719..c146736ead19 100644 --- a/tools/perf/util/Build +++ b/tools/perf/util/Build @@ -24,7 +24,6 @@ perf-y += llvm-utils.o perf-y += mmap.o perf-y += memswap.o perf-y += parse-events.o -perf-y += parse-events-hybrid.o perf-y += print-events.o perf-y += tracepoint.o perf-y += perf_regs.o diff --git a/tools/perf/util/parse-events-hybrid.c b/tools/perf/util/parse-events-hybrid.c deleted file mode 100644 index d2c0be051d46..000000000000 --- a/tools/perf/util/parse-events-hybrid.c +++ /dev/null @@ -1,181 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include -#include -#include -#include -#include -#include -#include -#include "evlist.h" -#include "evsel.h" -#include "parse-events.h" -#include "parse-events-hybrid.h" -#include "debug.h" -#include "pmu.h" -#include "pmu-hybrid.h" -#include "perf.h" - -static void config_hybrid_attr(struct perf_event_attr *attr, - int type, int pmu_type) -{ - /* - * attr.config layout for type PERF_TYPE_HARDWARE and - * PERF_TYPE_HW_CACHE - * - * PERF_TYPE_HARDWARE: 0xEEEEEEEE000000AA - * AA: hardware event ID - * EEEEEEEE: PMU type ID - * PERF_TYPE_HW_CACHE: 0xEEEEEEEE00DDCCBB - * BB: hardware cache ID - * CC: hardware cache op ID - * DD: hardware cache op result ID - * EEEEEEEE: PMU type ID - * If the PMU type ID is 0, the PERF_TYPE_RAW will be applied. - */ - attr->type = type; - attr->config = (attr->config & PERF_HW_EVENT_MASK) | - ((__u64)pmu_type << PERF_PMU_TYPE_SHIFT); -} - -static int create_event_hybrid(__u32 config_type, int *idx, - struct list_head *list, - struct perf_event_attr *attr, const char *name, - const char *metric_id, - struct list_head *config_terms, - struct perf_pmu *pmu) -{ - struct evsel *evsel; - __u32 type = attr->type; - __u64 config = attr->config; - - config_hybrid_attr(attr, config_type, pmu->type); - - /* - * Some hybrid hardware cache events are only available on one CPU - * PMU. For example, the 'L1-dcache-load-misses' is only available - * on cpu_core, while the 'L1-icache-loads' is only available on - * cpu_atom. We need to remove "not supported" hybrid cache events. - */ - if (attr->type == PERF_TYPE_HW_CACHE - && !is_event_supported(attr->type, attr->config)) - return 0; - - evsel = parse_events__add_event_hybrid(list, idx, attr, name, metric_id, - pmu, config_terms); - if (evsel) { - evsel->pmu_name = strdup(pmu->name); - if (!evsel->pmu_name) - return -ENOMEM; - } else - return -ENOMEM; - attr->type = type; - attr->config = config; - return 0; -} - -static int pmu_cmp(struct parse_events_state *parse_state, - struct perf_pmu *pmu) -{ - if (parse_state->evlist && parse_state->evlist->hybrid_pmu_name) - return strcmp(parse_state->evlist->hybrid_pmu_name, pmu->name); - - if (parse_state->hybrid_pmu_name) - return strcmp(parse_state->hybrid_pmu_name, pmu->name); - - return 0; -} - -static int add_hw_hybrid(struct parse_events_state *parse_state, - struct list_head *list, struct perf_event_attr *attr, - const char *name, const char *metric_id, - struct list_head *config_terms) -{ - struct perf_pmu *pmu; - int ret; - - perf_pmu__for_each_hybrid_pmu(pmu) { - LIST_HEAD(terms); - - if (pmu_cmp(parse_state, pmu)) - continue; - - copy_config_terms(&terms, config_terms); - ret = create_event_hybrid(PERF_TYPE_HARDWARE, - &parse_state->idx, list, attr, name, - metric_id, &terms, pmu); - free_config_terms(&terms); - if (ret) - return ret; - } - - return 0; -} - -static int create_raw_event_hybrid(int *idx, struct list_head *list, - struct perf_event_attr *attr, - const char *name, - const char *metric_id, - struct list_head *config_terms, - struct perf_pmu *pmu) -{ - struct evsel *evsel; - - attr->type = pmu->type; - evsel = parse_events__add_event_hybrid(list, idx, attr, name, metric_id, - pmu, config_terms); - if (evsel) - evsel->pmu_name = strdup(pmu->name); - else - return -ENOMEM; - - return 0; -} - -static int add_raw_hybrid(struct parse_events_state *parse_state, - struct list_head *list, struct perf_event_attr *attr, - const char *name, const char *metric_id, - struct list_head *config_terms) -{ - struct perf_pmu *pmu; - int ret; - - perf_pmu__for_each_hybrid_pmu(pmu) { - LIST_HEAD(terms); - - if (pmu_cmp(parse_state, pmu)) - continue; - - copy_config_terms(&terms, config_terms); - ret = create_raw_event_hybrid(&parse_state->idx, list, attr, - name, metric_id, &terms, pmu); - free_config_terms(&terms); - if (ret) - return ret; - } - - return 0; -} - -int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state, - struct list_head *list, - struct perf_event_attr *attr, - const char *name, const char *metric_id, - struct list_head *config_terms, - bool *hybrid) -{ - *hybrid = false; - if (attr->type == PERF_TYPE_SOFTWARE) - return 0; - - if (!perf_pmu__has_hybrid()) - return 0; - - *hybrid = true; - if (attr->type != PERF_TYPE_RAW) { - return add_hw_hybrid(parse_state, list, attr, name, metric_id, - config_terms); - } - - return add_raw_hybrid(parse_state, list, attr, name, metric_id, - config_terms); -} diff --git a/tools/perf/util/parse-events-hybrid.h b/tools/perf/util/parse-events-hybrid.h deleted file mode 100644 index bc2966e73897..000000000000 --- a/tools/perf/util/parse-events-hybrid.h +++ /dev/null @@ -1,18 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef __PERF_PARSE_EVENTS_HYBRID_H -#define __PERF_PARSE_EVENTS_HYBRID_H - -#include -#include -#include -#include -#include - -int parse_events__add_numeric_hybrid(struct parse_events_state *parse_state, - struct list_head *list, - struct perf_event_attr *attr, - const char *name, const char *metric_id, - struct list_head *config_terms, - bool *hybrid); - -#endif /* __PERF_PARSE_EVENTS_HYBRID_H */ diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index c8b4ec076825..1d8c3cf9c185 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -25,7 +25,6 @@ #include "util/parse-branch-options.h" #include "util/evsel_config.h" #include "util/event.h" -#include "util/pmu-hybrid.h" #include "util/bpf-filter.h" #include "util/util.h" #include "tracepoint.h" @@ -39,9 +38,6 @@ extern int parse_events_debug; int parse_events_parse(void *parse_state, void *scanner); static int get_config_terms(struct list_head *head_config, struct list_head *head_terms __maybe_unused); -static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state, - const char *str, char *pmu_name, - struct list_head *list); struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = { [PERF_COUNT_HW_CPU_CYCLES] = { @@ -1526,33 +1522,6 @@ static bool config_term_percore(struct list_head *config_terms) return false; } -static int parse_events__inside_hybrid_pmu(struct parse_events_state *parse_state, - struct list_head *list, char *name, - struct list_head *head_config) -{ - struct parse_events_term *term; - int ret = -1; - - if (parse_state->fake_pmu || !head_config || list_empty(head_config) || - !perf_pmu__is_hybrid(name)) { - return -1; - } - - /* - * More than one term in list. - */ - if (head_config->next && head_config->next->next != head_config) - return -1; - - term = list_first_entry(head_config, struct parse_events_term, list); - if (term && term->config && strcmp(term->config, "event")) { - ret = parse_events__with_hybrid_pmu(parse_state, term->config, - name, list); - } - - return ret; -} - int parse_events_add_pmu(struct parse_events_state *parse_state, struct list_head *list, char *name, struct list_head *head_config, @@ -1642,11 +1611,6 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, if (pmu->default_config && get_config_chgs(pmu, head_config, &config_terms)) return -ENOMEM; - if (!parse_events__inside_hybrid_pmu(parse_state, list, name, - head_config)) { - return 0; - } - if (!parse_state->fake_pmu && perf_pmu__config(pmu, &attr, head_config, parse_state->error)) { free_config_terms(&config_terms); return -EINVAL; @@ -2023,32 +1987,6 @@ int parse_events_terms(struct list_head *terms, const char *str) return ret; } -static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state, - const char *str, char *pmu_name, - struct list_head *list) -{ - struct parse_events_state ps = { - .list = LIST_HEAD_INIT(ps.list), - .stoken = PE_START_EVENTS, - .hybrid_pmu_name = pmu_name, - .idx = parse_state->idx, - }; - int ret; - - ret = parse_events__scanner(str, &ps); - - if (!ret) { - if (!list_empty(&ps.list)) { - list_splice(&ps.list, list); - parse_state->idx = ps.idx; - return 0; - } else - return -1; - } - - return ret; -} - __weak int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs) { /* Order by insertion index. */ @@ -2779,15 +2717,3 @@ char *parse_events_formats_error_string(char *additional_terms) fail: return NULL; } - -struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx, - struct perf_event_attr *attr, - const char *name, - const char *metric_id, - struct perf_pmu *pmu, - struct list_head *config_terms) -{ - return __add_event(list, idx, attr, /*init_attr=*/true, name, metric_id, - pmu, config_terms, /*auto_merge_stats=*/false, - /*cpu_list=*/NULL); -} diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 831cd1ff4702..77b8f7efdb94 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -122,7 +122,6 @@ struct parse_events_state { struct list_head *terms; int stoken; struct perf_pmu *fake_pmu; - char *hybrid_pmu_name; /* Should PE_LEGACY_NAME tokens be generated for config terms? */ bool match_legacy_cache_terms; bool wild_card_pmus; @@ -235,11 +234,4 @@ static inline bool is_sdt_event(char *str __maybe_unused) } #endif /* HAVE_LIBELF_SUPPORT */ -struct evsel *parse_events__add_event_hybrid(struct list_head *list, int *idx, - struct perf_event_attr *attr, - const char *name, - const char *metric_id, - struct perf_pmu *pmu, - struct list_head *config_terms); - #endif /* __PERF_PARSE_EVENTS_H */ -- cgit v1.2.3 From 411ad22ecf0281d666a82aa7f4de90c70365da7d Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:36 -0700 Subject: perf parse-events: Add pmu filter To support the cputype argument added to "perf stat" for hybrid it is necessary to filter events during wildcard matching. Add a scanner argument for the filter and checking it when wildcard matching. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-30-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-record.c | 13 +++++++++-- tools/perf/builtin-stat.c | 10 +++++--- tools/perf/builtin-top.c | 5 +++- tools/perf/builtin-trace.c | 5 +++- tools/perf/tests/parse-events.c | 3 ++- tools/perf/tests/pmu-events.c | 3 ++- tools/perf/util/evlist.h | 1 - tools/perf/util/metricgroup.c | 4 ++-- tools/perf/util/parse-events.c | 51 ++++++++++++++++++++++++++++++----------- tools/perf/util/parse-events.h | 21 +++++++++++++---- tools/perf/util/parse-events.y | 6 +++-- 11 files changed, 90 insertions(+), 32 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index efa03e4ac2c9..ec0f2d5f189f 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -3335,6 +3335,14 @@ const char record_callchain_help[] = CALLCHAIN_RECORD_HELP static bool dry_run; +static struct parse_events_option_args parse_events_option_args = { + .evlistp = &record.evlist, +}; + +static struct parse_events_option_args switch_output_parse_events_option_args = { + .evlistp = &record.sb_evlist, +}; + /* * XXX Will stay a global variable till we fix builtin-script.c to stop messing * with it and switch to use the library functions in perf_evlist that came @@ -3343,7 +3351,7 @@ static bool dry_run; * using pipes, etc. */ static struct option __record_options[] = { - OPT_CALLBACK('e', "event", &record.evlist, "event", + OPT_CALLBACK('e', "event", &parse_events_option_args, "event", "event selector. use 'perf list' to list available events", parse_events_option), OPT_CALLBACK(0, "filter", &record.evlist, "filter", @@ -3496,7 +3504,8 @@ static struct option __record_options[] = { &record.switch_output.set, "signal or size[BKMG] or time[smhd]", "Switch output when receiving SIGUSR2 (signal) or cross a size or time threshold", "signal"), - OPT_CALLBACK_SET(0, "switch-output-event", &record.sb_evlist, &record.switch_output_event_set, "switch output event", + OPT_CALLBACK_SET(0, "switch-output-event", &switch_output_parse_events_option_args, + &record.switch_output_event_set, "switch output event", "switch output event selector. use 'perf list' to list available events", parse_events_option_new_evlist), OPT_INTEGER(0, "switch-max-files", &record.switch_output.num_files, diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index b9ad32f21e57..de2e915427c9 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -101,6 +101,10 @@ static void print_counters(struct timespec *ts, int argc, const char **argv); static struct evlist *evsel_list; +static struct parse_events_option_args parse_events_option_args = { + .evlistp = &evsel_list, +}; + static bool all_counters_use_bpf = true; static struct target target = { @@ -1096,8 +1100,8 @@ static int parse_hybrid_type(const struct option *opt, return -1; } - evlist->hybrid_pmu_name = perf_pmu__hybrid_type_to_pmu(str); - if (!evlist->hybrid_pmu_name) { + parse_events_option_args.pmu_filter = perf_pmu__hybrid_type_to_pmu(str); + if (!parse_events_option_args.pmu_filter) { fprintf(stderr, "--cputype %s is not supported!\n", str); return -1; } @@ -1108,7 +1112,7 @@ static int parse_hybrid_type(const struct option *opt, static struct option stat_options[] = { OPT_BOOLEAN('T', "transaction", &transaction_run, "hardware transaction statistics"), - OPT_CALLBACK('e', "event", &evsel_list, "event", + OPT_CALLBACK('e', "event", &parse_events_option_args, "event", "event selector. use 'perf list' to list available events", parse_events_option), OPT_CALLBACK(0, "filter", &evsel_list, "filter", diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index eb5740154bc0..48ee49e95c5e 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1440,12 +1440,15 @@ int cmd_top(int argc, const char **argv) .max_stack = sysctl__max_stack(), .nr_threads_synthesize = UINT_MAX, }; + struct parse_events_option_args parse_events_option_args = { + .evlistp = &top.evlist, + }; bool branch_call_mode = false; struct record_opts *opts = &top.record_opts; struct target *target = &opts->target; const char *disassembler_style = NULL, *objdump_path = NULL, *addr2line_path = NULL; const struct option options[] = { - OPT_CALLBACK('e', "event", &top.evlist, "event", + OPT_CALLBACK('e', "event", &parse_events_option_args, "event", "event selector. use 'perf list' to list available events", parse_events_option), OPT_U64('c', "count", &opts->user_interval, "event period to sample"), diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 8ee3a45c3c54..b49d3abb1203 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -4591,8 +4591,11 @@ do_concat: err = 0; if (lists[0]) { + struct parse_events_option_args parse_events_option_args = { + .evlistp = &trace->evlist, + }; struct option o = { - .value = &trace->evlist, + .value = &parse_events_option_args, }; err = parse_events_option(&o, lists[0], 0); } diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index c3afd0b129bb..0d0c869d2d09 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1952,7 +1952,8 @@ static int test_event_fake_pmu(const char *str) return -ENOMEM; parse_events_error__init(&err); - ret = __parse_events(evlist, str, &err, &perf_pmu__fake, /*warn_if_reordered=*/true); + ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err, + &perf_pmu__fake, /*warn_if_reordered=*/true); if (ret) { pr_debug("failed to parse event '%s', err %d, str '%s'\n", str, ret, err.str); diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c index a2cde61b1c77..734004f1a37d 100644 --- a/tools/perf/tests/pmu-events.c +++ b/tools/perf/tests/pmu-events.c @@ -776,7 +776,8 @@ static int check_parse_id(const char *id, struct parse_events_error *error, for (cur = strchr(dup, '@') ; cur; cur = strchr(++cur, '@')) *cur = '/'; - ret = __parse_events(evlist, dup, error, fake_pmu, /*warn_if_reordered=*/true); + ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, fake_pmu, + /*warn_if_reordered=*/true); free(dup); evlist__delete(evlist); diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 46cf402add93..e7e5540cc970 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -67,7 +67,6 @@ struct evlist { struct evsel *selected; struct events_stats stats; struct perf_env *env; - const char *hybrid_pmu_name; void (*trace_event_sample_raw)(struct evlist *evlist, union perf_event *event, struct perf_sample *sample); diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 4e7d41d285b4..908124dab122 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -1441,8 +1441,8 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu, } pr_debug("Parsing metric events '%s'\n", events.buf); parse_events_error__init(&parse_error); - ret = __parse_events(parsed_evlist, events.buf, &parse_error, fake_pmu, - /*warn_if_reordered=*/false); + ret = __parse_events(parsed_evlist, events.buf, /*pmu_filter=*/NULL, + &parse_error, fake_pmu, /*warn_if_reordered=*/false); if (ret) { parse_events_error__print(&parse_error, events.buf); goto err_out; diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 1d8c3cf9c185..d9d964bbc0e2 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -465,8 +465,24 @@ int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *con return 0; } +/** + * parse_events__filter_pmu - returns false if a wildcard PMU should be + * considered, true if it should be filtered. + */ +bool parse_events__filter_pmu(const struct parse_events_state *parse_state, + const struct perf_pmu *pmu) +{ + if (parse_state->pmu_filter == NULL) + return false; + + if (pmu->name == NULL) + return true; + + return strcmp(parse_state->pmu_filter, pmu->name) != 0; +} + int parse_events_add_cache(struct list_head *list, int *idx, const char *name, - struct parse_events_error *err, + struct parse_events_state *parse_state, struct list_head *head_config) { struct perf_pmu *pmu = NULL; @@ -483,6 +499,9 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name, if (!perf_pmu__supports_legacy_cache(pmu)) continue; + if (parse_events__filter_pmu(parse_state, pmu)) + continue; + memset(&attr, 0, sizeof(attr)); attr.type = PERF_TYPE_HW_CACHE; @@ -493,8 +512,7 @@ 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, err, - config_term_common)) + if (config_attr(&attr, head_config, parse_state->error, config_term_common)) return -EINVAL; if (get_config_terms(head_config, &config_terms)) @@ -1494,6 +1512,9 @@ int parse_events_add_numeric(struct parse_events_state *parse_state, if (!perf_pmu__supports_wildcard_numeric(pmu)) continue; + if (parse_events__filter_pmu(parse_state, pmu)) + continue; + found_supported = true; ret = __parse_events_add_numeric(parse_state, list, pmu, pmu->type, config, head_config); @@ -1682,6 +1703,9 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, while ((pmu = perf_pmu__scan(pmu)) != NULL) { struct perf_pmu_alias *alias; + if (parse_events__filter_pmu(parse_state, pmu)) + continue; + list_for_each_entry(alias, &pmu->aliases, list) { if (!strcasecmp(alias->name, str)) { parse_events_copy_term_list(head, &orig_head); @@ -2121,7 +2145,7 @@ static bool parse_events__sort_events_and_fix_groups(struct list_head *list) return idx_changed || num_leaders != orig_num_leaders; } -int __parse_events(struct evlist *evlist, const char *str, +int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter, struct parse_events_error *err, struct perf_pmu *fake_pmu, bool warn_if_reordered) { @@ -2132,6 +2156,7 @@ int __parse_events(struct evlist *evlist, const char *str, .evlist = evlist, .stoken = PE_START_EVENTS, .fake_pmu = fake_pmu, + .pmu_filter = pmu_filter, .match_legacy_cache_terms = true, }; int ret; @@ -2313,12 +2338,13 @@ void parse_events_error__print(struct parse_events_error *err, int parse_events_option(const struct option *opt, const char *str, int unset __maybe_unused) { - struct evlist *evlist = *(struct evlist **)opt->value; + struct parse_events_option_args *args = opt->value; struct parse_events_error err; int ret; parse_events_error__init(&err); - ret = parse_events(evlist, str, &err); + ret = __parse_events(*args->evlistp, str, args->pmu_filter, &err, + /*fake_pmu=*/NULL, /*warn_if_reordered=*/true); if (ret) { parse_events_error__print(&err, str); @@ -2331,22 +2357,21 @@ int parse_events_option(const struct option *opt, const char *str, int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset) { - struct evlist **evlistp = opt->value; + struct parse_events_option_args *args = opt->value; int ret; - if (*evlistp == NULL) { - *evlistp = evlist__new(); + if (*args->evlistp == NULL) { + *args->evlistp = evlist__new(); - if (*evlistp == NULL) { + if (*args->evlistp == NULL) { fprintf(stderr, "Not enough memory to create evlist\n"); return -1; } } - ret = parse_events_option(opt, str, unset); if (ret) { - evlist__delete(*evlistp); - *evlistp = NULL; + evlist__delete(*args->evlistp); + *args->evlistp = NULL; } return ret; diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 77b8f7efdb94..d4cbda6e946a 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -22,17 +22,24 @@ bool is_event_supported(u8 type, u64 config); const char *event_type(int type); +/* Arguments encoded in opt->value. */ +struct parse_events_option_args { + struct evlist **evlistp; + const char *pmu_filter; +}; int parse_events_option(const struct option *opt, const char *str, int unset); int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset); -__attribute__((nonnull(1, 2, 3))) -int __parse_events(struct evlist *evlist, const char *str, struct parse_events_error *error, - struct perf_pmu *fake_pmu, bool warn_if_reordered); +__attribute__((nonnull(1, 2, 4))) +int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter, + struct parse_events_error *error, struct perf_pmu *fake_pmu, + bool warn_if_reordered); __attribute__((nonnull(1, 2, 3))) static inline int parse_events(struct evlist *evlist, const char *str, struct parse_events_error *err) { - return __parse_events(evlist, str, err, /*fake_pmu=*/NULL, /*warn_if_reordered=*/true); + return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/NULL, + /*warn_if_reordered=*/true); } int parse_event(struct evlist *evlist, const char *str); @@ -122,11 +129,15 @@ struct parse_events_state { struct list_head *terms; int stoken; struct perf_pmu *fake_pmu; + /* If non-null, when wildcard matching only match the given PMU. */ + const char *pmu_filter; /* Should PE_LEGACY_NAME tokens be generated for config terms? */ bool match_legacy_cache_terms; bool wild_card_pmus; }; +bool parse_events__filter_pmu(const struct parse_events_state *parse_state, + const struct perf_pmu *pmu); void parse_events__shrink_config_terms(void); int parse_events__is_hardcoded_term(struct parse_events_term *term); int parse_events_term__num(struct parse_events_term **term, @@ -171,7 +182,7 @@ int parse_events_add_tool(struct parse_events_state *parse_state, struct list_head *list, int tool_event); int parse_events_add_cache(struct list_head *list, int *idx, const char *name, - struct parse_events_error *error, + struct parse_events_state *parse_state, struct list_head *head_config); int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config); int parse_events_add_breakpoint(struct list_head *list, int *idx, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index e709508b1d6e..c95877cbd6cf 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -312,6 +312,9 @@ PE_NAME opt_pmu_config while ((pmu = perf_pmu__scan(pmu)) != NULL) { char *name = pmu->name; + if (parse_events__filter_pmu(parse_state, pmu)) + continue; + if (!strncmp(name, "uncore_", 7) && strncmp($1, "uncore_", 7)) name += 7; @@ -473,13 +476,12 @@ event_legacy_cache: PE_LEGACY_CACHE opt_event_config { struct parse_events_state *parse_state = _parse_state; - struct parse_events_error *error = parse_state->error; struct list_head *list; int err; list = alloc_list(); ABORT_ON(!list); - err = parse_events_add_cache(list, &parse_state->idx, $1, error, $2); + err = parse_events_add_cache(list, &parse_state->idx, $1, parse_state, $2); parse_events_terms__delete($2); free($1); -- cgit v1.2.3 From 5ea8f2ccffb23983f02012a2731464586b10fbf3 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:40 -0700 Subject: perf parse-events: Support hardware events as terms An event like "cpu/instructions/" typically parses due to there being a sysfs event called instructions. On hybrid recursive parsing means that the hardware event is encoded in the attribute, with the PMU being placed in the high bits of the config: ''' $ perf stat -vv -e 'cpu_core/cycles/' true ... ------------------------------------------------------------ perf_event_attr: size 136 config 0x400000000 sample_type IDENTIFIER read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING disabled 1 inherit 1 enable_on_exec 1 exclude_guest 1 ------------------------------------------------------------ ''' Make this behavior the default by adding a new term type and token for hardware events. The token gathers both the numeric config and the parsed name, so that if the token appears like "cycles/name=cycles/" then the token can be handled like a name. The numeric value isn't sufficient to distinguish say "cpu-cycles" from "cycles". Extend the parse-events test so that all current non-PMU hardware parsing tests, also test with the PMU cpu - more than half the change. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-34-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 126 ++++++++++++++++++++++++++++++++++++++++ tools/perf/util/parse-events.c | 37 ++++-------- tools/perf/util/parse-events.h | 3 +- tools/perf/util/parse-events.l | 20 +++++++ tools/perf/util/parse-events.y | 34 +++++++++-- 5 files changed, 187 insertions(+), 33 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index 71c77d9d2744..9ca8e19bda00 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1926,6 +1926,132 @@ static const struct evlist_test test__events_pmu[] = { .check = test__checkevent_config_cache, /* 8 */ }, + { + .name = "cpu/instructions/", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_symbolic_name, + /* 9 */ + }, + { + .name = "cpu/cycles,period=100000,config2/", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_symbolic_name_config, + /* 0 */ + }, + { + .name = "cpu/instructions/h", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_symbolic_name_modifier, + /* 1 */ + }, + { + .name = "cpu/instructions/G", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_exclude_host_modifier, + /* 2 */ + }, + { + .name = "cpu/instructions/H", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_exclude_guest_modifier, + /* 3 */ + }, + { + .name = "{cpu/instructions/k,cpu/cycles/upp}", + .valid = test__pmu_cpu_valid, + .check = test__group1, + /* 4 */ + }, + { + .name = "{cpu/cycles/u,cpu/instructions/kp}:p", + .valid = test__pmu_cpu_valid, + .check = test__group4, + /* 5 */ + }, + { + .name = "{cpu/cycles/,cpu/cache-misses/G}:H", + .valid = test__pmu_cpu_valid, + .check = test__group_gh1, + /* 6 */ + }, + { + .name = "{cpu/cycles/,cpu/cache-misses/H}:G", + .valid = test__pmu_cpu_valid, + .check = test__group_gh2, + /* 7 */ + }, + { + .name = "{cpu/cycles/G,cpu/cache-misses/H}:u", + .valid = test__pmu_cpu_valid, + .check = test__group_gh3, + /* 8 */ + }, + { + .name = "{cpu/cycles/G,cpu/cache-misses/H}:uG", + .valid = test__pmu_cpu_valid, + .check = test__group_gh4, + /* 9 */ + }, + { + .name = "{cpu/cycles/,cpu/cache-misses/,cpu/branch-misses/}:S", + .valid = test__pmu_cpu_valid, + .check = test__leader_sample1, + /* 0 */ + }, + { + .name = "{cpu/instructions/,cpu/branch-misses/}:Su", + .valid = test__pmu_cpu_valid, + .check = test__leader_sample2, + /* 1 */ + }, + { + .name = "cpu/instructions/uDp", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_pinned_modifier, + /* 2 */ + }, + { + .name = "{cpu/cycles/,cpu/cache-misses/,cpu/branch-misses/}:D", + .valid = test__pmu_cpu_valid, + .check = test__pinned_group, + /* 3 */ + }, + { + .name = "cpu/instructions/I", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_exclude_idle_modifier, + /* 4 */ + }, + { + .name = "cpu/instructions/kIG", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_exclude_idle_modifier_1, + /* 5 */ + }, + { + .name = "cpu/cycles/u", + .valid = test__pmu_cpu_valid, + .check = test__sym_event_slash, + /* 6 */ + }, + { + .name = "cpu/cycles/k", + .valid = test__pmu_cpu_valid, + .check = test__sym_event_dc, + /* 7 */ + }, + { + .name = "cpu/instructions/uep", + .valid = test__pmu_cpu_valid, + .check = test__checkevent_exclusive_modifier, + /* 8 */ + }, + { + .name = "{cpu/cycles/,cpu/cache-misses/,cpu/branch-misses/}:e", + .valid = test__pmu_cpu_valid, + .check = test__exclusive_group, + /* 9 */ + }, }; struct terms_test { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index d9d964bbc0e2..dea27bc0b376 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1052,6 +1052,7 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = { [PARSE_EVENTS__TERM_TYPE_METRIC_ID] = "metric-id", [PARSE_EVENTS__TERM_TYPE_RAW] = "raw", [PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE] = "legacy-cache", + [PARSE_EVENTS__TERM_TYPE_HARDWARE] = "hardware", }; static bool config_term_shrinked; @@ -1239,6 +1240,17 @@ static int config_term_pmu(struct perf_event_attr *attr, } else term->type_term = PARSE_EVENTS__TERM_TYPE_USER; } + if (term->type_term == PARSE_EVENTS__TERM_TYPE_HARDWARE) { + const struct perf_pmu *pmu = perf_pmu__find_by_type(attr->type); + + if (!pmu) { + pr_debug("Failed to find PMU for type %d", attr->type); + return -EINVAL; + } + attr->type = PERF_TYPE_HARDWARE; + attr->config = ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT) | term->val.num; + return 0; + } if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER || term->type_term == PARSE_EVENTS__TERM_TYPE_DRV_CFG) { /* @@ -2569,31 +2581,6 @@ int parse_events_term__str(struct parse_events_term **term, return new_term(term, &temp, str, 0); } -int parse_events_term__sym_hw(struct parse_events_term **term, - char *config, unsigned idx) -{ - struct event_symbol *sym; - char *str; - struct parse_events_term temp = { - .type_val = PARSE_EVENTS__TERM_TYPE_STR, - .type_term = PARSE_EVENTS__TERM_TYPE_USER, - .config = config, - }; - - if (!temp.config) { - temp.config = strdup("event"); - if (!temp.config) - return -ENOMEM; - } - BUG_ON(idx >= PERF_COUNT_HW_MAX); - sym = &event_symbols_hw[idx]; - - str = strdup(sym->symbol); - if (!str) - return -ENOMEM; - return new_term(term, &temp, str, 0); -} - int parse_events_term__clone(struct parse_events_term **new, struct parse_events_term *term) { diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index d4cbda6e946a..7fe80b416143 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -79,6 +79,7 @@ enum { PARSE_EVENTS__TERM_TYPE_METRIC_ID, PARSE_EVENTS__TERM_TYPE_RAW, PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE, + PARSE_EVENTS__TERM_TYPE_HARDWARE, __PARSE_EVENTS__TERM_TYPE_NR, }; @@ -147,8 +148,6 @@ int parse_events_term__num(struct parse_events_term **term, int parse_events_term__str(struct parse_events_term **term, int type_term, char *config, char *str, void *loc_term, void *loc_val); -int parse_events_term__sym_hw(struct parse_events_term **term, - char *config, unsigned idx); int parse_events_term__clone(struct parse_events_term **new, struct parse_events_term *term); void parse_events_term__delete(struct parse_events_term *term); diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index abe0ce681d29..6deb70c25984 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -149,6 +149,16 @@ static int term(yyscan_t scanner, int type) return PE_TERM; } +static int hw_term(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; + return PE_TERM_HW; +} + #define YY_USER_ACTION \ do { \ yylloc->last_column = yylloc->first_column; \ @@ -269,6 +279,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); } r{num_raw_hex} { return str(yyscanner, PE_RAW); } r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } , { return ','; } diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index c95877cbd6cf..819a5123fd77 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -65,6 +65,7 @@ static void free_list_evsel(struct list_head* list_evsel) %token PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE %token PE_ARRAY_ALL PE_ARRAY_RANGE %token PE_DRV_CFG_TERM +%token PE_TERM_HW %type PE_VALUE %type PE_VALUE_SYM_HW %type PE_VALUE_SYM_SW @@ -112,6 +113,8 @@ static void free_list_evsel(struct list_head* list_evsel) %type array_term %type array_terms %destructor { free ($$.ranges); } +%type PE_TERM_HW +%destructor { free ($$.str); } %union { @@ -125,6 +128,10 @@ static void free_list_evsel(struct list_head* list_evsel) char *event; } tracepoint_name; struct parse_events_array array; + struct hardware_term { + char *str; + u64 num; + } hardware_term; } %% @@ -770,13 +777,14 @@ name_or_raw '=' PE_VALUE $$ = term; } | -name_or_raw '=' PE_VALUE_SYM_HW +name_or_raw '=' PE_TERM_HW { struct parse_events_term *term; - int config = $3 & 255; - if (parse_events_term__sym_hw(&term, $1, config)) { + if (parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER, + $1, $3.str, &@1, &@3)) { free($1); + free($3.str); YYABORT; } $$ = term; @@ -806,12 +814,15 @@ PE_NAME $$ = term; } | -PE_VALUE_SYM_HW +PE_TERM_HW { struct parse_events_term *term; - int config = $1 & 255; - ABORT_ON(parse_events_term__sym_hw(&term, NULL, config)); + if (parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_HARDWARE, + $1.str, $1.num & 255, false, &@1, NULL)) { + free($1.str); + YYABORT; + } $$ = term; } | @@ -826,6 +837,17 @@ PE_TERM '=' PE_NAME $$ = term; } | +PE_TERM '=' PE_TERM_HW +{ + struct parse_events_term *term; + + if (parse_events_term__str(&term, (int)$1, NULL, $3.str, &@1, &@3)) { + free($3.str); + YYABORT; + } + $$ = term; +} +| PE_TERM '=' PE_VALUE { struct parse_events_term *term; -- cgit v1.2.3 From e831f3ccf9920fa099d4ebb9d57214cc7ecd2e70 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:41 -0700 Subject: perf parse-events: Avoid error when assigning a term Avoid the parser error: ''' $ perf stat -e 'cycles/name=name/' true event syntax error: 'cycles/name=name/' \___ parser error ''' by turning the term back to a string if it is on the right. Add PMU and generic parsing tests. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-35-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 21 +++++++++++++++++++++ tools/perf/util/parse-events.c | 9 +++++++++ tools/perf/util/parse-events.h | 3 +++ tools/perf/util/parse-events.y | 8 ++++++++ 4 files changed, 41 insertions(+) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index 9ca8e19bda00..eb893cc15878 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1499,6 +1499,16 @@ static int test__sym_event_dc(struct evlist *evlist) return TEST_OK; } +static int test__term_equal_term(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + TEST_ASSERT_VAL("wrong type", evsel->core.attr.type == PERF_TYPE_HARDWARE); + TEST_ASSERT_VAL("wrong config", test_config(evsel, PERF_COUNT_HW_CPU_CYCLES)); + TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "name") == 0); + return TEST_OK; +} + #ifdef HAVE_LIBTRACEEVENT static int count_tracepoints(void) { @@ -1871,6 +1881,11 @@ static const struct evlist_test test__events[] = { .check = test__exclusive_group, /* 7 */ }, + { + .name = "cycles/name=name/", + .check = test__term_equal_term, + /* 8 */ + }, }; static const struct evlist_test test__events_pmu[] = { @@ -2052,6 +2067,12 @@ static const struct evlist_test test__events_pmu[] = { .check = test__exclusive_group, /* 9 */ }, + { + .name = "cpu/cycles,name=name/", + .valid = test__pmu_cpu_valid, + .check = test__term_equal_term, + /* 0 */ + }, }; struct terms_test { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index dea27bc0b376..5d5d77fa398b 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2581,6 +2581,15 @@ int parse_events_term__str(struct parse_events_term **term, return new_term(term, &temp, str, 0); } +int parse_events_term__term(struct parse_events_term **term, + int term_lhs, int term_rhs, + void *loc_term, void *loc_val) +{ + return parse_events_term__str(term, term_lhs, NULL, + strdup(config_term_names[term_rhs]), + loc_term, loc_val); +} + int parse_events_term__clone(struct parse_events_term **new, struct parse_events_term *term) { diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 7fe80b416143..2a8cafe0ee8f 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -148,6 +148,9 @@ int parse_events_term__num(struct parse_events_term **term, int parse_events_term__str(struct parse_events_term **term, int type_term, char *config, char *str, void *loc_term, void *loc_val); +int parse_events_term__term(struct parse_events_term **term, + int term_lhs, int term_rhs, + void *loc_term, void *loc_val); int parse_events_term__clone(struct parse_events_term **new, struct parse_events_term *term); void parse_events_term__delete(struct parse_events_term *term); diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 819a5123fd77..0aaebc57748e 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -848,6 +848,14 @@ PE_TERM '=' PE_TERM_HW $$ = term; } | +PE_TERM '=' PE_TERM +{ + struct parse_events_term *term; + + ABORT_ON(parse_events_term__term(&term, (int)$1, (int)$3, &@1, &@3)); + $$ = term; +} +| PE_TERM '=' PE_VALUE { struct parse_events_term *term; -- cgit v1.2.3 From 9a1bc9ea01e2e95ed56801ed946b310f5562abfc Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 May 2023 15:38:51 -0700 Subject: perf parse-events: Reduce scope of is_event_supported Move to print-events.c and make static. Signed-off-by: Ian Rogers Tested-by: Kan Liang Cc: Adrian Hunter Cc: Ahmad Yasin Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Caleb Biggers Cc: Edward Baker Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kang Minchul Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Perry Taylor Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Rob Herring Cc: Samantha Alt Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Suzuki Poulouse Cc: Thomas Richter Cc: Tiezhu Yang Cc: Weilin Wang Cc: Xing Zhengjun Cc: Yang Jihong Link: https://lore.kernel.org/r/20230502223851.2234828-45-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 39 --------------------------------------- tools/perf/util/parse-events.h | 2 -- tools/perf/util/print-events.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 41 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 2dad88a6bf19..b93264f8a37c 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -28,7 +28,6 @@ #include "util/bpf-filter.h" #include "util/util.h" #include "tracepoint.h" -#include "thread_map.h" #define MAX_NAME_LEN 100 @@ -133,44 +132,6 @@ struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = { }, }; -bool is_event_supported(u8 type, u64 config) -{ - bool ret = true; - int open_return; - struct evsel *evsel; - struct perf_event_attr attr = { - .type = type, - .config = config, - .disabled = 1, - }; - struct perf_thread_map *tmap = thread_map__new_by_tid(0); - - if (tmap == NULL) - return false; - - evsel = evsel__new(&attr); - if (evsel) { - open_return = evsel__open(evsel, NULL, tmap); - ret = open_return >= 0; - - if (open_return == -EACCES) { - /* - * This happens if the paranoid value - * /proc/sys/kernel/perf_event_paranoid is set to 2 - * Re-run with exclude_kernel set; we don't do that - * by default as some ARM machines do not support it. - * - */ - evsel->core.attr.exclude_kernel = 1; - ret = evsel__open(evsel, NULL, tmap) >= 0; - } - evsel__delete(evsel); - } - - perf_thread_map__put(tmap); - return ret; -} - const char *event_type(int type) { switch (type) { diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 2a8cafe0ee8f..2021fe145410 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -18,8 +18,6 @@ struct parse_events_error; struct option; struct perf_pmu; -bool is_event_supported(u8 type, u64 config); - const char *event_type(int type); /* Arguments encoded in opt->value. */ diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c index d148842b205a..69492cbd6921 100644 --- a/tools/perf/util/print-events.c +++ b/tools/perf/util/print-events.c @@ -27,6 +27,7 @@ #include "tracepoint.h" #include "pfm.h" #include "pmu-hybrid.h" +#include "thread_map.h" #define MAX_NAME_LEN 100 @@ -228,6 +229,44 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state) strlist__delete(sdtlist); } +static bool is_event_supported(u8 type, u64 config) +{ + bool ret = true; + int open_return; + struct evsel *evsel; + struct perf_event_attr attr = { + .type = type, + .config = config, + .disabled = 1, + }; + struct perf_thread_map *tmap = thread_map__new_by_tid(0); + + if (tmap == NULL) + return false; + + evsel = evsel__new(&attr); + if (evsel) { + open_return = evsel__open(evsel, NULL, tmap); + ret = open_return >= 0; + + if (open_return == -EACCES) { + /* + * This happens if the paranoid value + * /proc/sys/kernel/perf_event_paranoid is set to 2 + * Re-run with exclude_kernel set; we don't do that + * by default as some ARM machines do not support it. + * + */ + evsel->core.attr.exclude_kernel = 1; + ret = evsel__open(evsel, NULL, tmap) >= 0; + } + evsel__delete(evsel); + } + + perf_thread_map__put(tmap); + return ret; +} + int print_hwcache_events(const struct print_callbacks *print_cb, void *print_state) { struct perf_pmu *pmu = NULL; -- cgit v1.2.3 From f0617f526cb0c482dd46ed798db28d3991f6f872 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Thu, 25 May 2023 11:29:02 +0300 Subject: perf parse: Allow config terms with breakpoints Add config terms to the parsing of breakpoint events. Extend "Test event parsing" to also cover using a confg term. This makes breakpoint events consistent with other events which already support config terms. Example: $ cat dr_test.c #include #include void func0(void) { } int main() { printf("func0 %p\n", &func0); while (1) { func0(); usleep(100000); } return 0; } $ gcc -g -O0 -o dr_test dr_test.c $ ./dr_test & [2] 19646 func0 0x55feb98dd169 $ perf record -e mem:0x55feb98dd169:x/name=breakpoint/ -p 19646 -- sleep 0.5 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.017 MB perf.data (5 samples) ] $ perf script dr_test 19646 5632.956628: 1 breakpoint: 55feb98dd169 func0+0x0 (/home/ahunter/git/work/dr_test) dr_test 19646 5633.056866: 1 breakpoint: 55feb98dd169 func0+0x0 (/home/ahunter/git/work/dr_test) dr_test 19646 5633.157084: 1 breakpoint: 55feb98dd169 func0+0x0 (/home/ahunter/git/work/dr_test) dr_test 19646 5633.257309: 1 breakpoint: 55feb98dd169 func0+0x0 (/home/ahunter/git/work/dr_test) dr_test 19646 5633.357532: 1 breakpoint: 55feb98dd169 func0+0x0 (/home/ahunter/git/work/dr_test) $ sudo perf test "Test event parsing" 6: Parse event definition strings : 6.1: Test event parsing : Ok $ sudo perf test -v "Test event parsing" |& grep mem running test 8 'mem:0' running test 9 'mem:0:x' running test 10 'mem:0:r' running test 11 'mem:0:w' running test 19 'mem:0:u' running test 20 'mem:0:x:k' running test 21 'mem:0:r:hp' running test 22 'mem:0:w:up' running test 26 'mem:0:rw' running test 27 'mem:0:rw:kp' running test 42 'mem:0/1' running test 43 'mem:0/2:w' running test 44 'mem:0/4:rw:u' running test 58 'mem:0/name=breakpoint/' running test 59 'mem:0:x/name=breakpoint/' running test 60 'mem:0:r/name=breakpoint/' running test 61 'mem:0:w/name=breakpoint/' running test 62 'mem:0/name=breakpoint/u' running test 63 'mem:0:x/name=breakpoint/k' running test 64 'mem:0:r/name=breakpoint/hp' running test 65 'mem:0:w/name=breakpoint/up' running test 66 'mem:0:rw/name=breakpoint/' running test 67 'mem:0:rw/name=breakpoint/kp' running test 68 'mem:0/1/name=breakpoint/' running test 69 'mem:0/2:w/name=breakpoint/' running test 70 'mem:0/4:rw/name=breakpoint/u' running test 71 'mem:0/1/name=breakpoint1/,mem:0/4:rw/name=breakpoint2/' Committer notes: Folded follow up patch (see 2nd link below) to address warnings about unused tokens: perf tools: Suppress bison unused value warnings Patch "perf tools: Allow config terms with breakpoints" introduced parse tokens for colons and slashes within breakpoint parsing to prevent mix up with colons and slashes related to config terms. The token values are not needed but introduce bison "unused value" warnings. Suppress those warnings. Committer testing: # cat ~acme/c/mem_breakpoint.c #include #include void func1(void) { } void func2(void) { } void func3(void) { } void func4(void) { } void func5(void) { } int main() { printf("func1 %p\n", &func1); printf("func2 %p\n", &func2); printf("func3 %p\n", &func3); printf("func4 %p\n", &func4); printf("func5 %p\n", &func5); while (1) { func1(); func2(); func3(); func4(); func5(); usleep(100000); } return 0; } # ~acme/c/mem_breakpoint & [1] 3186153 func1 0x401136 func2 0x40113d func3 0x401144 func4 0x40114b func5 0x401152 # Trying to watch the first 4 functions for eXecutable access: # perf record -e mem:0x401136:x/name=breakpoint1/,mem:0x40113d:x/name=breakpoint2/,mem:0x401144:x/name=breakpoint3/,mem:0x40114b:x/name=breakpoint4/ -p 3186153 -- sleep 0.5 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.026 MB perf.data (20 samples) ] [root@five ~]# perf script mem_breakpoint 3186153 131612.864793: 1 breakpoint1: 401136 func1+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.864795: 1 breakpoint2: 40113d func2+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.864796: 1 breakpoint3: 401144 func3+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.864797: 1 breakpoint4: 40114b func4+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.964868: 1 breakpoint1: 401136 func1+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.964870: 1 breakpoint2: 40113d func2+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.964871: 1 breakpoint3: 401144 func3+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131612.964872: 1 breakpoint4: 40114b func4+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.064945: 1 breakpoint1: 401136 func1+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.064948: 1 breakpoint2: 40113d func2+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.064948: 1 breakpoint3: 401144 func3+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.064949: 1 breakpoint4: 40114b func4+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.165024: 1 breakpoint1: 401136 func1+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.165026: 1 breakpoint2: 40113d func2+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.165027: 1 breakpoint3: 401144 func3+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.165028: 1 breakpoint4: 40114b func4+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.265103: 1 breakpoint1: 401136 func1+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.265105: 1 breakpoint2: 40113d func2+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.265106: 1 breakpoint3: 401144 func3+0x0 (/var/home/acme/c/mem_breakpoint) mem_breakpoint 3186153 131613.265107: 1 breakpoint4: 40114b func4+0x0 (/var/home/acme/c/mem_breakpoint) # Then all the 5 functions: # perf record -e mem:0x401136:x/name=breakpoint1/,mem:0x40113d:x/name=breakpoint2/,mem:0x401144:x/name=breakpoint3/,mem:0x40114b:x/name=breakpoint4/,mem:0x401152:x/name=breakpoint5/ -p 3186153 -- sleep 0.5 Error: The sys_perf_event_open() syscall returned with 28 (No space left on device) for event (breakpoint5). /bin/dmesg | grep -i perf may provide additional information. # grep -m1 'model name' /proc/cpuinfo model name : AMD Ryzen 9 5950X 16-Core Processor # Reviewed-by: Ian Rogers Signed-off-by: Adrian Hunter Tested-by: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Namhyung Kim Link: https://lore.kernel.org/r/20230525082902.25332-2-adrian.hunter@intel.com Link: https://lore.kernel.org/r/f7228dc9-fe18-a8e3-7d3f-52922e0e1113@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 157 ++++++++++++++++++++++++++++++++++++++++ tools/perf/util/parse-events.c | 23 +++++- tools/perf/util/parse-events.h | 10 ++- tools/perf/util/parse-events.l | 23 +++++- tools/perf/util/parse-events.y | 49 ++++++++----- 5 files changed, 235 insertions(+), 27 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index bba1cd655a1d..133218e51ab4 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -486,6 +486,93 @@ static int test__checkevent_breakpoint_rw_modifier(struct evlist *evlist) return test__checkevent_breakpoint_rw(evlist); } +static int test__checkevent_breakpoint_modifier_name(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + 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); + TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); + TEST_ASSERT_VAL("wrong name", + !strcmp(evsel__name(evsel), "breakpoint")); + + return test__checkevent_breakpoint(evlist); +} + +static int test__checkevent_breakpoint_x_modifier_name(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + 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); + TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); + TEST_ASSERT_VAL("wrong name", + !strcmp(evsel__name(evsel), "breakpoint")); + + return test__checkevent_breakpoint_x(evlist); +} + +static int test__checkevent_breakpoint_r_modifier_name(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + 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); + TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); + TEST_ASSERT_VAL("wrong name", + !strcmp(evsel__name(evsel), "breakpoint")); + + return test__checkevent_breakpoint_r(evlist); +} + +static int test__checkevent_breakpoint_w_modifier_name(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + 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); + TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); + TEST_ASSERT_VAL("wrong name", + !strcmp(evsel__name(evsel), "breakpoint")); + + return test__checkevent_breakpoint_w(evlist); +} + +static int test__checkevent_breakpoint_rw_modifier_name(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + 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); + TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); + TEST_ASSERT_VAL("wrong name", + !strcmp(evsel__name(evsel), "breakpoint")); + + return test__checkevent_breakpoint_rw(evlist); +} + +static int test__checkevent_breakpoint_2_events(struct evlist *evlist) +{ + struct evsel *evsel = evlist__first(evlist); + + TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); + + TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type); + TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint1")); + + evsel = evsel__next(evsel); + + TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type); + TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint2")); + + return TEST_OK; +} + static int test__checkevent_pmu(struct evlist *evlist) { @@ -1973,6 +2060,76 @@ static const struct evlist_test test__events[] = { .check = test__term_equal_legacy, /* 9 */ }, + { + .name = "mem:0/name=breakpoint/", + .check = test__checkevent_breakpoint, + /* 0 */ + }, + { + .name = "mem:0:x/name=breakpoint/", + .check = test__checkevent_breakpoint_x, + /* 1 */ + }, + { + .name = "mem:0:r/name=breakpoint/", + .check = test__checkevent_breakpoint_r, + /* 2 */ + }, + { + .name = "mem:0:w/name=breakpoint/", + .check = test__checkevent_breakpoint_w, + /* 3 */ + }, + { + .name = "mem:0/name=breakpoint/u", + .check = test__checkevent_breakpoint_modifier_name, + /* 4 */ + }, + { + .name = "mem:0:x/name=breakpoint/k", + .check = test__checkevent_breakpoint_x_modifier_name, + /* 5 */ + }, + { + .name = "mem:0:r/name=breakpoint/hp", + .check = test__checkevent_breakpoint_r_modifier_name, + /* 6 */ + }, + { + .name = "mem:0:w/name=breakpoint/up", + .check = test__checkevent_breakpoint_w_modifier_name, + /* 7 */ + }, + { + .name = "mem:0:rw/name=breakpoint/", + .check = test__checkevent_breakpoint_rw, + /* 8 */ + }, + { + .name = "mem:0:rw/name=breakpoint/kp", + .check = test__checkevent_breakpoint_rw_modifier_name, + /* 9 */ + }, + { + .name = "mem:0/1/name=breakpoint/", + .check = test__checkevent_breakpoint_len, + /* 0 */ + }, + { + .name = "mem:0/2:w/name=breakpoint/", + .check = test__checkevent_breakpoint_len_w, + /* 1 */ + }, + { + .name = "mem:0/4:rw/name=breakpoint/u", + .check = test__checkevent_breakpoint_len_rw_modifier, + /* 2 */ + }, + { + .name = "mem:0/1/name=breakpoint1/,mem:0/4:rw/name=breakpoint2/", + .check = test__checkevent_breakpoint_2_events, + /* 3 */ + }, }; static const struct evlist_test test__events_pmu[] = { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 629f7bd9fd59..2d36cadf35ec 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -946,10 +946,14 @@ do { \ return 0; } -int parse_events_add_breakpoint(struct list_head *list, int *idx, - u64 addr, char *type, u64 len) +int parse_events_add_breakpoint(struct parse_events_state *parse_state, + struct list_head *list, + u64 addr, char *type, u64 len, + struct list_head *head_config __maybe_unused) { struct perf_event_attr attr; + LIST_HEAD(config_terms); + const char *name; memset(&attr, 0, sizeof(attr)); attr.bp_addr = addr; @@ -970,8 +974,19 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx, attr.type = PERF_TYPE_BREAKPOINT; attr.sample_period = 1; - return add_event(list, idx, &attr, /*name=*/NULL, /*mertic_id=*/NULL, - /*config_terms=*/NULL); + if (head_config) { + if (config_attr(&attr, head_config, parse_state->error, + config_term_common)) + return -EINVAL; + + if (get_config_terms(head_config, &config_terms)) + return -ENOMEM; + } + + name = get_config_name(head_config); + + return add_event(list, &parse_state->idx, &attr, name, /*mertic_id=*/NULL, + &config_terms); } static int check_type_val(struct parse_events_term *term, diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 2021fe145410..5fdc1f33f57e 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -185,8 +185,10 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name, struct parse_events_state *parse_state, struct list_head *head_config); int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config); -int parse_events_add_breakpoint(struct list_head *list, int *idx, - u64 addr, char *type, u64 len); +int parse_events_add_breakpoint(struct parse_events_state *parse_state, + struct list_head *list, + u64 addr, char *type, u64 len, + struct list_head *head_config); int parse_events_add_pmu(struct parse_events_state *parse_state, struct list_head *list, char *name, struct list_head *head_config, @@ -226,6 +228,10 @@ void parse_events_error__handle(struct parse_events_error *err, int idx, void parse_events_error__print(struct parse_events_error *err, const char *event); +static inline void parse_events_unused_value(const void *x __maybe_unused) +{ +} + #ifdef HAVE_LIBELF_SUPPORT /* * If the probe point starts with '%', diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 6deb70c25984..7629af3d5c7c 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -190,11 +190,16 @@ name [a-zA-Z_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]* name_tag [\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\'] name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.:]* drv_cfg_term [a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)? -/* If you add a modifier you need to update check_modifier() */ +/* + * If you add a modifier you need to update check_modifier(). + * Also, the letters in modifier_event must not be in modifier_bp. + */ modifier_event [ukhpPGHSDIWeb]+ modifier_bp [rwx]{1,3} lc_type (L1-dcache|l1-d|l1d|L1-data|L1-icache|l1-i|l1i|L1-instruction|LLC|L2|dTLB|d-tlb|Data-TLB|iTLB|i-tlb|Instruction-TLB|branch|branches|bpu|btb|bpc|node) lc_op_result (load|loads|read|store|stores|write|prefetch|prefetches|speculative-read|speculative-load|refs|Reference|ops|access|misses|miss) +digit [0-9] +non_digit [^0-9] %% @@ -304,8 +309,20 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } { {modifier_bp} { return str(yyscanner, PE_MODIFIER_BP); } -: { return ':'; } -"/" { return '/'; } + /* + * The colon before memory access modifiers can get mixed up with the + * colon before event modifiers. Fortunately none of the option letters + * are the same, so trailing context can be used disambiguate the two + * cases. + */ +":"/{modifier_bp} { return str(yyscanner, PE_BP_COLON); } + /* + * The slash before memory length can get mixed up with the slash before + * config terms. Fortunately config terms do not start with a numeric + * digit, so trailing context can be used disambiguate the two cases. + */ +"/"/{digit} { return str(yyscanner, PE_BP_SLASH); } +"/"/{non_digit} { BEGIN(config); return '/'; } {num_dec} { return value(yyscanner, 10); } {num_hex} { return value(yyscanner, 16); } /* diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index f96afb0edd0c..0c3d086cc22a 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -59,7 +59,7 @@ static void free_list_evsel(struct list_head* list_evsel) %token PE_EVENT_NAME %token PE_RAW PE_NAME %token PE_BPF_OBJECT PE_BPF_SOURCE -%token PE_MODIFIER_EVENT PE_MODIFIER_BP +%token PE_MODIFIER_EVENT PE_MODIFIER_BP PE_BP_COLON PE_BP_SLASH %token PE_LEGACY_CACHE %token PE_PREFIX_MEM PE_PREFIX_RAW PE_PREFIX_GROUP %token PE_ERROR @@ -80,6 +80,8 @@ static void free_list_evsel(struct list_head* list_evsel) %type PE_LEGACY_CACHE %type PE_MODIFIER_EVENT %type PE_MODIFIER_BP +%type PE_BP_COLON +%type PE_BP_SLASH %type PE_EVENT_NAME %type PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE %type PE_DRV_CFG_TERM @@ -275,7 +277,7 @@ event_def event_def: event_pmu | event_legacy_symbol | event_legacy_cache sep_dc | - event_legacy_mem | + event_legacy_mem sep_dc | event_legacy_tracepoint sep_dc | event_legacy_numeric sep_dc | event_legacy_raw sep_dc | @@ -503,16 +505,19 @@ PE_LEGACY_CACHE opt_event_config } event_legacy_mem: -PE_PREFIX_MEM PE_VALUE '/' PE_VALUE ':' PE_MODIFIER_BP sep_dc +PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config { - struct parse_events_state *parse_state = _parse_state; struct list_head *list; int err; + parse_events_unused_value(&$3); + parse_events_unused_value(&$5); + list = alloc_list(); ABORT_ON(!list); - err = parse_events_add_breakpoint(list, &parse_state->idx, - $2, $6, $4); + err = parse_events_add_breakpoint(_parse_state, list, + $2, $6, $4, $7); + parse_events_terms__delete($7); free($6); if (err) { free(list); @@ -521,31 +526,37 @@ PE_PREFIX_MEM PE_VALUE '/' PE_VALUE ':' PE_MODIFIER_BP sep_dc $$ = list; } | -PE_PREFIX_MEM PE_VALUE '/' PE_VALUE sep_dc +PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE opt_event_config { - struct parse_events_state *parse_state = _parse_state; struct list_head *list; + int err; + + parse_events_unused_value(&$3); list = alloc_list(); ABORT_ON(!list); - if (parse_events_add_breakpoint(list, &parse_state->idx, - $2, NULL, $4)) { + err = parse_events_add_breakpoint(_parse_state, list, + $2, NULL, $4, $5); + parse_events_terms__delete($5); + if (err) { free(list); YYABORT; } $$ = list; } | -PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc +PE_PREFIX_MEM PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config { - struct parse_events_state *parse_state = _parse_state; struct list_head *list; int err; + parse_events_unused_value(&$3); + list = alloc_list(); ABORT_ON(!list); - err = parse_events_add_breakpoint(list, &parse_state->idx, - $2, $4, 0); + err = parse_events_add_breakpoint(_parse_state, list, + $2, $4, 0, $5); + parse_events_terms__delete($5); free($4); if (err) { free(list); @@ -554,15 +565,17 @@ PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc $$ = list; } | -PE_PREFIX_MEM PE_VALUE sep_dc +PE_PREFIX_MEM PE_VALUE opt_event_config { - struct parse_events_state *parse_state = _parse_state; struct list_head *list; + int err; list = alloc_list(); ABORT_ON(!list); - if (parse_events_add_breakpoint(list, &parse_state->idx, - $2, NULL, 0)) { + err = parse_events_add_breakpoint(_parse_state, list, + $2, NULL, 0, $3); + parse_events_terms__delete($3); + if (err) { free(list); YYABORT; } -- cgit v1.2.3 From 6ec9503f45740b6ae4cb7f3e7441b1539c6d51d6 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 13 Jun 2023 11:26:29 -0700 Subject: perf parse-events: Avoid string for PE_BP_COLON, PE_BP_SLASH There's no need to read the string ':' or '/' for PE_BP_COLON or PE_BP_SLASH and doing so causes parse-events.y to leak memory. The original patch has a committer note about not using these tokens presumably as yacc spotted they were a memory leak because no %destructor could be run. Remove the unused token workaround as there is now no value associated with these tokens. Fixes: f0617f526cb0c482 ("perf parse: Allow config terms with breakpoints") Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Rob Herring Link: https://lore.kernel.org/r/20230613182629.1500317-1-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.h | 4 ---- tools/perf/util/parse-events.l | 4 ++-- tools/perf/util/parse-events.y | 9 --------- 3 files changed, 2 insertions(+), 15 deletions(-) (limited to 'tools/perf/util/parse-events.h') diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 5fdc1f33f57e..b0eb95f93e9c 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -228,10 +228,6 @@ void parse_events_error__handle(struct parse_events_error *err, int idx, void parse_events_error__print(struct parse_events_error *err, const char *event); -static inline void parse_events_unused_value(const void *x __maybe_unused) -{ -} - #ifdef HAVE_LIBELF_SUPPORT /* * If the probe point starts with '%', diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 7629af3d5c7c..99335ec586ae 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -315,13 +315,13 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } * are the same, so trailing context can be used disambiguate the two * cases. */ -":"/{modifier_bp} { return str(yyscanner, PE_BP_COLON); } +":"/{modifier_bp} { return PE_BP_COLON; } /* * The slash before memory length can get mixed up with the slash before * config terms. Fortunately config terms do not start with a numeric * digit, so trailing context can be used disambiguate the two cases. */ -"/"/{digit} { return str(yyscanner, PE_BP_SLASH); } +"/"/{digit} { return PE_BP_SLASH; } "/"/{non_digit} { BEGIN(config); return '/'; } {num_dec} { return value(yyscanner, 10); } {num_hex} { return value(yyscanner, 16); } diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 0c3d086cc22a..9f28d4b5502f 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -80,8 +80,6 @@ static void free_list_evsel(struct list_head* list_evsel) %type PE_LEGACY_CACHE %type PE_MODIFIER_EVENT %type PE_MODIFIER_BP -%type PE_BP_COLON -%type PE_BP_SLASH %type PE_EVENT_NAME %type PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE %type PE_DRV_CFG_TERM @@ -510,9 +508,6 @@ PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event struct list_head *list; int err; - parse_events_unused_value(&$3); - parse_events_unused_value(&$5); - list = alloc_list(); ABORT_ON(!list); err = parse_events_add_breakpoint(_parse_state, list, @@ -531,8 +526,6 @@ PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE opt_event_config struct list_head *list; int err; - parse_events_unused_value(&$3); - list = alloc_list(); ABORT_ON(!list); err = parse_events_add_breakpoint(_parse_state, list, @@ -550,8 +543,6 @@ PE_PREFIX_MEM PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config struct list_head *list; int err; - parse_events_unused_value(&$3); - list = alloc_list(); ABORT_ON(!list); err = parse_events_add_breakpoint(_parse_state, list, -- cgit v1.2.3