[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250310195518.2012571-3-irogers@google.com>
Date: Mon, 10 Mar 2025 12:55:18 -0700
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
James Clark <james.clark@...aro.org>, Dominique Martinet <asmadeus@...ewreck.org>,
Yang Jihong <yangjihong@...edance.com>, Yang Li <yang.lee@...ux.alibaba.com>,
Weilin Wang <weilin.wang@...el.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
Leo Yan <leo.yan@....com>
Subject: [PATCH v1 2/2] Revert "perf evsel: Add alternate_hw_config and use in evsel__match"
This reverts commit 22a4db3c36034e2b034c5b88414680857fc59cf4.
Now legacy events are prioritized there is no need to worry about a
sysfs/json event being used when trying to pattern match a legacy
event.
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/builtin-diff.c | 6 ++--
tools/perf/util/evsel.c | 21 -------------
tools/perf/util/evsel.h | 19 ++++++++++--
tools/perf/util/parse-events.c | 55 +++++++++++++---------------------
tools/perf/util/parse-events.h | 8 +----
tools/perf/util/parse-events.y | 2 +-
tools/perf/util/pmu.c | 6 +---
tools/perf/util/pmu.h | 2 +-
8 files changed, 45 insertions(+), 74 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index ae490d58af92..b49c1dc1fd96 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -470,13 +470,13 @@ static int diff__process_sample_event(const struct perf_tool *tool,
static struct perf_diff pdiff;
-static struct evsel *evsel_match(struct evsel *evsel, struct evlist *evlist)
+static struct evsel *evsel_match(struct evsel *evsel,
+ struct evlist *evlist)
{
struct evsel *e;
evlist__for_each_entry(evlist, e) {
- if ((evsel->core.attr.type == e->core.attr.type) &&
- (evsel->core.attr.config == e->core.attr.config))
+ if (evsel__match2(evsel, e))
return e;
}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4a0ef095db92..5566232f9bb9 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -404,7 +404,6 @@ void evsel__init(struct evsel *evsel,
evsel->collect_stat = false;
evsel->group_pmu_name = NULL;
evsel->skippable = false;
- evsel->alternate_hw_config = PERF_COUNT_HW_MAX;
evsel->script_output_type = -1; // FIXME: OUTPUT_TYPE_UNSET, see builtin-script.c
}
@@ -551,8 +550,6 @@ struct evsel *evsel__clone(struct evsel *dest, struct evsel *orig)
if (evsel__copy_config_terms(evsel, orig) < 0)
goto out_err;
- evsel->alternate_hw_config = orig->alternate_hw_config;
-
return evsel;
out_err:
@@ -1848,24 +1845,6 @@ static int evsel__read_group(struct evsel *leader, int cpu_map_idx, int thread)
return evsel__process_group_data(leader, cpu_map_idx, thread, data);
}
-bool __evsel__match(const struct evsel *evsel, u32 type, u64 config)
-{
-
- u32 e_type = evsel->core.attr.type;
- u64 e_config = evsel->core.attr.config;
-
- if (e_type != type) {
- return type == PERF_TYPE_HARDWARE && evsel->pmu && evsel->pmu->is_core &&
- evsel->alternate_hw_config == config;
- }
-
- if ((type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE) &&
- perf_pmus__supports_extended_type())
- e_config &= PERF_HW_EVENT_MASK;
-
- return e_config == config;
-}
-
int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread)
{
if (evsel__is_tool(evsel))
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index aae431d63d64..c199481d3338 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -87,7 +87,6 @@ struct evsel {
int bpf_fd;
struct bpf_object *bpf_obj;
struct list_head config_terms;
- u64 alternate_hw_config;
};
/*
@@ -370,10 +369,26 @@ u64 format_field__intval(struct tep_format_field *field, struct perf_sample *sam
struct tep_format_field *evsel__field(struct evsel *evsel, const char *name);
struct tep_format_field *evsel__common_field(struct evsel *evsel, const char *name);
-bool __evsel__match(const struct evsel *evsel, u32 type, u64 config);
+static inline bool __evsel__match(const struct evsel *evsel, u32 type, u64 config)
+{
+ if (evsel->core.attr.type != type)
+ return false;
+
+ if ((type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE) &&
+ perf_pmus__supports_extended_type())
+ return (evsel->core.attr.config & PERF_HW_EVENT_MASK) == config;
+
+ return evsel->core.attr.config == config;
+}
#define evsel__match(evsel, t, c) __evsel__match(evsel, PERF_TYPE_##t, PERF_COUNT_##c)
+static inline bool evsel__match2(struct evsel *e1, struct evsel *e2)
+{
+ return (e1->core.attr.type == e2->core.attr.type) &&
+ (e1->core.attr.config == e2->core.attr.config);
+}
+
int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread);
int __evsel__read_on_cpu(struct evsel *evsel, int cpu_map_idx, int thread, bool scale);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 04bb70ba4292..a71492829c58 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -229,7 +229,7 @@ __add_event(struct list_head *list, int *idx,
bool init_attr,
const char *name, const char *metric_id, struct perf_pmu *pmu,
struct list_head *config_terms, bool auto_merge_stats,
- struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
+ struct perf_cpu_map *cpu_list)
{
struct evsel *evsel;
struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
@@ -264,7 +264,6 @@ __add_event(struct list_head *list, int *idx,
evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
evsel->auto_merge_stats = auto_merge_stats;
evsel->pmu = pmu;
- evsel->alternate_hw_config = alternate_hw_config;
if (name)
evsel->name = strdup(name);
@@ -287,19 +286,16 @@ struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
{
return __add_event(/*list=*/NULL, &idx, attr, /*init_attr=*/false, name,
metric_id, pmu, /*config_terms=*/NULL,
- /*auto_merge_stats=*/false, /*cpu_list=*/NULL,
- /*alternate_hw_config=*/PERF_COUNT_HW_MAX);
+ /*auto_merge_stats=*/false, /*cpu_list=*/NULL);
}
static int add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr, const char *name,
- const char *metric_id, struct list_head *config_terms,
- u64 alternate_hw_config)
+ const char *metric_id, struct list_head *config_terms)
{
return __add_event(list, idx, attr, /*init_attr*/true, name, metric_id,
/*pmu=*/NULL, config_terms,
- /*auto_merge_stats=*/false, /*cpu_list=*/NULL,
- alternate_hw_config) ? 0 : -ENOMEM;
+ /*auto_merge_stats=*/false, /*cpu_list=*/NULL) ? 0 : -ENOMEM;
}
/**
@@ -423,7 +419,7 @@ bool parse_events__filter_pmu(const struct parse_events_state *parse_state,
static int parse_events_add_pmu(struct parse_events_state *parse_state,
struct list_head *list, struct perf_pmu *pmu,
const struct parse_events_terms *const_parsed_terms,
- bool auto_merge_stats, u64 alternate_hw_config);
+ bool auto_merge_stats);
int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
struct parse_events_state *parse_state,
@@ -449,8 +445,7 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
*/
ret = parse_events_add_pmu(parse_state, list, pmu,
parsed_terms,
- perf_pmu__auto_merge_stats(pmu),
- /*alternate_hw_config=*/PERF_COUNT_HW_MAX);
+ perf_pmu__auto_merge_stats(pmu));
if (ret)
return ret;
continue;
@@ -481,8 +476,7 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
if (__add_event(list, idx, &attr, /*init_attr*/true, config_name ?: name,
metric_id, pmu, &config_terms, /*auto_merge_stats=*/false,
- /*cpu_list=*/NULL,
- /*alternate_hw_config=*/PERF_COUNT_HW_MAX) == NULL)
+ /*cpu_list=*/NULL) == NULL)
return -ENOMEM;
free_config_terms(&config_terms);
@@ -750,7 +744,7 @@ int parse_events_add_breakpoint(struct parse_events_state *parse_state,
name = get_config_name(head_config);
return add_event(list, &parse_state->idx, &attr, name, /*mertic_id=*/NULL,
- &config_terms, /*alternate_hw_config=*/PERF_COUNT_HW_MAX);
+ &config_terms);
}
static int check_type_val(struct parse_events_term *term,
@@ -1354,9 +1348,8 @@ static 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 = __add_event(list, &parse_state->idx, &attr, /*init_attr*/true, name,
- metric_id, pmu, &config_terms, /*auto_merge_stats=*/false,
- /*cpu_list=*/NULL, /*alternate_hw_config=*/PERF_COUNT_HW_MAX
- ) == NULL ? -ENOMEM : 0;
+ metric_id, pmu, &config_terms, /*auto_merge_stats=*/false,
+ /*cpu_list=*/NULL) ? 0 : -ENOMEM;
free_config_terms(&config_terms);
return ret;
}
@@ -1407,7 +1400,7 @@ static bool config_term_percore(struct list_head *config_terms)
static int parse_events_add_pmu(struct parse_events_state *parse_state,
struct list_head *list, struct perf_pmu *pmu,
const struct parse_events_terms *const_parsed_terms,
- bool auto_merge_stats, u64 alternate_hw_config)
+ bool auto_merge_stats)
{
struct perf_event_attr attr;
struct perf_pmu_info info;
@@ -1444,7 +1437,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
/*init_attr=*/true, /*name=*/NULL,
/*metric_id=*/NULL, pmu,
/*config_terms=*/NULL, auto_merge_stats,
- /*cpu_list=*/NULL, alternate_hw_config);
+ /*cpu_list=*/NULL);
return evsel ? 0 : -ENOMEM;
}
@@ -1465,8 +1458,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
/* Look for event names in the terms and rewrite into format based terms. */
if (perf_pmu__check_alias(pmu, &parsed_terms,
- &info, &alias_rewrote_terms,
- &alternate_hw_config, err)) {
+ &info, &alias_rewrote_terms, err)) {
parse_events_terms__exit(&parsed_terms);
return -EINVAL;
}
@@ -1513,8 +1505,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
evsel = __add_event(list, &parse_state->idx, &attr, /*init_attr=*/true,
get_config_name(&parsed_terms),
get_config_metric_id(&parsed_terms), pmu,
- &config_terms, auto_merge_stats, /*cpu_list=*/NULL,
- alternate_hw_config);
+ &config_terms, auto_merge_stats, /*cpu_list=*/NULL);
if (!evsel) {
parse_events_terms__exit(&parsed_terms);
return -ENOMEM;
@@ -1535,7 +1526,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
}
int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
- const char *event_name, u64 hw_config,
+ const char *event_name,
const struct parse_events_terms *const_parsed_terms,
struct list_head **listp, void *loc_)
{
@@ -1588,7 +1579,7 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
auto_merge_stats = perf_pmu__auto_merge_stats(pmu);
if (!parse_events_add_pmu(parse_state, list, pmu,
- &parsed_terms, auto_merge_stats, hw_config)) {
+ &parsed_terms, auto_merge_stats)) {
struct strbuf sb;
strbuf_init(&sb, /*hint=*/ 0);
@@ -1601,7 +1592,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, perf_pmus__fake_pmu(), &parsed_terms,
- /*auto_merge_stats=*/true, hw_config)) {
+ /*auto_merge_stats=*/true)) {
struct strbuf sb;
strbuf_init(&sb, /*hint=*/ 0);
@@ -1642,15 +1633,13 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
/* Attempt to add to list assuming event_or_pmu is a PMU name. */
pmu = perf_pmus__find(event_or_pmu);
if (pmu && !parse_events_add_pmu(parse_state, *listp, pmu, const_parsed_terms,
- /*auto_merge_stats=*/false,
- /*alternate_hw_config=*/PERF_COUNT_HW_MAX))
+ /*auto_merge_stats=*/false))
return 0;
if (parse_state->fake_pmu) {
if (!parse_events_add_pmu(parse_state, *listp, perf_pmus__fake_pmu(),
const_parsed_terms,
- /*auto_merge_stats=*/false,
- /*alternate_hw_config=*/PERF_COUNT_HW_MAX))
+ /*auto_merge_stats=*/false))
return 0;
}
@@ -1663,8 +1652,7 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
if (!parse_events_add_pmu(parse_state, *listp, pmu,
const_parsed_terms,
- auto_merge_stats,
- /*alternate_hw_config=*/PERF_COUNT_HW_MAX)) {
+ auto_merge_stats)) {
ok++;
parse_state->wild_card_pmus = true;
}
@@ -1675,8 +1663,7 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
/* Failure to add, assume event_or_pmu is an event name. */
zfree(listp);
- if (!parse_events_multi_pmu_add(parse_state, event_or_pmu, PERF_COUNT_HW_MAX,
- const_parsed_terms, listp, loc))
+ if (!parse_events_multi_pmu_add(parse_state, event_or_pmu, const_parsed_terms, listp, loc))
return 0;
if (asprintf(&help, "Unable to find PMU or event on a PMU of '%s'", event_or_pmu) < 0)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index e176a34ab088..0edb8b14e522 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -128,12 +128,6 @@ struct parse_events_term {
* value is assumed to be 1. An event name also has no value.
*/
bool no_value;
- /**
- * @alternate_hw_config: config is the event name but num is an
- * alternate PERF_TYPE_HARDWARE config value which is often nice for the
- * sake of quick matching.
- */
- bool alternate_hw_config;
};
struct parse_events_error {
@@ -244,7 +238,7 @@ struct evsel *parse_events__add_event(int idx, struct perf_event_attr *attr,
struct perf_pmu *pmu);
int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
- const char *event_name, u64 hw_config,
+ const char *event_name,
const struct parse_events_terms *const_parsed_terms,
struct list_head **listp, void *loc);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index f888cbb076d6..5c4669264177 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -290,7 +290,7 @@ PE_NAME sep_dc
struct list_head *list;
int err;
- err = parse_events_multi_pmu_add(_parse_state, $1, PERF_COUNT_HW_MAX, NULL, &list, &@1);
+ err = parse_events_multi_pmu_add(_parse_state, $1, NULL, &list, &@1);
if (err < 0) {
struct parse_events_state *parse_state = _parse_state;
struct parse_events_error *error = parse_state->error;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 57450c73fb63..7b32b9638d5f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1662,7 +1662,7 @@ static int check_info_data(struct perf_pmu *pmu,
*/
int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_terms,
struct perf_pmu_info *info, bool *rewrote_terms,
- u64 *alternate_hw_config, struct parse_events_error *err)
+ struct parse_events_error *err)
{
struct parse_events_term *term, *h;
struct perf_pmu_alias *alias;
@@ -1699,7 +1699,6 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
NULL);
return ret;
}
-
*rewrote_terms = true;
ret = check_info_data(pmu, alias, info, err, term->err_term);
if (ret)
@@ -1708,9 +1707,6 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
if (alias->per_pkg)
info->per_pkg = true;
- if (term->alternate_hw_config)
- *alternate_hw_config = term->val.num;
-
list_del_init(&term->list);
parse_events_term__delete(term);
}
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index b93014cc3670..71b979a4a9dc 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -231,7 +231,7 @@ __u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name);
int perf_pmu__format_type(struct perf_pmu *pmu, const char *name);
int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_terms,
struct perf_pmu_info *info, bool *rewrote_terms,
- u64 *alternate_hw_config, struct parse_events_error *err);
+ struct parse_events_error *err);
int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, pmu_event_callback cb);
void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
--
2.49.0.rc0.332.g42c0ae87b1-goog
Powered by blists - more mailing lists