tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

perf parse-events: Corrections to topdown sorting

In the case of '{instructions,slots},faults,topdown-retiring' the
first event that must be grouped, slots, is ignored causing the
topdown-retiring event not to be adjacent to the group it needs to be
inserted into. Don't ignore the group members when computing the
force_grouped_index.

Make the force_grouped_index be for the leader of the group it is
within and always use it first rather than a group leader index so
that topdown events may be sorted from one group into another.

As the PMU name comparison applies to moving events in the same group
ensure the name ordering is always respected.

Change the group splitting logic to not group if there are no other
topdown events and to fix cases where the force group leader wasn't
being grouped with the other members of its group.

Reported-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Closes: https://lore.kernel.org/lkml/20250224083306.71813-2-dapeng1.mi@linux.intel.com/
Closes: https://lore.kernel.org/lkml/f7e4f7e8-748c-4ec7-9088-0e844392c11a@linux.intel.com/
Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Link: https://lore.kernel.org/r/20250307023906.1135613-3-irogers@google.com
Signed-off-by: Namhyung Kim <namhyung@kernel.org>

authored by

Ian Rogers and committed by
Namhyung Kim 9a1c57fe b74683b3

+104 -54
+9 -6
tools/perf/arch/x86/util/evlist.c
··· 76 76 * topdown metrics events are already in same group with slots 77 77 * event, do nothing. 78 78 */ 79 - if (arch_is_topdown_metrics(lhs) && !arch_is_topdown_metrics(rhs) && 80 - lhs->core.leader != rhs->core.leader) 81 - return -1; 82 - if (!arch_is_topdown_metrics(lhs) && arch_is_topdown_metrics(rhs) && 83 - lhs->core.leader != rhs->core.leader) 84 - return 1; 79 + if (lhs->core.leader != rhs->core.leader) { 80 + bool lhs_topdown = arch_is_topdown_metrics(lhs); 81 + bool rhs_topdown = arch_is_topdown_metrics(rhs); 82 + 83 + if (lhs_topdown && !rhs_topdown) 84 + return -1; 85 + if (!lhs_topdown && rhs_topdown) 86 + return 1; 87 + } 85 88 } 86 89 87 90 /* Retire latency event should not be group leader*/
+95 -48
tools/perf/util/parse-events.c
··· 1980 1980 int *force_grouped_idx = _fg_idx; 1981 1981 int lhs_sort_idx, rhs_sort_idx, ret; 1982 1982 const char *lhs_pmu_name, *rhs_pmu_name; 1983 - bool lhs_has_group, rhs_has_group; 1984 1983 1985 1984 /* 1986 - * First sort by grouping/leader. Read the leader idx only if the evsel 1987 - * is part of a group, by default ungrouped events will be sorted 1988 - * relative to grouped events based on where the first ungrouped event 1989 - * occurs. If both events don't have a group we want to fall-through to 1990 - * the arch specific sorting, that can reorder and fix things like 1991 - * Intel's topdown events. 1985 + * Get the indexes of the 2 events to sort. If the events are 1986 + * in groups then the leader's index is used otherwise the 1987 + * event's index is used. An index may be forced for events that 1988 + * must be in the same group, namely Intel topdown events. 1992 1989 */ 1993 - if (lhs_core->leader != lhs_core || lhs_core->nr_members > 1) { 1994 - lhs_has_group = true; 1995 - lhs_sort_idx = lhs_core->leader->idx; 1990 + if (*force_grouped_idx != -1 && arch_evsel__must_be_in_group(lhs)) { 1991 + lhs_sort_idx = *force_grouped_idx; 1996 1992 } else { 1997 - lhs_has_group = false; 1998 - lhs_sort_idx = *force_grouped_idx != -1 && arch_evsel__must_be_in_group(lhs) 1999 - ? *force_grouped_idx 2000 - : lhs_core->idx; 1993 + bool lhs_has_group = lhs_core->leader != lhs_core || lhs_core->nr_members > 1; 1994 + 1995 + lhs_sort_idx = lhs_has_group ? lhs_core->leader->idx : lhs_core->idx; 2001 1996 } 2002 - if (rhs_core->leader != rhs_core || rhs_core->nr_members > 1) { 2003 - rhs_has_group = true; 2004 - rhs_sort_idx = rhs_core->leader->idx; 1997 + if (*force_grouped_idx != -1 && arch_evsel__must_be_in_group(rhs)) { 1998 + rhs_sort_idx = *force_grouped_idx; 2005 1999 } else { 2006 - rhs_has_group = false; 2007 - rhs_sort_idx = *force_grouped_idx != -1 && arch_evsel__must_be_in_group(rhs) 2008 - ? *force_grouped_idx 2009 - : rhs_core->idx; 2000 + bool rhs_has_group = rhs_core->leader != rhs_core || rhs_core->nr_members > 1; 2001 + 2002 + rhs_sort_idx = rhs_has_group ? rhs_core->leader->idx : rhs_core->idx; 2010 2003 } 2011 2004 2005 + /* If the indices differ then respect the insertion order. */ 2012 2006 if (lhs_sort_idx != rhs_sort_idx) 2013 2007 return lhs_sort_idx - rhs_sort_idx; 2014 2008 2015 - /* Group by PMU if there is a group. Groups can't span PMUs. */ 2016 - if (lhs_has_group && rhs_has_group) { 2017 - lhs_pmu_name = lhs->group_pmu_name; 2018 - rhs_pmu_name = rhs->group_pmu_name; 2019 - ret = strcmp(lhs_pmu_name, rhs_pmu_name); 2020 - if (ret) 2021 - return ret; 2022 - } 2009 + /* 2010 + * Ignoring forcing, lhs_sort_idx == rhs_sort_idx so lhs and rhs should 2011 + * be in the same group. Events in the same group need to be ordered by 2012 + * their grouping PMU name as the group will be broken to ensure only 2013 + * events on the same PMU are programmed together. 2014 + * 2015 + * With forcing the lhs_sort_idx == rhs_sort_idx shows that one or both 2016 + * events are being forced to be at force_group_index. If only one event 2017 + * is being forced then the other event is the group leader of the group 2018 + * we're trying to force the event into. Ensure for the force grouped 2019 + * case that the PMU name ordering is also respected. 2020 + */ 2021 + lhs_pmu_name = lhs->group_pmu_name; 2022 + rhs_pmu_name = rhs->group_pmu_name; 2023 + ret = strcmp(lhs_pmu_name, rhs_pmu_name); 2024 + if (ret) 2025 + return ret; 2023 2026 2024 - /* Architecture specific sorting. */ 2027 + /* 2028 + * Architecture specific sorting, by default sort events in the same 2029 + * group with the same PMU by their insertion index. On Intel topdown 2030 + * constraints must be adhered to - slots first, etc. 2031 + */ 2025 2032 return arch_evlist__cmp(lhs, rhs); 2026 2033 } 2027 2034 ··· 2037 2030 int idx = 0, force_grouped_idx = -1; 2038 2031 struct evsel *pos, *cur_leader = NULL; 2039 2032 struct perf_evsel *cur_leaders_grp = NULL; 2040 - bool idx_changed = false, cur_leader_force_grouped = false; 2033 + bool idx_changed = false; 2041 2034 int orig_num_leaders = 0, num_leaders = 0; 2042 2035 int ret; 2036 + struct evsel *force_grouped_leader = NULL; 2037 + bool last_event_was_forced_leader = false; 2043 2038 2044 2039 /* 2045 2040 * Compute index to insert ungrouped events at. Place them where the ··· 2064 2055 */ 2065 2056 pos->core.idx = idx++; 2066 2057 2067 - /* Remember an index to sort all forced grouped events together to. */ 2068 - if (force_grouped_idx == -1 && pos == pos_leader && pos->core.nr_members < 2 && 2069 - arch_evsel__must_be_in_group(pos)) 2070 - force_grouped_idx = pos->core.idx; 2058 + /* 2059 + * Remember an index to sort all forced grouped events 2060 + * together to. Use the group leader as some events 2061 + * must appear first within the group. 2062 + */ 2063 + if (force_grouped_idx == -1 && arch_evsel__must_be_in_group(pos)) 2064 + force_grouped_idx = pos_leader->core.idx; 2071 2065 } 2072 2066 2073 2067 /* Sort events. */ ··· 2098 2086 * Set the group leader respecting the given groupings and that 2099 2087 * groups can't span PMUs. 2100 2088 */ 2101 - if (!cur_leader) 2089 + if (!cur_leader) { 2102 2090 cur_leader = pos; 2091 + cur_leaders_grp = &pos->core; 2092 + if (pos_force_grouped) 2093 + force_grouped_leader = pos; 2094 + } 2103 2095 2104 2096 cur_leader_pmu_name = cur_leader->group_pmu_name; 2105 - if ((cur_leaders_grp != pos->core.leader && 2106 - (!pos_force_grouped || !cur_leader_force_grouped)) || 2107 - strcmp(cur_leader_pmu_name, pos_pmu_name)) { 2108 - /* Event is for a different group/PMU than last. */ 2097 + if (strcmp(cur_leader_pmu_name, pos_pmu_name)) { 2098 + /* PMU changed so the group/leader must change. */ 2109 2099 cur_leader = pos; 2110 - /* 2111 - * Remember the leader's group before it is overwritten, 2112 - * so that later events match as being in the same 2113 - * group. 2114 - */ 2115 2100 cur_leaders_grp = pos->core.leader; 2101 + if (pos_force_grouped && force_grouped_leader == NULL) 2102 + force_grouped_leader = pos; 2103 + } else if (cur_leaders_grp != pos->core.leader) { 2104 + bool split_even_if_last_leader_was_forced = true; 2105 + 2116 2106 /* 2117 - * Avoid forcing events into groups with events that 2118 - * don't need to be in the group. 2107 + * Event is for a different group. If the last event was 2108 + * the forced group leader then subsequent group events 2109 + * and forced events should be in the same group. If 2110 + * there are no other forced group events then the 2111 + * forced group leader wasn't really being forced into a 2112 + * group, it just set arch_evsel__must_be_in_group, and 2113 + * we don't want the group to split here. 2119 2114 */ 2120 - cur_leader_force_grouped = pos_force_grouped; 2115 + if (force_grouped_idx != -1 && last_event_was_forced_leader) { 2116 + struct evsel *pos2 = pos; 2117 + /* 2118 + * Search the whole list as the group leaders 2119 + * aren't currently valid. 2120 + */ 2121 + list_for_each_entry_continue(pos2, list, core.node) { 2122 + if (pos->core.leader == pos2->core.leader && 2123 + arch_evsel__must_be_in_group(pos2)) { 2124 + split_even_if_last_leader_was_forced = false; 2125 + break; 2126 + } 2127 + } 2128 + } 2129 + if (!last_event_was_forced_leader || split_even_if_last_leader_was_forced) { 2130 + if (pos_force_grouped) { 2131 + if (force_grouped_leader) { 2132 + cur_leader = force_grouped_leader; 2133 + cur_leaders_grp = force_grouped_leader->core.leader; 2134 + } else { 2135 + cur_leader = force_grouped_leader = pos; 2136 + cur_leaders_grp = &pos->core; 2137 + } 2138 + } else { 2139 + cur_leader = pos; 2140 + cur_leaders_grp = pos->core.leader; 2141 + } 2142 + } 2121 2143 } 2122 2144 if (pos_leader != cur_leader) { 2123 2145 /* The leader changed so update it. */ 2124 2146 evsel__set_leader(pos, cur_leader); 2125 2147 } 2148 + last_event_was_forced_leader = (force_grouped_leader == pos); 2126 2149 } 2127 2150 list_for_each_entry(pos, list, core.node) { 2128 2151 struct evsel *pos_leader = evsel__leader(pos);