[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250718132750.1546457-2-irogers@google.com>
Date: Fri, 18 Jul 2025 06:27:50 -0700
From: Ian Rogers <irogers@...gle.com>
To: Thomas Falcon <thomas.falcon@...el.com>, 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>,
"Liang, Kan" <kan.liang@...ux.intel.com>, Ravi Bangoria <ravi.bangoria@....com>,
James Clark <james.clark@...aro.org>, Dapeng Mi <dapeng1.mi@...ux.intel.com>,
Weilin Wang <weilin.wang@...el.com>, Andi Kleen <ak@...ux.intel.com>, linux-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org
Subject: [PATCH v2 2/2] perf parse-events: Fix missing slots for Intel topdown
metric events
Topdown metric events require grouping with a slots event. In perf
metrics this is currently achieved by metrics adding an unnecessary
"0 * tma_info_thread_slots". New TMA metrics trigger optimizations of
the metric expression that removes the event and breaks the metric due
to the missing but required event. Add a pass immediately before
sorting and fixing parsed events, that insert a slots event if one is
missing. Update test expectations to match this.
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/arch/x86/util/evlist.c | 24 ++++++++++++++++++++++++
tools/perf/arch/x86/util/topdown.c | 27 +++++++++++++++++++++++++++
tools/perf/arch/x86/util/topdown.h | 2 ++
tools/perf/tests/parse-events.c | 24 ++++++++++++------------
tools/perf/util/evlist.h | 1 +
tools/perf/util/parse-events.c | 10 ++++++++++
6 files changed, 76 insertions(+), 12 deletions(-)
diff --git a/tools/perf/arch/x86/util/evlist.c b/tools/perf/arch/x86/util/evlist.c
index 1969758cc8c1..75e9d00a1494 100644
--- a/tools/perf/arch/x86/util/evlist.c
+++ b/tools/perf/arch/x86/util/evlist.c
@@ -81,3 +81,27 @@ int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs)
/* Default ordering by insertion index. */
return lhs->core.idx - rhs->core.idx;
}
+
+int arch_evlist__add_required_events(struct list_head *list)
+{
+ struct evsel *pos, *metric_event = NULL;
+ int idx = 0;
+
+ if (!topdown_sys_has_perf_metrics())
+ return 0;
+
+ list_for_each_entry(pos, list, core.node) {
+ if (arch_is_topdown_slots(pos)) {
+ /* Slots event already present, nothing to do. */
+ return 0;
+ }
+ if (metric_event == NULL && arch_is_topdown_metrics(pos))
+ metric_event = pos;
+ idx++;
+ }
+ if (metric_event == NULL) {
+ /* No topdown metric events, nothing to do. */
+ return 0;
+ }
+ return topdown_insert_slots_event(list, idx + 1, metric_event);
+}
diff --git a/tools/perf/arch/x86/util/topdown.c b/tools/perf/arch/x86/util/topdown.c
index 66b231fbf52e..5d53e3d59c0c 100644
--- a/tools/perf/arch/x86/util/topdown.c
+++ b/tools/perf/arch/x86/util/topdown.c
@@ -77,3 +77,30 @@ bool arch_topdown_sample_read(struct evsel *leader)
return false;
}
+
+/*
+ * Make a copy of the topdown metric event metric_event with the given index but
+ * change its configuration to be a topdown slots event. Copying from
+ * metric_event ensures modifiers are the same.
+ */
+int topdown_insert_slots_event(struct list_head *list, int idx, struct evsel *metric_event)
+{
+ struct evsel *evsel = evsel__new_idx(&metric_event->core.attr, idx);
+
+ if (!evsel)
+ return -ENOMEM;
+
+ evsel->core.attr.config = TOPDOWN_SLOTS;
+ evsel->core.cpus = perf_cpu_map__get(metric_event->core.cpus);
+ evsel->core.own_cpus = perf_cpu_map__get(metric_event->core.own_cpus);
+ evsel->core.is_pmu_core = true;
+ evsel->pmu = metric_event->pmu;
+ evsel->name = strdup("slots");
+ evsel->precise_max = metric_event->precise_max;
+ evsel->sample_read = metric_event->sample_read;
+ evsel->weak_group = metric_event->weak_group;
+ evsel->bpf_counter = metric_event->bpf_counter;
+ evsel->retire_lat = metric_event->retire_lat;
+ list_add_tail(&evsel->core.node, list);
+ return 0;
+}
diff --git a/tools/perf/arch/x86/util/topdown.h b/tools/perf/arch/x86/util/topdown.h
index 2349536cf882..69035565e649 100644
--- a/tools/perf/arch/x86/util/topdown.h
+++ b/tools/perf/arch/x86/util/topdown.h
@@ -5,9 +5,11 @@
#include <stdbool.h>
struct evsel;
+struct list_head;
bool topdown_sys_has_perf_metrics(void);
bool arch_is_topdown_slots(const struct evsel *evsel);
bool arch_is_topdown_metrics(const struct evsel *evsel);
+int topdown_insert_slots_event(struct list_head *list, int idx, struct evsel *metric_event);
#endif
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 5ec2e5607987..bb8004397650 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -719,20 +719,20 @@ static int test__checkevent_pmu_partial_time_callgraph(struct evlist *evlist)
static int test__checkevent_pmu_events(struct evlist *evlist)
{
- struct evsel *evsel = evlist__first(evlist);
+ struct evsel *evsel;
- TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type ||
- strcmp(evsel->pmu->name, "cpu"));
- 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 pinned", !evsel->core.attr.pinned);
- TEST_ASSERT_VAL("wrong exclusive", !evsel->core.attr.exclusive);
+ TEST_ASSERT_VAL("wrong number of entries", 1 <= evlist->core.nr_entries);
+ evlist__for_each_entry(evlist, evsel) {
+ TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type ||
+ strcmp(evsel->pmu->name, "cpu"));
+ 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 pinned", !evsel->core.attr.pinned);
+ TEST_ASSERT_VAL("wrong exclusive", !evsel->core.attr.exclusive);
+ }
return TEST_OK;
}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index fac1a01ba13f..1472d2179be1 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -111,6 +111,7 @@ void evlist__add(struct evlist *evlist, struct evsel *entry);
void evlist__remove(struct evlist *evlist, struct evsel *evsel);
int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs);
+int arch_evlist__add_required_events(struct list_head *list);
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 a59ae5ca0f89..01dab5dd3540 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2132,6 +2132,11 @@ static int evlist__cmp(void *_fg_idx, const struct list_head *l, const struct li
return arch_evlist__cmp(lhs, rhs);
}
+int __weak arch_evlist__add_required_events(struct list_head *list __always_unused)
+{
+ return 0;
+}
+
static int parse_events__sort_events_and_fix_groups(struct list_head *list)
{
int idx = 0, force_grouped_idx = -1;
@@ -2143,6 +2148,11 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
struct evsel *force_grouped_leader = NULL;
bool last_event_was_forced_leader = false;
+ /* On x86 topdown metrics events require a slots event. */
+ ret = arch_evlist__add_required_events(list);
+ if (ret)
+ return ret;
+
/*
* Compute index to insert ungrouped events at. Place them where the
* first ungrouped event appears.
--
2.50.0.727.gbf7dc18ff4-goog
Powered by blists - more mailing lists