Skip to content

Commit 8dcd27b

Browse files
captain5050namhyung
authored andcommitted
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@google.com> Link: https://lore.kernel.org/r/20250719030517.1990983-15-irogers@google.com Signed-off-by: Namhyung Kim <namhyung@kernel.org>
1 parent 5b546de commit 8dcd27b

6 files changed

Lines changed: 77 additions & 12 deletions

File tree

tools/perf/arch/x86/util/evlist.c

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,27 @@ int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs)
8181
/* Default ordering by insertion index. */
8282
return lhs->core.idx - rhs->core.idx;
8383
}
84+
85+
int arch_evlist__add_required_events(struct list_head *list)
86+
{
87+
struct evsel *pos, *metric_event = NULL;
88+
int idx = 0;
89+
90+
if (!topdown_sys_has_perf_metrics())
91+
return 0;
92+
93+
list_for_each_entry(pos, list, core.node) {
94+
if (arch_is_topdown_slots(pos)) {
95+
/* Slots event already present, nothing to do. */
96+
return 0;
97+
}
98+
if (metric_event == NULL && arch_is_topdown_metrics(pos))
99+
metric_event = pos;
100+
idx++;
101+
}
102+
if (metric_event == NULL) {
103+
/* No topdown metric events, nothing to do. */
104+
return 0;
105+
}
106+
return topdown_insert_slots_event(list, idx + 1, metric_event);
107+
}

tools/perf/arch/x86/util/topdown.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,31 @@ bool arch_topdown_sample_read(struct evsel *leader)
7777

7878
return false;
7979
}
80+
81+
/*
82+
* Make a copy of the topdown metric event metric_event with the given index but
83+
* change its configuration to be a topdown slots event. Copying from
84+
* metric_event ensures modifiers are the same.
85+
*/
86+
int topdown_insert_slots_event(struct list_head *list, int idx, struct evsel *metric_event)
87+
{
88+
struct evsel *evsel = evsel__new_idx(&metric_event->core.attr, idx);
89+
90+
if (!evsel)
91+
return -ENOMEM;
92+
93+
evsel->core.attr.config = TOPDOWN_SLOTS;
94+
evsel->core.cpus = perf_cpu_map__get(metric_event->core.cpus);
95+
evsel->core.pmu_cpus = perf_cpu_map__get(metric_event->core.pmu_cpus);
96+
evsel->core.is_pmu_core = true;
97+
evsel->pmu = metric_event->pmu;
98+
evsel->name = strdup("slots");
99+
evsel->precise_max = metric_event->precise_max;
100+
evsel->sample_read = metric_event->sample_read;
101+
evsel->weak_group = metric_event->weak_group;
102+
evsel->bpf_counter = metric_event->bpf_counter;
103+
evsel->retire_lat = metric_event->retire_lat;
104+
evsel__set_leader(evsel, evsel__leader(metric_event));
105+
list_add_tail(&evsel->core.node, list);
106+
return 0;
107+
}

tools/perf/arch/x86/util/topdown.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
#include <stdbool.h>
66

77
struct evsel;
8+
struct list_head;
89

910
bool topdown_sys_has_perf_metrics(void);
1011
bool arch_is_topdown_slots(const struct evsel *evsel);
1112
bool arch_is_topdown_metrics(const struct evsel *evsel);
13+
int topdown_insert_slots_event(struct list_head *list, int idx, struct evsel *metric_event);
1214

1315
#endif

tools/perf/tests/parse-events.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -719,20 +719,20 @@ static int test__checkevent_pmu_partial_time_callgraph(struct evlist *evlist)
719719

720720
static int test__checkevent_pmu_events(struct evlist *evlist)
721721
{
722-
struct evsel *evsel = evlist__first(evlist);
722+
struct evsel *evsel;
723723

724-
TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->core.nr_entries);
725-
TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type ||
726-
strcmp(evsel->pmu->name, "cpu"));
727-
TEST_ASSERT_VAL("wrong exclude_user",
728-
!evsel->core.attr.exclude_user);
729-
TEST_ASSERT_VAL("wrong exclude_kernel",
730-
evsel->core.attr.exclude_kernel);
731-
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
732-
TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
733-
TEST_ASSERT_VAL("wrong pinned", !evsel->core.attr.pinned);
734-
TEST_ASSERT_VAL("wrong exclusive", !evsel->core.attr.exclusive);
724+
TEST_ASSERT_VAL("wrong number of entries", 1 <= evlist->core.nr_entries);
735725

726+
evlist__for_each_entry(evlist, evsel) {
727+
TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type ||
728+
strcmp(evsel->pmu->name, "cpu"));
729+
TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
730+
TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
731+
TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
732+
TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
733+
TEST_ASSERT_VAL("wrong pinned", !evsel->core.attr.pinned);
734+
TEST_ASSERT_VAL("wrong exclusive", !evsel->core.attr.exclusive);
735+
}
736736
return TEST_OK;
737737
}
738738

tools/perf/util/evlist.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ void evlist__add(struct evlist *evlist, struct evsel *entry);
111111
void evlist__remove(struct evlist *evlist, struct evsel *evsel);
112112

113113
int arch_evlist__cmp(const struct evsel *lhs, const struct evsel *rhs);
114+
int arch_evlist__add_required_events(struct list_head *list);
114115

115116
int evlist__add_dummy(struct evlist *evlist);
116117
struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide);

tools/perf/util/parse-events.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2190,6 +2190,11 @@ static int evlist__cmp(void *_fg_idx, const struct list_head *l, const struct li
21902190
return arch_evlist__cmp(lhs, rhs);
21912191
}
21922192

2193+
int __weak arch_evlist__add_required_events(struct list_head *list __always_unused)
2194+
{
2195+
return 0;
2196+
}
2197+
21932198
static int parse_events__sort_events_and_fix_groups(struct list_head *list)
21942199
{
21952200
int idx = 0, force_grouped_idx = -1;
@@ -2201,6 +2206,11 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
22012206
struct evsel *force_grouped_leader = NULL;
22022207
bool last_event_was_forced_leader = false;
22032208

2209+
/* On x86 topdown metrics events require a slots event. */
2210+
ret = arch_evlist__add_required_events(list);
2211+
if (ret)
2212+
return ret;
2213+
22042214
/*
22052215
* Compute index to insert ungrouped events at. Place them where the
22062216
* first ungrouped event appears.

0 commit comments

Comments
 (0)