From 4bb311b29e82c795340a6e4a5db83d8ccbe2dc49 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Sat, 11 Mar 2023 18:15:39 -0800 Subject: perf parse-events: Pass ownership of the group name Pass ownership of the group name rather than copying and freeing the original. This saves a memory allocation and copy. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kim Phillips Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Stephane Eranian Cc: Suzuki Poulouse Cc: Xing Zhengjun Link: https://lore.kernel.org/r/20230312021543.3060328-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 0336ff27c15f..1be454697d57 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1761,6 +1761,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list, handled: ret = 1; + free(name); out: free(leaders); return ret; @@ -1786,7 +1787,7 @@ void parse_events__set_leader(char *name, struct list_head *list, leader = arch_evlist__leader(list); __perf_evlist__set_leader(list, &leader->core); - leader->group_name = name ? strdup(name) : NULL; + leader->group_name = name; list_move(&leader->core.node, list); } -- cgit v1.2.3-59-g8ed1b From 347c2f0a0988c59c402148054ef54648853fa669 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Sat, 11 Mar 2023 18:15:40 -0800 Subject: perf parse-events: Sort and group parsed events This change is intended to be a no-op for most current cases, the default sort order is the order the events were parsed. Where it varies is in how groups are handled. Previously an uncore and core event that are grouped would most often cause the group to be removed: ``` $ perf stat -e '{instructions,uncore_imc_free_running_0/data_total/}' -a sleep 1 WARNING: grouped events cpus do not match, disabling group: anon group { instructions, uncore_imc_free_running_0/data_total/ } ... ``` However, when wildcards are used the events should be re-sorted and re-grouped in parse_events__set_leader, but this currently fails for simple examples: ``` $ perf stat -e '{uncore_imc_free_running/data_read/,uncore_imc_free_running/data_write/}' -a sleep 1 Performance counter stats for 'system wide': MiB uncore_imc_free_running/data_read/ MiB uncore_imc_free_running/data_write/ 1.000996992 seconds time elapsed ``` A futher failure mode, fixed in this patch, is to force topdown events into a group. This change moves sorting the evsels in the evlist after parsing. It requires parsing to set up groups. First the evsels are sorted respecting the existing groupings and parse order, but also reordering to ensure evsels of the same PMU and group appear together. So that software and aux events respect groups, their pmu_name is taken from the group leader. The sorting is done with list_sort removing a memory allocation. After sorting a pass is done to correct the group leaders and for topdown events ensuring they have a group leader. This fixes the problems seen before: ``` $ perf stat -e '{uncore_imc_free_running/data_read/,uncore_imc_free_running/data_write/}' -a sleep 1 Performance counter stats for 'system wide': 727.42 MiB uncore_imc_free_running/data_read/ 81.84 MiB uncore_imc_free_running/data_write/ 1.000948615 seconds time elapsed ``` As well as making groups not fail for cases like: ``` $ perf stat -e '{imc_free_running_0/data_total/,imc_free_running_1/data_total/}' -a sleep 1 Performance counter stats for 'system wide': 256.47 MiB imc_free_running_0/data_total/ 256.48 MiB imc_free_running_1/data_total/ 1.001165442 seconds time elapsed ``` Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kim Phillips Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Stephane Eranian Cc: Suzuki Poulouse Cc: Xing Zhengjun Link: https://lore.kernel.org/r/20230312021543.3060328-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/arch/x86/util/evlist.c | 39 +++---- tools/perf/util/evlist.h | 2 +- tools/perf/util/parse-events.c | 240 ++++++++++++++++++-------------------- tools/perf/util/parse-events.h | 3 +- tools/perf/util/parse-events.y | 4 +- 5 files changed, 136 insertions(+), 152 deletions(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/arch/x86/util/evlist.c b/tools/perf/arch/x86/util/evlist.c index 8a7ae4162563..d4193479a364 100644 --- a/tools/perf/arch/x86/util/evlist.c +++ b/tools/perf/arch/x86/util/evlist.c @@ -65,29 +65,22 @@ int arch_evlist__add_default_attrs(struct evlist *evlist, return ___evlist__add_default_attrs(evlist, attrs, nr_attrs); } -struct evsel *arch_evlist__leader(struct list_head *list) +int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs) { - struct evsel *evsel, *first, *slots = NULL; - bool has_topdown = false; - - first = list_first_entry(list, struct evsel, core.node); - - if (!topdown_sys_has_perf_metrics()) - return first; - - /* If there is a slots event and a topdown event then the slots event comes first. */ - __evlist__for_each_entry(list, evsel) { - if (evsel->pmu_name && !strncmp(evsel->pmu_name, "cpu", 3) && evsel->name) { - if (strcasestr(evsel->name, "slots")) { - slots = evsel; - if (slots == first) - return first; - } - if (strcasestr(evsel->name, "topdown")) - has_topdown = true; - if (slots && has_topdown) - return slots; - } + if (topdown_sys_has_perf_metrics() && + (!lhs->pmu_name || !strncmp(lhs->pmu_name, "cpu", 3))) { + /* Ensure the topdown slots comes first. */ + if (strcasestr(lhs->name, "slots")) + return -1; + if (strcasestr(rhs->name, "slots")) + return 1; + /* Followed by topdown events. */ + if (strcasestr(lhs->name, "topdown") && !strcasestr(rhs->name, "topdown")) + return -1; + if (!strcasestr(lhs->name, "topdown") && strcasestr(rhs->name, "topdown")) + return 1; } - return first; + + /* Default ordering by insertion index. */ + return lhs->core.idx - rhs->core.idx; } diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 01fa9d592c5a..d89d8f92802b 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -119,7 +119,7 @@ int arch_evlist__add_default_attrs(struct evlist *evlist, #define evlist__add_default_attrs(evlist, array) \ arch_evlist__add_default_attrs(evlist, array, ARRAY_SIZE(array)) -struct evsel *arch_evlist__leader(struct list_head *list); +int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs); int evlist__add_dummy(struct evlist *evlist); struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide); diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 1be454697d57..394ab23089d0 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include #include +#include #include #include #include @@ -1655,125 +1656,7 @@ int parse_events__modifier_group(struct list_head *list, return parse_events__modifier_event(list, event_mod, true); } -/* - * Check if the two uncore PMUs are from the same uncore block - * The format of the uncore PMU name is uncore_#blockname_#pmuidx - */ -static bool is_same_uncore_block(const char *pmu_name_a, const char *pmu_name_b) -{ - char *end_a, *end_b; - - end_a = strrchr(pmu_name_a, '_'); - end_b = strrchr(pmu_name_b, '_'); - - if (!end_a || !end_b) - return false; - - if ((end_a - pmu_name_a) != (end_b - pmu_name_b)) - return false; - - return (strncmp(pmu_name_a, pmu_name_b, end_a - pmu_name_a) == 0); -} - -static int -parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list, - struct parse_events_state *parse_state) -{ - struct evsel *evsel, *leader; - uintptr_t *leaders; - bool is_leader = true; - int i, nr_pmu = 0, total_members, ret = 0; - - leader = list_first_entry(list, struct evsel, core.node); - evsel = list_last_entry(list, struct evsel, core.node); - total_members = evsel->core.idx - leader->core.idx + 1; - - leaders = calloc(total_members, sizeof(uintptr_t)); - if (WARN_ON(!leaders)) - return 0; - - /* - * Going through the whole group and doing sanity check. - * All members must use alias, and be from the same uncore block. - * Also, storing the leader events in an array. - */ - __evlist__for_each_entry(list, evsel) { - - /* Only split the uncore group which members use alias */ - if (!evsel->use_uncore_alias) - goto out; - - /* The events must be from the same uncore block */ - if (!is_same_uncore_block(leader->pmu_name, evsel->pmu_name)) - goto out; - - if (!is_leader) - continue; - /* - * If the event's PMU name starts to repeat, it must be a new - * event. That can be used to distinguish the leader from - * other members, even they have the same event name. - */ - if ((leader != evsel) && - !strcmp(leader->pmu_name, evsel->pmu_name)) { - is_leader = false; - continue; - } - - /* Store the leader event for each PMU */ - leaders[nr_pmu++] = (uintptr_t) evsel; - } - - /* only one event alias */ - if (nr_pmu == total_members) { - parse_state->nr_groups--; - goto handled; - } - - /* - * An uncore event alias is a joint name which means the same event - * runs on all PMUs of a block. - * Perf doesn't support mixed events from different PMUs in the same - * group. The big group has to be split into multiple small groups - * which only include the events from the same PMU. - * - * Here the uncore event aliases must be from the same uncore block. - * The number of PMUs must be same for each alias. The number of new - * small groups equals to the number of PMUs. - * Setting the leader event for corresponding members in each group. - */ - i = 0; - __evlist__for_each_entry(list, evsel) { - if (i >= nr_pmu) - i = 0; - evsel__set_leader(evsel, (struct evsel *) leaders[i++]); - } - - /* The number of members and group name are same for each group */ - for (i = 0; i < nr_pmu; i++) { - evsel = (struct evsel *) leaders[i]; - evsel->core.nr_members = total_members / nr_pmu; - evsel->group_name = name ? strdup(name) : NULL; - } - - /* Take the new small groups into account */ - parse_state->nr_groups += nr_pmu - 1; - -handled: - ret = 1; - free(name); -out: - free(leaders); - return ret; -} - -__weak struct evsel *arch_evlist__leader(struct list_head *list) -{ - return list_first_entry(list, struct evsel, core.node); -} - -void parse_events__set_leader(char *name, struct list_head *list, - struct parse_events_state *parse_state) +void parse_events__set_leader(char *name, struct list_head *list) { struct evsel *leader; @@ -1782,13 +1665,9 @@ void parse_events__set_leader(char *name, struct list_head *list, return; } - if (parse_events__set_leader_for_uncore_aliase(name, list, parse_state)) - return; - - leader = arch_evlist__leader(list); + leader = list_first_entry(list, struct evsel, core.node); __perf_evlist__set_leader(list, &leader->core); leader->group_name = name; - list_move(&leader->core.node, list); } /* list_event is assumed to point to malloc'ed memory */ @@ -2245,6 +2124,117 @@ static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state, return ret; } +__weak int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs) +{ + /* Order by insertion index. */ + return lhs->core.idx - rhs->core.idx; +} + +static int evlist__cmp(void *state, const struct list_head *l, const struct list_head *r) +{ + const struct perf_evsel *lhs_core = container_of(l, struct perf_evsel, node); + const struct evsel *lhs = container_of(lhs_core, struct evsel, core); + const struct perf_evsel *rhs_core = container_of(r, struct perf_evsel, node); + const struct evsel *rhs = container_of(rhs_core, struct evsel, core); + int *leader_idx = state; + int lhs_leader_idx = *leader_idx, rhs_leader_idx = *leader_idx, ret; + const char *lhs_pmu_name, *rhs_pmu_name; + + /* + * First sort by grouping/leader. Read the leader idx only if the evsel + * is part of a group, as -1 indicates no group. + */ + if (lhs_core->leader != lhs_core || lhs_core->nr_members > 1) + lhs_leader_idx = lhs_core->leader->idx; + if (rhs_core->leader != rhs_core || rhs_core->nr_members > 1) + rhs_leader_idx = rhs_core->leader->idx; + + if (lhs_leader_idx != rhs_leader_idx) + return lhs_leader_idx - rhs_leader_idx; + + /* Group by PMU. Groups can't span PMUs. */ + lhs_pmu_name = evsel__group_pmu_name(lhs); + rhs_pmu_name = evsel__group_pmu_name(rhs); + ret = strcmp(lhs_pmu_name, rhs_pmu_name); + if (ret) + return ret; + + /* Architecture specific sorting. */ + return arch_evlist__cmp(lhs, rhs); +} + +static void parse_events__sort_events_and_fix_groups(struct list_head *list) +{ + int idx = -1; + struct evsel *pos, *cur_leader = NULL; + struct perf_evsel *cur_leaders_grp = NULL; + + /* + * Compute index to insert ungrouped events at. Place them where the + * first ungrouped event appears. + */ + list_for_each_entry(pos, list, core.node) { + const struct evsel *pos_leader = evsel__leader(pos); + + if (pos != pos_leader || pos->core.nr_members > 1) + continue; + + idx = pos->core.idx; + break; + } + + /* Sort events. */ + list_sort(&idx, list, evlist__cmp); + + /* + * Recompute groups, splitting for PMUs and adding groups for events + * that require them. + */ + idx = 0; + list_for_each_entry(pos, list, core.node) { + const struct evsel *pos_leader = evsel__leader(pos); + const char *pos_pmu_name = evsel__group_pmu_name(pos); + const char *cur_leader_pmu_name, *pos_leader_pmu_name; + bool force_grouped = arch_evsel__must_be_in_group(pos); + + /* Reset index and nr_members. */ + pos->core.idx = idx++; + pos->core.nr_members = 0; + + /* + * Set the group leader respecting the given groupings and that + * groups can't span PMUs. + */ + if (!cur_leader) + cur_leader = pos; + + cur_leader_pmu_name = evsel__group_pmu_name(cur_leader); + if ((cur_leaders_grp != pos->core.leader && !force_grouped) || + strcmp(cur_leader_pmu_name, pos_pmu_name)) { + /* Event is for a different group/PMU than last. */ + cur_leader = pos; + /* + * Remember the leader's group before it is overwritten, + * so that later events match as being in the same + * group. + */ + cur_leaders_grp = pos->core.leader; + } + pos_leader_pmu_name = evsel__group_pmu_name(pos_leader); + if (strcmp(pos_leader_pmu_name, pos_pmu_name) || force_grouped) { + /* + * Event's PMU differs from its leader's. Groups can't + * span PMUs, so update leader from the group/PMU + * tracker. + */ + evsel__set_leader(pos, cur_leader); + } + } + list_for_each_entry(pos, list, core.node) { + pos->core.leader->nr_members++; + } +} + int __parse_events(struct evlist *evlist, const char *str, struct parse_events_error *err, struct perf_pmu *fake_pmu) { @@ -2266,6 +2256,8 @@ int __parse_events(struct evlist *evlist, const char *str, return -1; } + parse_events__sort_events_and_fix_groups(&parse_state.list); + /* * Add list to the evlist even with errors to allow callers to clean up. */ diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 428e72eaafcc..22fc11b0bd59 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -200,8 +200,7 @@ int parse_events_copy_term_list(struct list_head *old, enum perf_pmu_event_symbol_type perf_pmu__parse_check(const char *name); -void parse_events__set_leader(char *name, struct list_head *list, - struct parse_events_state *parse_state); +void parse_events__set_leader(char *name, struct list_head *list); void parse_events_update_lists(struct list_head *list_event, struct list_head *list_all); void parse_events_evlist_error(struct parse_events_state *parse_state, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 541b8dde2063..90d12f2bc8be 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -203,7 +203,7 @@ PE_NAME '{' events '}' inc_group_count(list, _parse_state); /* Takes ownership of $1. */ - parse_events__set_leader($1, list, _parse_state); + parse_events__set_leader($1, list); $$ = list; } | @@ -212,7 +212,7 @@ PE_NAME '{' events '}' struct list_head *list = $2; inc_group_count(list, _parse_state); - parse_events__set_leader(NULL, list, _parse_state); + parse_events__set_leader(NULL, list); $$ = list; } -- cgit v1.2.3-59-g8ed1b From e733f87e8c77751a00a538627eab1ac05e77cf0d Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Sat, 11 Mar 2023 18:15:41 -0800 Subject: perf evsel: Remove use_uncore_alias This flag used to be used when regrouping uncore events in particular due to wildcard matches. This is now handled by sorting evlist and so the flag is redundant. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kim Phillips Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Stephane Eranian Cc: Suzuki Poulouse Cc: Xing Zhengjun Link: https://lore.kernel.org/r/20230312021543.3060328-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evsel.c | 1 - tools/perf/util/evsel.h | 1 - tools/perf/util/parse-events.c | 12 +++--------- tools/perf/util/parse-events.h | 3 +-- tools/perf/util/parse-events.y | 11 +++++++---- 5 files changed, 11 insertions(+), 17 deletions(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 3dda8a25cf50..a83d8cd5eb51 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -458,7 +458,6 @@ struct evsel *evsel__clone(struct evsel *orig) evsel->per_pkg = orig->per_pkg; evsel->percore = orig->percore; evsel->precise_max = orig->precise_max; - evsel->use_uncore_alias = orig->use_uncore_alias; evsel->is_libpfm_event = orig->is_libpfm_event; evsel->exclude_GH = orig->exclude_GH; diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h index d26745ca6147..c272c06565c0 100644 --- a/tools/perf/util/evsel.h +++ b/tools/perf/util/evsel.h @@ -89,7 +89,6 @@ struct evsel { bool per_pkg; bool percore; bool precise_max; - bool use_uncore_alias; bool is_libpfm_event; bool auto_merge_stats; bool collect_stat; diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 394ab23089d0..93a90651266f 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1445,15 +1445,13 @@ static int parse_events__inside_hybrid_pmu(struct parse_events_state *parse_stat int parse_events_add_pmu(struct parse_events_state *parse_state, struct list_head *list, char *name, struct list_head *head_config, - bool auto_merge_stats, - bool use_alias) + bool auto_merge_stats) { struct perf_event_attr attr; struct perf_pmu_info info; struct perf_pmu *pmu; struct evsel *evsel; struct parse_events_error *err = parse_state->error; - bool use_uncore_alias; LIST_HEAD(config_terms); pmu = parse_state->fake_pmu ?: perf_pmu__find(name); @@ -1488,8 +1486,6 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, memset(&attr, 0, sizeof(attr)); } - use_uncore_alias = (pmu->is_uncore && use_alias); - if (!head_config) { attr.type = pmu->type; evsel = __add_event(list, &parse_state->idx, &attr, @@ -1499,7 +1495,6 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, /*cpu_list=*/NULL); if (evsel) { evsel->pmu_name = name ? strdup(name) : NULL; - evsel->use_uncore_alias = use_uncore_alias; return 0; } else { return -ENOMEM; @@ -1560,7 +1555,6 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, evsel->use_config_name = true; evsel->pmu_name = name ? strdup(name) : NULL; - evsel->use_uncore_alias = use_uncore_alias; evsel->percore = config_term_percore(&evsel->config_terms); if (parse_state->fake_pmu) @@ -1622,7 +1616,7 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, parse_events_copy_term_list(head, &orig_head); if (!parse_events_add_pmu(parse_state, list, pmu->name, orig_head, - true, true)) { + /*auto_merge_stats=*/true)) { pr_debug("%s -> %s/%s/\n", str, pmu->name, alias->str); ok++; @@ -1634,7 +1628,7 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, if (parse_state->fake_pmu) { if (!parse_events_add_pmu(parse_state, list, str, head, - true, true)) { + /*auto_merge_stats=*/true)) { pr_debug("%s -> %s/%s/\n", str, "fake_pmu", str); ok++; } diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 22fc11b0bd59..fdac44dc696b 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -183,8 +183,7 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx, int parse_events_add_pmu(struct parse_events_state *parse_state, struct list_head *list, char *name, struct list_head *head_config, - bool auto_merge_stats, - bool use_alias); + bool auto_merge_stats); struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr, const char *name, const char *metric_id, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 90d12f2bc8be..f1b153c72d67 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -313,7 +313,7 @@ event_pmu_name opt_pmu_config list = alloc_list(); if (!list) CLEANUP_YYABORT; - if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) { + if (parse_events_add_pmu(_parse_state, list, $1, $2, /*auto_merge_stats=*/false)) { struct perf_pmu *pmu = NULL; int ok = 0; @@ -330,8 +330,10 @@ event_pmu_name opt_pmu_config !perf_pmu__match(pattern, pmu->alias_name, $1)) { if (parse_events_copy_term_list(orig_terms, &terms)) CLEANUP_YYABORT; - if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false)) + if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, + /*auto_merge_stats=*/true)) { ok++; + } parse_events_terms__delete(terms); } } @@ -407,7 +409,8 @@ PE_PMU_EVENT_FAKE sep_dc if (!list) YYABORT; - err = parse_events_add_pmu(_parse_state, list, $1, NULL, false, false); + err = parse_events_add_pmu(_parse_state, list, $1, /*head_config=*/NULL, + /*auto_merge_stats=*/false); free($1); if (err < 0) { free(list); @@ -425,7 +428,7 @@ PE_PMU_EVENT_FAKE opt_pmu_config if (!list) YYABORT; - err = parse_events_add_pmu(_parse_state, list, $1, $2, false, false); + err = parse_events_add_pmu(_parse_state, list, $1, $2, /*auto_merge_stats=*/false); free($1); parse_events_terms__delete($2); if (err < 0) { -- cgit v1.2.3-59-g8ed1b From 9d2dc632e09c0fe3a8a5890845bbd65b211fd662 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Sat, 11 Mar 2023 18:15:42 -0800 Subject: perf evlist: Remove nr_groups Maintaining the number of groups during event parsing is problematic and since changing to sort/regroup events can only be computed by a linear pass over the evlist. As the value is generally only used in tests, rather than hold it in a variable compute it by passing over the evlist when necessary. This change highlights that libpfm's counting of groups with a single entry disagreed with regular event parsing. The libpfm tests are updated accordingly. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kim Phillips Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Stephane Eranian Cc: Suzuki Poulouse Cc: Xing Zhengjun Link: https://lore.kernel.org/r/20230312021543.3060328-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/perf/evlist.c | 18 +++++++++++++++++- tools/lib/perf/include/internal/evlist.h | 1 - tools/lib/perf/include/perf/evlist.h | 1 + tools/perf/builtin-record.c | 2 +- tools/perf/builtin-report.c | 2 +- tools/perf/tests/bpf.c | 1 - tools/perf/tests/parse-events.c | 22 +++++++++++----------- tools/perf/tests/pfm.c | 12 ++++++------ tools/perf/util/evlist.c | 2 +- tools/perf/util/evlist.h | 6 ++++++ tools/perf/util/header.c | 3 +-- tools/perf/util/parse-events.c | 1 - tools/perf/util/parse-events.h | 1 - tools/perf/util/parse-events.y | 10 ---------- tools/perf/util/pfm.c | 1 - 15 files changed, 45 insertions(+), 38 deletions(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c index 2d6121e89ccb..81e8b5fcd8ba 100644 --- a/tools/lib/perf/evlist.c +++ b/tools/lib/perf/evlist.c @@ -703,7 +703,23 @@ void perf_evlist__set_leader(struct perf_evlist *evlist) struct perf_evsel *first = list_entry(evlist->entries.next, struct perf_evsel, node); - evlist->nr_groups = evlist->nr_entries > 1 ? 1 : 0; __perf_evlist__set_leader(&evlist->entries, first); } } + +int perf_evlist__nr_groups(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel; + int nr_groups = 0; + + perf_evlist__for_each_evsel(evlist, evsel) { + /* + * evsels by default have a nr_members of 1, and they are their + * own leader. If the nr_members is >1 then this is an + * indication of a group. + */ + if (evsel->leader == evsel && evsel->nr_members > 1) + nr_groups++; + } + return nr_groups; +} diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h index 850f07070036..3339bc2f1765 100644 --- a/tools/lib/perf/include/internal/evlist.h +++ b/tools/lib/perf/include/internal/evlist.h @@ -17,7 +17,6 @@ struct perf_mmap_param; struct perf_evlist { struct list_head entries; int nr_entries; - int nr_groups; bool has_user_cpus; bool needs_map_propagation; /** diff --git a/tools/lib/perf/include/perf/evlist.h b/tools/lib/perf/include/perf/evlist.h index 9ca399d49bb4..e894b770779e 100644 --- a/tools/lib/perf/include/perf/evlist.h +++ b/tools/lib/perf/include/perf/evlist.h @@ -47,4 +47,5 @@ LIBPERF_API struct perf_mmap *perf_evlist__next_mmap(struct perf_evlist *evlist, (pos) = perf_evlist__next_mmap((evlist), (pos), overwrite)) LIBPERF_API void perf_evlist__set_leader(struct perf_evlist *evlist); +LIBPERF_API int perf_evlist__nr_groups(struct perf_evlist *evlist); #endif /* __LIBPERF_EVLIST_H */ diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index fb8aab0cedd0..74388164a571 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -2474,7 +2474,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) rec->tool.ordered_events = false; } - if (!rec->evlist->core.nr_groups) + if (evlist__nr_groups(rec->evlist) == 0) perf_header__clear_feat(&session->header, HEADER_GROUP_DESC); if (data->is_pipe) { diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 2ee2ecca208e..6400615b5e98 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -1481,7 +1481,7 @@ repeat: setup_forced_leader(&report, session->evlist); - if (symbol_conf.group_sort_idx && !session->evlist->core.nr_groups) { + if (symbol_conf.group_sort_idx && evlist__nr_groups(session->evlist) == 0) { parse_options_usage(NULL, options, "group-sort-idx", 0); ret = -EINVAL; goto error; diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c index ae9223f27cfb..8beb46066034 100644 --- a/tools/perf/tests/bpf.c +++ b/tools/perf/tests/bpf.c @@ -153,7 +153,6 @@ static int do_test(struct bpf_object *obj, int (*func)(void), } evlist__splice_list_tail(evlist, &parse_state.list); - evlist->core.nr_groups = parse_state.nr_groups; evlist__config(evlist, &opts, NULL); diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index 71a5cb343311..ffa6f0a90741 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -53,7 +53,7 @@ static int test__checkevent_tracepoint(struct evlist *evlist) struct evsel *evsel = evlist__first(evlist); TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 0 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 0 == evlist__nr_groups(evlist)); TEST_ASSERT_VAL("wrong type", PERF_TYPE_TRACEPOINT == evsel->core.attr.type); TEST_ASSERT_VAL("wrong sample_type", PERF_TP_SAMPLE_TYPE == evsel->core.attr.sample_type); @@ -66,7 +66,7 @@ static int test__checkevent_tracepoint_multi(struct evlist *evlist) struct evsel *evsel; TEST_ASSERT_VAL("wrong number of entries", evlist->core.nr_entries > 1); - TEST_ASSERT_VAL("wrong number of groups", 0 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 0 == evlist__nr_groups(evlist)); evlist__for_each_entry(evlist, evsel) { TEST_ASSERT_VAL("wrong type", @@ -677,7 +677,7 @@ static int test__group1(struct evlist *evlist) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* instructions:k */ evsel = leader = evlist__first(evlist); @@ -719,7 +719,7 @@ static int test__group2(struct evlist *evlist) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 3 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* faults + :ku modifier */ evsel = leader = evlist__first(evlist); @@ -775,7 +775,7 @@ static int test__group3(struct evlist *evlist __maybe_unused) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 5 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 2 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 2 == evlist__nr_groups(evlist)); /* group1 syscalls:sys_enter_openat:H */ evsel = leader = evlist__first(evlist); @@ -868,7 +868,7 @@ static int test__group4(struct evlist *evlist __maybe_unused) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* cycles:u + p */ evsel = leader = evlist__first(evlist); @@ -912,7 +912,7 @@ static int test__group5(struct evlist *evlist __maybe_unused) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 5 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 2 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 2 == evlist__nr_groups(evlist)); /* cycles + G */ evsel = leader = evlist__first(evlist); @@ -998,7 +998,7 @@ static int test__group_gh1(struct evlist *evlist) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* cycles + :H group modifier */ evsel = leader = evlist__first(evlist); @@ -1038,7 +1038,7 @@ static int test__group_gh2(struct evlist *evlist) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* cycles + :G group modifier */ evsel = leader = evlist__first(evlist); @@ -1078,7 +1078,7 @@ static int test__group_gh3(struct evlist *evlist) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* cycles:G + :u group modifier */ evsel = leader = evlist__first(evlist); @@ -1118,7 +1118,7 @@ static int test__group_gh4(struct evlist *evlist) struct evsel *evsel, *leader; TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); - TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups); + TEST_ASSERT_VAL("wrong number of groups", 1 == evlist__nr_groups(evlist)); /* cycles:G + :uG group modifier */ evsel = leader = evlist__first(evlist); diff --git a/tools/perf/tests/pfm.c b/tools/perf/tests/pfm.c index 71b76deb1f92..2e38dfa34b6c 100644 --- a/tools/perf/tests/pfm.c +++ b/tools/perf/tests/pfm.c @@ -76,7 +76,7 @@ static int test__pfm_events(struct test_suite *test __maybe_unused, count_pfm_events(&evlist->core), table[i].nr_events); TEST_ASSERT_EQUAL(table[i].events, - evlist->core.nr_groups, + evlist__nr_groups(evlist), 0); evlist__delete(evlist); @@ -103,22 +103,22 @@ static int test__pfm_group(struct test_suite *test __maybe_unused, { .events = "{instructions}", .nr_events = 1, - .nr_groups = 1, + .nr_groups = 0, }, { .events = "{instructions},{}", .nr_events = 1, - .nr_groups = 1, + .nr_groups = 0, }, { .events = "{},{instructions}", .nr_events = 1, - .nr_groups = 1, + .nr_groups = 0, }, { .events = "{instructions},{instructions}", .nr_events = 2, - .nr_groups = 2, + .nr_groups = 0, }, { .events = "{instructions,cycles},{instructions,cycles}", @@ -161,7 +161,7 @@ static int test__pfm_group(struct test_suite *test __maybe_unused, count_pfm_events(&evlist->core), table[i].nr_events); TEST_ASSERT_EQUAL(table[i].events, - evlist->core.nr_groups, + evlist__nr_groups(evlist), table[i].nr_groups); evlist__delete(evlist); diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 9e4b2bb0e6fa..b74e12239aec 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1777,7 +1777,7 @@ bool evlist__exclude_kernel(struct evlist *evlist) */ void evlist__force_leader(struct evlist *evlist) { - if (!evlist->core.nr_groups) { + if (evlist__nr_groups(evlist) == 0) { struct evsel *leader = evlist__first(evlist); evlist__set_leader(evlist); diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index d89d8f92802b..46cf402add93 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -9,6 +9,7 @@ #include #include #include +#include #include "events_stats.h" #include "evsel.h" #include @@ -255,6 +256,11 @@ static inline struct evsel *evlist__last(struct evlist *evlist) return container_of(evsel, struct evsel, core); } +static inline int evlist__nr_groups(struct evlist *evlist) +{ + return perf_evlist__nr_groups(&evlist->core); +} + int evlist__strerror_open(struct evlist *evlist, int err, char *buf, size_t size); int evlist__strerror_mmap(struct evlist *evlist, int err, char *buf, size_t size); diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 404d816ca124..276870221ce0 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -786,7 +786,7 @@ static int write_pmu_mappings(struct feat_fd *ff, static int write_group_desc(struct feat_fd *ff, struct evlist *evlist) { - u32 nr_groups = evlist->core.nr_groups; + u32 nr_groups = evlist__nr_groups(evlist); struct evsel *evsel; int ret; @@ -2807,7 +2807,6 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused) * Rebuild group relationship based on the group_desc */ session = container_of(ff->ph, struct perf_session, header); - session->evlist->core.nr_groups = nr_groups; i = nr = 0; evlist__for_each_entry(session->evlist, evsel) { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 93a90651266f..9ec3c1dc81e0 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2260,7 +2260,6 @@ int __parse_events(struct evlist *evlist, const char *str, if (!ret) { struct evsel *last; - evlist->core.nr_groups += parse_state.nr_groups; last = evlist__last(evlist); last->cmdline_group_boundary = true; diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index fdac44dc696b..767ad1729228 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -122,7 +122,6 @@ struct parse_events_error { struct parse_events_state { struct list_head list; int idx; - int nr_groups; struct parse_events_error *error; struct evlist *evlist; struct list_head *terms; diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index f1b153c72d67..3a04602d2982 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -49,14 +49,6 @@ static void free_list_evsel(struct list_head* list_evsel) free(list_evsel); } -static void inc_group_count(struct list_head *list, - struct parse_events_state *parse_state) -{ - /* Count groups only have more than 1 members */ - if (!list_is_last(list->next, list)) - parse_state->nr_groups++; -} - %} %token PE_START_EVENTS PE_START_TERMS @@ -201,7 +193,6 @@ PE_NAME '{' events '}' { struct list_head *list = $3; - inc_group_count(list, _parse_state); /* Takes ownership of $1. */ parse_events__set_leader($1, list); $$ = list; @@ -211,7 +202,6 @@ PE_NAME '{' events '}' { struct list_head *list = $2; - inc_group_count(list, _parse_state); parse_events__set_leader(NULL, list); $$ = list; } diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c index b59ba825ddc9..6c11914c179f 100644 --- a/tools/perf/util/pfm.c +++ b/tools/perf/util/pfm.c @@ -112,7 +112,6 @@ int parse_libpfm_events_option(const struct option *opt, const char *str, "cannot close a non-existing event group\n"); goto error; } - evlist->core.nr_groups++; grp_leader = NULL; grp_evt = -1; } -- cgit v1.2.3-59-g8ed1b From a4c7d7c502b935f3a8324d954de78aecf6940897 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Sat, 11 Mar 2023 18:15:43 -0800 Subject: perf parse-events: Warn when events are regrouped Use if an event is reordered or the number of groups increases to signal that regrouping has happened and warn about it. Disable the warning in the case wild card PMU names are used and for metrics. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kim Phillips Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Stephane Eranian Cc: Suzuki Poulouse Cc: Xing Zhengjun Link: https://lore.kernel.org/r/20230312021543.3060328-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 2 +- tools/perf/tests/pmu-events.c | 2 +- tools/perf/util/metricgroup.c | 3 ++- tools/perf/util/parse-events.c | 39 +++++++++++++++++++++++++++++---------- tools/perf/util/parse-events.h | 7 ++++--- tools/perf/util/parse-events.y | 1 + 6 files changed, 38 insertions(+), 16 deletions(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index ffa6f0a90741..b1c2f0a20306 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -2103,7 +2103,7 @@ static int test_event_fake_pmu(const char *str) parse_events_error__init(&err); perf_pmu__test_parse_init(); - ret = __parse_events(evlist, str, &err, &perf_pmu__fake); + ret = __parse_events(evlist, str, &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 6ccd413b5983..7f8e86452527 100644 --- a/tools/perf/tests/pmu-events.c +++ b/tools/perf/tests/pmu-events.c @@ -785,7 +785,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error, */ perf_pmu__test_parse_init(); } - ret = __parse_events(evlist, dup, error, fake_pmu); + ret = __parse_events(evlist, dup, error, fake_pmu, /*warn_if_reordered=*/true); free(dup); evlist__delete(evlist); diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index de6dd527a2ba..5783f4c2d1ef 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -1441,7 +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); + ret = __parse_events(parsed_evlist, events.buf, &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 9ec3c1dc81e0..3b2e5bb3e852 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2157,11 +2157,13 @@ static int evlist__cmp(void *state, const struct list_head *l, const struct list return arch_evlist__cmp(lhs, rhs); } -static void parse_events__sort_events_and_fix_groups(struct list_head *list) +static bool parse_events__sort_events_and_fix_groups(struct list_head *list) { - int idx = -1; + int idx = 0, unsorted_idx = -1; struct evsel *pos, *cur_leader = NULL; struct perf_evsel *cur_leaders_grp = NULL; + bool idx_changed = false; + int orig_num_leaders = 0, num_leaders = 0; /* * Compute index to insert ungrouped events at. Place them where the @@ -2170,15 +2172,22 @@ static void parse_events__sort_events_and_fix_groups(struct list_head *list) list_for_each_entry(pos, list, core.node) { const struct evsel *pos_leader = evsel__leader(pos); - if (pos != pos_leader || pos->core.nr_members > 1) - continue; + if (pos == pos_leader) + orig_num_leaders++; - idx = pos->core.idx; - break; + /* + * Ensure indexes are sequential, in particular for multiple + * event lists being merged. The indexes are used to detect when + * the user order is modified. + */ + pos->core.idx = idx++; + + if (unsorted_idx == -1 && pos == pos_leader && pos->core.nr_members < 2) + unsorted_idx = pos->core.idx; } /* Sort events. */ - list_sort(&idx, list, evlist__cmp); + list_sort(&unsorted_idx, list, evlist__cmp); /* * Recompute groups, splitting for PMUs and adding groups for events @@ -2192,6 +2201,8 @@ static void parse_events__sort_events_and_fix_groups(struct list_head *list) bool force_grouped = arch_evsel__must_be_in_group(pos); /* Reset index and nr_members. */ + if (pos->core.idx != idx) + idx_changed = true; pos->core.idx = idx++; pos->core.nr_members = 0; @@ -2225,12 +2236,18 @@ static void parse_events__sort_events_and_fix_groups(struct list_head *list) } } list_for_each_entry(pos, list, core.node) { - pos->core.leader->nr_members++; + struct evsel *pos_leader = evsel__leader(pos); + + if (pos == pos_leader) + num_leaders++; + pos_leader->core.nr_members++; } + return idx_changed || num_leaders != orig_num_leaders; } int __parse_events(struct evlist *evlist, const char *str, - struct parse_events_error *err, struct perf_pmu *fake_pmu) + struct parse_events_error *err, struct perf_pmu *fake_pmu, + bool warn_if_reordered) { struct parse_events_state parse_state = { .list = LIST_HEAD_INIT(parse_state.list), @@ -2250,7 +2267,9 @@ int __parse_events(struct evlist *evlist, const char *str, return -1; } - parse_events__sort_events_and_fix_groups(&parse_state.list); + if (parse_events__sort_events_and_fix_groups(&parse_state.list) && + warn_if_reordered && !parse_state.wild_card_pmus) + pr_warning("WARNING: events were regrouped to match PMUs\n"); /* * Add list to the evlist even with errors to allow callers to clean up. diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 767ad1729228..46204c1a7916 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -26,13 +26,13 @@ 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); + struct perf_pmu *fake_pmu, bool warn_if_reordered); -__attribute__((nonnull)) +__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, NULL); + return __parse_events(evlist, str, err, /*fake_pmu=*/NULL, /*warn_if_reordered=*/true); } int parse_event(struct evlist *evlist, const char *str); @@ -128,6 +128,7 @@ struct parse_events_state { int stoken; struct perf_pmu *fake_pmu; char *hybrid_pmu_name; + bool wild_card_pmus; }; void parse_events__shrink_config_terms(void); diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 3a04602d2982..4488443e506e 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -323,6 +323,7 @@ event_pmu_name opt_pmu_config if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, /*auto_merge_stats=*/true)) { ok++; + parse_state->wild_card_pmus = true; } parse_events_terms__delete(terms); } -- cgit v1.2.3-59-g8ed1b From d180aa56b50dd243dec89b24d23e0a59c3f0c0eb Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 14 Mar 2023 16:42:30 -0700 Subject: perf record: Add BPF event filter support Use --filter option to set BPF filter for generic events other than the tracepoints or Intel PT. The BPF program will check the sample data and filter according to the expression. For example, the below is the typical perf record for frequency mode. The sample period started from 1 and increased gradually. $ sudo ./perf record -e cycles true $ sudo ./perf script perf-exec 2272336 546683.916875: 1 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916892: 1 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916899: 3 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916905: 17 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916911: 100 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916917: 589 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916924: 3470 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) perf-exec 2272336 546683.916930: 20465 cycles: ffffffff828499b8 perf_event_exec+0x298 ([kernel.kallsyms]) true 2272336 546683.916940: 119873 cycles: ffffffff8283afdd perf_iterate_ctx+0x2d ([kernel.kallsyms]) true 2272336 546683.917003: 461349 cycles: ffffffff82892517 vma_interval_tree_insert+0x37 ([kernel.kallsyms]) true 2272336 546683.917237: 635778 cycles: ffffffff82a11400 security_mmap_file+0x20 ([kernel.kallsyms]) When you add a BPF filter to get samples having periods greater than 1000, the output would look like below: $ sudo ./perf record -e cycles --filter 'period > 1000' true $ sudo ./perf script perf-exec 2273949 546850.708501: 5029 cycles: ffffffff826f9e25 finish_wait+0x5 ([kernel.kallsyms]) perf-exec 2273949 546850.708508: 32409 cycles: ffffffff826f9e25 finish_wait+0x5 ([kernel.kallsyms]) perf-exec 2273949 546850.708526: 143369 cycles: ffffffff82b4cdbf xas_start+0x5f ([kernel.kallsyms]) perf-exec 2273949 546850.708600: 372650 cycles: ffffffff8286b8f7 __pagevec_lru_add+0x117 ([kernel.kallsyms]) perf-exec 2273949 546850.708791: 482953 cycles: ffffffff829190de __mod_memcg_lruvec_state+0x4e ([kernel.kallsyms]) true 2273949 546850.709036: 501985 cycles: ffffffff828add7c tlb_gather_mmu+0x4c ([kernel.kallsyms]) true 2273949 546850.709292: 503065 cycles: 7f2446d97c03 _dl_map_object_deps+0x973 (/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2) Committer notes: Add stubs for perf_bpf_filter__prepare() and perf_bpf_filter__destroy() to tools/perf/util/python.c to keep it building. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Adrian Hunter Cc: Andi Kleen Cc: Hao Luo Cc: Ian Rogers Cc: Ingo Molnar Cc: James Clark Cc: Kan Liang Cc: Leo Yan Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Song Liu Cc: Stephane Eranian Cc: bpf@vger.kernel.org Link: https://lore.kernel.org/r/20230314234237.3008956-2-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-record.txt | 15 ++++++++++++--- tools/perf/util/bpf_counter.c | 3 +-- tools/perf/util/evlist.c | 25 ++++++++++++++++++------- tools/perf/util/evsel.c | 2 ++ tools/perf/util/parse-events.c | 8 +++----- tools/perf/util/python.c | 14 ++++++++++++++ 6 files changed, 50 insertions(+), 17 deletions(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index ff815c2f67e8..122f71726eaa 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -119,9 +119,12 @@ OPTIONS "perf report" to view group events together. --filter=:: - Event filter. This option should follow an event selector (-e) which - selects either tracepoint event(s) or a hardware trace PMU - (e.g. Intel PT or CoreSight). + Event filter. This option should follow an event selector (-e). + If the event is a tracepoint, the filter string will be parsed by + the kernel. If the event is a hardware trace PMU (e.g. Intel PT + or CoreSight), it'll be processed as an address filter. Otherwise + it means a general filter using BPF which can be applied for any + kind of event. - tracepoint filters @@ -176,6 +179,12 @@ OPTIONS Multiple filters can be separated with space or comma. + - bpf filters + + A BPF filter can access the sample data and make a decision based on the + data. Users need to set an appropriate sample type to use the BPF + filter. + --exclude-perf:: Don't record events issued by perf itself. This option should follow an event selector (-e) which selects tracepoint event(s). It adds a diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c index aa78a15a6f0a..1b77436e067e 100644 --- a/tools/perf/util/bpf_counter.c +++ b/tools/perf/util/bpf_counter.c @@ -763,8 +763,7 @@ extern struct bpf_counter_ops bperf_cgrp_ops; static inline bool bpf_counter_skip(struct evsel *evsel) { - return list_empty(&evsel->bpf_counter_list) && - evsel->follower_skel == NULL; + return evsel->bpf_counter_ops == NULL; } int bpf_counter__install_pe(struct evsel *evsel, int cpu_map_idx, int fd) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index b74e12239aec..cc491a037836 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -31,6 +31,7 @@ #include "util/evlist-hybrid.h" #include "util/pmu.h" #include "util/sample.h" +#include "util/bpf-filter.h" #include #include #include @@ -1086,17 +1087,27 @@ int evlist__apply_filters(struct evlist *evlist, struct evsel **err_evsel) int err = 0; evlist__for_each_entry(evlist, evsel) { - if (evsel->filter == NULL) - continue; - /* * filters only work for tracepoint event, which doesn't have cpu limit. * So evlist and evsel should always be same. */ - err = perf_evsel__apply_filter(&evsel->core, evsel->filter); - if (err) { - *err_evsel = evsel; - break; + if (evsel->filter) { + err = perf_evsel__apply_filter(&evsel->core, evsel->filter); + if (err) { + *err_evsel = evsel; + break; + } + } + + /* + * non-tracepoint events can have BPF filters. + */ + if (!list_empty(&evsel->bpf_filters)) { + err = perf_bpf_filter__prepare(evsel); + if (err) { + *err_evsel = evsel; + break; + } } } diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index a83d8cd5eb51..dc3faf005c3b 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -50,6 +50,7 @@ #include "off_cpu.h" #include "../perf-sys.h" #include "util/parse-branch-options.h" +#include "util/bpf-filter.h" #include #include #include @@ -1517,6 +1518,7 @@ void evsel__exit(struct evsel *evsel) assert(list_empty(&evsel->core.node)); assert(evsel->evlist == NULL); bpf_counter__destroy(evsel); + perf_bpf_filter__destroy(evsel); evsel__free_counts(evsel); perf_evsel__free_fd(&evsel->core); perf_evsel__free_id(&evsel->core); diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 3b2e5bb3e852..6c5cf5244486 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -28,6 +28,7 @@ #include "perf.h" #include "util/parse-events-hybrid.h" #include "util/pmu-hybrid.h" +#include "util/bpf-filter.h" #include "tracepoint.h" #include "thread_map.h" @@ -2542,11 +2543,8 @@ static int set_filter(struct evsel *evsel, const void *arg) perf_pmu__scan_file(pmu, "nr_addr_filters", "%d", &nr_addr_filters); - if (!nr_addr_filters) { - fprintf(stderr, - "This CPU does not support address filtering\n"); - return -1; - } + if (!nr_addr_filters) + return perf_bpf_filter__parse(&evsel->bpf_filters, str); if (evsel__append_addr_filter(evsel, str) < 0) { fprintf(stderr, diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c index be336f1b2b68..0faea4c75eed 100644 --- a/tools/perf/util/python.c +++ b/tools/perf/util/python.c @@ -19,6 +19,7 @@ #include "mmap.h" #include "stat.h" #include "metricgroup.h" +#include "util/bpf-filter.h" #include "util/env.h" #include "util/pmu.h" #include @@ -135,6 +136,19 @@ int bpf_counter__disable(struct evsel *evsel __maybe_unused) return 0; } +// not to drag util/bpf-filter.c +#ifdef HAVE_BPF_SKEL +int perf_bpf_filter__prepare(struct evsel *evsel __maybe_unused) +{ + return 0; +} + +int perf_bpf_filter__destroy(struct evsel *evsel __maybe_unused) +{ + return 0; +} +#endif + /* * Support debug printing even though util/debug.c is not linked. That means * implementing 'verbose' and 'eprintf'. -- cgit v1.2.3-59-g8ed1b From 204e7c499f5fa7732144ef9765c195982be72f31 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 17 Feb 2023 16:32:11 -0600 Subject: perf tools: Add support for perf_event_attr::config3 perf_event_attr has gained a new field, config3, so add support for it extending the existing configN support. Signed-off-by: Rob Herring Cc: Alexander Shishkin Cc: James Clark Cc: Jiri Olsa Cc: Leo Yan Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lore.kernel.org/lkml/20220914-arm-perf-tool-spe1-2-v2-v5-2-2cf5210b2f77@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 13 ++++++++++++- tools/perf/util/parse-events.c | 6 ++++++ tools/perf/util/parse-events.h | 1 + tools/perf/util/parse-events.l | 1 + tools/perf/util/pmu.c | 3 +++ tools/perf/util/pmu.h | 1 + 6 files changed, 24 insertions(+), 1 deletion(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index b1c2f0a20306..6eb1400443ad 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -449,6 +449,7 @@ static int test__checkevent_pmu(struct evlist *evlist) TEST_ASSERT_VAL("wrong config", 10 == evsel->core.attr.config); TEST_ASSERT_VAL("wrong config1", 1 == evsel->core.attr.config1); TEST_ASSERT_VAL("wrong config2", 3 == evsel->core.attr.config2); + TEST_ASSERT_VAL("wrong config3", 0 == evsel->core.attr.config3); /* * The period value gets configured within evlist__config, * while this test executes only parse events method. @@ -470,6 +471,7 @@ static int test__checkevent_list(struct evlist *evlist) TEST_ASSERT_VAL("wrong config", 1 == evsel->core.attr.config); TEST_ASSERT_VAL("wrong config1", 0 == evsel->core.attr.config1); TEST_ASSERT_VAL("wrong config2", 0 == evsel->core.attr.config2); + TEST_ASSERT_VAL("wrong config3", 0 == evsel->core.attr.config3); 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); @@ -632,6 +634,15 @@ static int test__checkterms_simple(struct list_head *terms) TEST_ASSERT_VAL("wrong val", term->val.num == 3); TEST_ASSERT_VAL("wrong config", !strcmp(term->config, "config2")); + /* config3=4 */ + term = list_entry(term->list.next, struct parse_events_term, list); + TEST_ASSERT_VAL("wrong type term", + term->type_term == PARSE_EVENTS__TERM_TYPE_CONFIG3); + TEST_ASSERT_VAL("wrong type val", + term->type_val == PARSE_EVENTS__TERM_TYPE_NUM); + TEST_ASSERT_VAL("wrong val", term->val.num == 4); + TEST_ASSERT_VAL("wrong config", !strcmp(term->config, "config3")); + /* umask=1*/ term = list_entry(term->list.next, struct parse_events_term, list); TEST_ASSERT_VAL("wrong type term", @@ -2004,7 +2015,7 @@ struct terms_test { static const struct terms_test test__terms[] = { [0] = { - .str = "config=10,config1,config2=3,umask=1,read,r0xead", + .str = "config=10,config1,config2=3,config3=4,umask=1,read,r0xead", .check = test__checkterms_simple, }, }; diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 6c5cf5244486..cc8e8766ca30 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -949,6 +949,7 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = { [PARSE_EVENTS__TERM_TYPE_CONFIG] = "config", [PARSE_EVENTS__TERM_TYPE_CONFIG1] = "config1", [PARSE_EVENTS__TERM_TYPE_CONFIG2] = "config2", + [PARSE_EVENTS__TERM_TYPE_CONFIG3] = "config3", [PARSE_EVENTS__TERM_TYPE_NAME] = "name", [PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD] = "period", [PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ] = "freq", @@ -988,6 +989,7 @@ config_term_avail(int term_type, struct parse_events_error *err) case PARSE_EVENTS__TERM_TYPE_CONFIG: case PARSE_EVENTS__TERM_TYPE_CONFIG1: case PARSE_EVENTS__TERM_TYPE_CONFIG2: + case PARSE_EVENTS__TERM_TYPE_CONFIG3: case PARSE_EVENTS__TERM_TYPE_NAME: case PARSE_EVENTS__TERM_TYPE_METRIC_ID: case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD: @@ -1033,6 +1035,10 @@ do { \ CHECK_TYPE_VAL(NUM); attr->config2 = term->val.num; break; + case PARSE_EVENTS__TERM_TYPE_CONFIG3: + CHECK_TYPE_VAL(NUM); + attr->config3 = term->val.num; + break; case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD: CHECK_TYPE_VAL(NUM); break; diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 46204c1a7916..86ad4438a2aa 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -59,6 +59,7 @@ enum { PARSE_EVENTS__TERM_TYPE_CONFIG, PARSE_EVENTS__TERM_TYPE_CONFIG1, PARSE_EVENTS__TERM_TYPE_CONFIG2, + PARSE_EVENTS__TERM_TYPE_CONFIG3, PARSE_EVENTS__TERM_TYPE_NAME, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD, PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ, diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 3a9ce96c8bce..51fe0a9fb3de 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -285,6 +285,7 @@ modifier_bp [rwx]{1,3} config { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG); } config1 { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG1); } config2 { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG2); } +config3 { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG3); } name { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NAME); } period { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD); } freq { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ); } diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 45d9b8e28e16..e3aae731bd6f 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1209,6 +1209,9 @@ static int pmu_config_term(const char *pmu_name, case PERF_PMU_FORMAT_VALUE_CONFIG2: vp = &attr->config2; break; + case PERF_PMU_FORMAT_VALUE_CONFIG3: + vp = &attr->config3; + break; default: return -EINVAL; } diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h index 6b770f17eb86..24cf69ab32cd 100644 --- a/tools/perf/util/pmu.h +++ b/tools/perf/util/pmu.h @@ -19,6 +19,7 @@ enum { PERF_PMU_FORMAT_VALUE_CONFIG, PERF_PMU_FORMAT_VALUE_CONFIG1, PERF_PMU_FORMAT_VALUE_CONFIG2, + PERF_PMU_FORMAT_VALUE_CONFIG3, PERF_PMU_FORMAT_VALUE_CONFIG_END, }; -- cgit v1.2.3-59-g8ed1b From 66c9598bd8916c6c1319d0100ac916cac89d4b0e Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Fri, 31 Mar 2023 13:29:42 -0700 Subject: perf tools: Fix a asan issue in parse_events_multi_pmu_add() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the parse_events_multi_pmu_add() it passes the 'config' variable twice to parse_events_term__num() - one for config and another for loc_term. I'm not sure about the second one as it's converted to YYLTYPE variable. Asan reports it like below: In function ‘parse_events_term__num’, inlined from ‘parse_events_multi_pmu_add’ at util/parse-events.c:1602:6: util/parse-events.c:2653:64: error: array subscript ‘YYLTYPE[0]’ is partly outside array bounds of ‘char[8]’ [-Werror=array-bounds] 2653 | .err_term = loc_term ? loc_term->first_column : 0, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ util/parse-events.c: In function ‘parse_events_multi_pmu_add’: util/parse-events.c:1587:15: note: object ‘config’ of size 8 1587 | char *config; | ^~~~~~ cc1: all warnings being treated as errors Signed-off-by: Namhyung Kim Acked-by: Ian Rogers Cc: Adrian Hunter Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Leo Yan Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20230331202949.810326-2-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index cc8e8766ca30..0010e5e0ee68 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1601,7 +1601,7 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state, if (parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER, - config, 1, false, &config, + config, 1, false, NULL, NULL) < 0) { free(config); goto out_err; -- cgit v1.2.3-59-g8ed1b From ea0c52399d99d2e48cafdf97e2d8589bf8beb7c2 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Mon, 10 Apr 2023 09:25:11 -0700 Subject: perf util: Move perf_guest/host declarations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The definitions are in util.c so move the declarations to match. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Athira Rajeev Cc: Chengdong Li Cc: Denis Nikitin Cc: Florian Fischer Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kan Liang Cc: Leo Yan Cc: Mark Rutland Cc: Martin Liška Cc: Mathieu Poirier Cc: Mike Leach Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Raul Silvera Cc: Ravi Bangoria Cc: Rob Herring Cc: Sean Christopherson Cc: Suzuki Poulouse Cc: Will Deacon Cc: Xing Zhengjun Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Link: https://lore.kernel.org/r/20230410162511.3055900-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 2 +- tools/perf/builtin-kvm.c | 1 + tools/perf/perf.h | 4 ---- tools/perf/ui/hist.c | 2 +- tools/perf/util/cs-etm.c | 1 + tools/perf/util/event.c | 2 +- tools/perf/util/evlist.c | 1 + tools/perf/util/parse-events.c | 2 +- tools/perf/util/session.c | 2 +- tools/perf/util/top.c | 2 +- tools/perf/util/util.h | 3 +++ 11 files changed, 12 insertions(+), 10 deletions(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 22b526766e14..dbb0562d6a4f 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -6,7 +6,6 @@ * DSOs and symbol information, sort them and produce a diff. */ #include "builtin.h" -#include "perf.h" #include "util/debug.h" #include "util/event.h" @@ -26,6 +25,7 @@ #include "util/spark.h" #include "util/block-info.h" #include "util/stream.h" +#include "util/util.h" #include #include #include diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c index fb9dc0dc46f9..747d19336340 100644 --- a/tools/perf/builtin-kvm.c +++ b/tools/perf/builtin-kvm.c @@ -23,6 +23,7 @@ #include "util/data.h" #include "util/ordered-events.h" #include "util/kvm-stat.h" +#include "util/util.h" #include "ui/browsers/hists.h" #include "ui/progress.h" #include "ui/ui.h" diff --git a/tools/perf/perf.h b/tools/perf/perf.h index 49e15e2be49e..c004dd4e65a3 100644 --- a/tools/perf/perf.h +++ b/tools/perf/perf.h @@ -2,14 +2,10 @@ #ifndef _PERF_PERF_H #define _PERF_PERF_H -#include - #ifndef MAX_NR_CPUS #define MAX_NR_CPUS 2048 #endif -extern bool perf_host, perf_guest; - enum perf_affinity { PERF_AFFINITY_SYS = 0, PERF_AFFINITY_NODE, diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c index 5075ecead5f3..f164bd26fc41 100644 --- a/tools/perf/ui/hist.c +++ b/tools/perf/ui/hist.c @@ -11,7 +11,7 @@ #include "../util/sort.h" #include "../util/evsel.h" #include "../util/evlist.h" -#include "../perf.h" +#include "../util/util.h" /* hist period print (hpp) functions */ diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 944835e16430..103865968700 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -38,6 +38,7 @@ #include "tsc.h" #include #include "util/synthetic-events.h" +#include "util/util.h" struct cs_etm_auxtrace { struct auxtrace auxtrace; diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index 13f7f85e92e1..8ae742e32e3c 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -33,7 +33,7 @@ #include "bpf-event.h" #include "print_binary.h" #include "tool.h" -#include "../perf.h" +#include "util.h" static const char *perf_event__names[] = { [0] = "TOTAL", diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index cc491a037836..df6af38ca22e 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -32,6 +32,7 @@ #include "util/pmu.h" #include "util/sample.h" #include "util/bpf-filter.h" +#include "util/util.h" #include #include #include diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 0010e5e0ee68..f341995cb04e 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -25,10 +25,10 @@ #include "util/parse-branch-options.h" #include "util/evsel_config.h" #include "util/event.h" -#include "perf.h" #include "util/parse-events-hybrid.h" #include "util/pmu-hybrid.h" #include "util/bpf-filter.h" +#include "util/util.h" #include "tracepoint.h" #include "thread_map.h" diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 7d8d057d1772..e2806791c76a 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -33,7 +33,7 @@ #include "stat.h" #include "tsc.h" #include "ui/progress.h" -#include "../perf.h" +#include "util.h" #include "arch/common.h" #include "units.h" #include diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c index b8b32431d2f7..be7157de0451 100644 --- a/tools/perf/util/top.c +++ b/tools/perf/util/top.c @@ -11,7 +11,7 @@ #include "parse-events.h" #include "symbol.h" #include "top.h" -#include "../perf.h" +#include "util.h" #include #define SNPRINTF(buf, size, fmt, args...) \ diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h index 8bd515b67739..7c8915d92dca 100644 --- a/tools/perf/util/util.h +++ b/tools/perf/util/util.h @@ -20,6 +20,9 @@ extern const char perf_more_info_string[]; extern const char *input_name; +extern bool perf_host; +extern bool perf_guest; + /* General helper functions */ void usage(const char *err) __noreturn; void die(const char *err, ...) __noreturn __printf(1, 2); -- cgit v1.2.3-59-g8ed1b From 25feb605fe3b2674142d084379e0737a99945aba Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 12 Apr 2023 09:50:08 -0300 Subject: perf parse-events: Use zfree() to reduce chances of use after free Do defensive programming by using zfree() to initialize freed pointers to NULL, so that eventual use after free result in a NULL pointer deref instead of more subtle behaviour. Also remove one NULL test before free(), as it accepts a NULL arg and we get one line shaved not doing it explicitely. Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/util/parse-events.c') diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index f341995cb04e..d71019dcd614 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2020,7 +2020,7 @@ int perf_pmu__test_parse_init(void) err_free: for (j = 0, tmp = list; j < i; j++, tmp++) - free(tmp->symbol); + zfree(&tmp->symbol); free(list); return -ENOMEM; } -- cgit v1.2.3-59-g8ed1b