Messages in this thread | | | Date | Wed, 3 May 2023 16:58:02 -0400 | Subject | Re: [PATCH v4 04/44] perf parse-events: Don't reorder ungrouped events by pmu | From | "Liang, Kan" <> |
| |
On 2023-05-02 6:38 p.m., Ian Rogers wrote: > The pmu_group_name by default returns "cpu" which on non-hybrid/ARM > means that ungrouped software, and hardware events are all going to > sort by the original insertion index. However, on hybrid and ARM > wildcard expansion may mean the PMU name is set and events will be > unnecessarily reordered - triggering the reordering warning. > > Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Kan Liang <kan.liang@linux.intel.com>
Thanks, Kan
> --- > tools/perf/util/parse-events.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index d71019dcd614..34ba840ae19a 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -2140,25 +2140,32 @@ static int evlist__cmp(void *state, const struct list_head *l, const struct list > int *leader_idx = state; > int lhs_leader_idx = *leader_idx, rhs_leader_idx = *leader_idx, ret; > const char *lhs_pmu_name, *rhs_pmu_name; > + bool lhs_has_group = false, rhs_has_group = false; > > /* > * First sort by grouping/leader. Read the leader idx only if the evsel > * is part of a group, as -1 indicates no group. > */ > - if (lhs_core->leader != lhs_core || lhs_core->nr_members > 1) > + if (lhs_core->leader != lhs_core || lhs_core->nr_members > 1) { > + lhs_has_group = true; > lhs_leader_idx = lhs_core->leader->idx; > - if (rhs_core->leader != rhs_core || rhs_core->nr_members > 1) > + } > + if (rhs_core->leader != rhs_core || rhs_core->nr_members > 1) { > + rhs_has_group = true; > rhs_leader_idx = rhs_core->leader->idx; > + } > > if (lhs_leader_idx != rhs_leader_idx) > return lhs_leader_idx - rhs_leader_idx; > > - /* Group by PMU. Groups can't span PMUs. */ > - lhs_pmu_name = evsel__group_pmu_name(lhs); > - rhs_pmu_name = evsel__group_pmu_name(rhs); > - ret = strcmp(lhs_pmu_name, rhs_pmu_name); > - if (ret) > - return ret; > + /* Group by PMU if there is a group. Groups can't span PMUs. */ > + if (lhs_has_group && rhs_has_group) { > + lhs_pmu_name = evsel__group_pmu_name(lhs); > + rhs_pmu_name = evsel__group_pmu_name(rhs); > + ret = strcmp(lhs_pmu_name, rhs_pmu_name); > + if (ret) > + return ret; > + } > > /* Architecture specific sorting. */ > return arch_evlist__cmp(lhs, rhs);
| |