Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 10 Oct 2022 15:31:41 -0700 | Subject | Re: [PATCH 02/19] perf tools: Use pmu info in evsel__is_hybrid() |
| |
On Sun, Oct 9, 2022 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote: > > If evsel has pmu, it can use pmu->is_hybrid directly. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/evsel.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 196f8e4859d7..a6ea91c72659 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -3132,6 +3132,9 @@ void evsel__zero_per_pkg(struct evsel *evsel) > > bool evsel__is_hybrid(struct evsel *evsel) > { > + if (evsel->pmu) > + return evsel->pmu->is_hybrid; > + > return evsel->pmu_name && perf_pmu__is_hybrid(evsel->pmu_name);
Wow, there's so much duplicated state. Why do evsels have a pmu_name and a pmu? Why not just pmu->name? I feel always having a pmu would be cleanest here. That said what does evsel__is_hybrid even mean? Does it mean this event is on a PMU normally called cpu and called cpu_core and cpu_atom on hybrid systems? And of course there are no comments to explain what this little mystery could be. Anyway, that's not a fault of this change, and probably later changes will go someway toward cleaning this up. It was a shame the code wasn't cleaner in the first place.
Acked-by: Ian Rogers
Thanks, Ian
> } > > -- > 2.38.0.rc1.362.ged0d419d3c-goog >
| |