Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 11 Dec 2023 09:23:49 -0800 | Subject | Re: 'perf top' broken on intel hybrid systems |
| |
On Sat, Dec 9, 2023 at 9:26 PM Ian Rogers <irogers@google.com> wrote: > > On Sat, Dec 9, 2023, 12:14 PM Leah Neukirchen <leah@vuxu.org> wrote: >> >> >> > The error is because the perf top always tries to open an event on the >> > user_requested_cpus, which are all CPUs by default. >> > >> > Something as below should fix it. For hybrid, open a PMU event on an >> > unsupported CPU should be error out. >> > >> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c >> >> This fixes "perf top" on my i7-1355U. It would be great if you could >> get this patch into upstream and stable branches. > > > > I'll try to take a look ASAP. As we don't need this for record there is some existing logic that perf top is clearly missing. We should also have a test on perf top.
So for stat and record there is a call to evlist__create_maps: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/builtin-stat.c?h=perf-tools-next#n2730 https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/builtin-record.c?h=perf-tools-next#n4210
In evlist__create_maps we do perf_evlist__propagate_maps and that will: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/perf/evlist.c?h=perf-tools-next#n43 ``` } else if (evlist->has_user_cpus && evsel->is_pmu_core) { /* * User requested CPUs on a core PMU, ensure the requested CPUs * are valid by intersecting with those of the PMU. */ perf_cpu_map__put(evsel->cpus); evsel->cpus = perf_cpu_map__intersect(evlist->user_requested_cpus, evsel->own_cpus); ```
I think the fix should be looking to do the same map set up in top as stat and record, presumably there was a refactor and top was overlooked. I'll try to address this and add a basic sanity test of doing a few iterations of perf top on stdio, which would have been sufficient to catch this.
Thanks, Ian
| |