Skip to content

Commit e8d3834

Browse files
captain5050acmel
authored andcommitted
perf parse-events: When fixing group leaders always set the leader
The evsel grouping fix iterates over evsels tracking the leader group and the current position's group, updating the current position's leader if an evsel is being forced into a group or groups changed. However, groups changing isn't a sufficient condition as sorting may have reordered events and the leader may no longer come first. For this reason update all leaders whenever they disagree. This change breaks certain Icelake+ metrics due to bugs in the kernel. For example, tma_l3_bound with threshold enabled tries to program the events: {topdown-retiring,slots,CYCLE_ACTIVITY.STALLS_L2_MISS,topdown-fe-bound,EXE_ACTIVITY.BOUND_ON_STORES,EXE_ACTIVITY.1_PORTS_UTIL,topdown-be-bound,cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/,CYCLE_ACTIVITY.STALLS_L3_MISS,CPU_CLK_UNHALTED.THREAD,CYCLE_ACTIVITY.STALLS_MEM_ANY,EXE_ACTIVITY.2_PORTS_UTIL,CYCLE_ACTIVITY.STALLS_TOTAL,topdown-bad-spec}:W fixing the perf metric event order gives: {slots,topdown-retiring,topdown-fe-bound,topdown-be-bound,topdown-bad-spec,CYCLE_ACTIVITY.STALLS_L2_MISS,EXE_ACTIVITY.BOUND_ON_STORES,EXE_ACTIVITY.1_PORTS_UTIL,cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/,CYCLE_ACTIVITY.STALLS_L3_MISS,CPU_CLK_UNHALTED.THREAD,CYCLE_ACTIVITY.STALLS_MEM_ANY,EXE_ACTIVITY.2_PORTS_UTIL,CYCLE_ACTIVITY.STALLS_TOTAL}:W Both of these return "<not counted>" for all events, whilst they work with the group removed respecting that the perf metric events must still be grouped. A vendor events update will need to add METRIC_NO_GROUP to these metrics to workaround the kernel PMU driver issue. Fixes: a90cc5a ("perf evsel: Don't let evsel__group_pmu_name() traverse unsorted group") Signed-off-by: Ian Rogers <[email protected]> Tested-by: Andi Kleen <[email protected]> Cc: Adrian Hunter <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Ian Rogers <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Kan Liang <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Xing Zhengjun <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 5c49b6c commit e8d3834

File tree

1 file changed

+3
-8
lines changed

1 file changed

+3
-8
lines changed

tools/perf/util/parse-events.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2189,7 +2189,7 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
21892189
list_for_each_entry(pos, list, core.node) {
21902190
const struct evsel *pos_leader = evsel__leader(pos);
21912191
const char *pos_pmu_name = pos->group_pmu_name;
2192-
const char *cur_leader_pmu_name, *pos_leader_pmu_name;
2192+
const char *cur_leader_pmu_name;
21932193
bool pos_force_grouped = arch_evsel__must_be_in_group(pos);
21942194

21952195
/* Reset index and nr_members. */
@@ -2223,13 +2223,8 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
22232223
*/
22242224
cur_leader_force_grouped = pos_force_grouped;
22252225
}
2226-
pos_leader_pmu_name = pos_leader->group_pmu_name;
2227-
if (strcmp(pos_leader_pmu_name, pos_pmu_name) || pos_force_grouped) {
2228-
/*
2229-
* Event's PMU differs from its leader's. Groups can't
2230-
* span PMUs, so update leader from the group/PMU
2231-
* tracker.
2232-
*/
2226+
if (pos_leader != cur_leader) {
2227+
/* The leader changed so update it. */
22332228
evsel__set_leader(pos, cur_leader);
22342229
}
22352230
}

0 commit comments

Comments
 (0)