From: Ian Rogers <irogers@google.com>
To: Leah Neukirchen <leah@vuxu.org>
Cc: "Liang, Kan" <kan.liang@linux.intel.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users <linux-perf-users@vger.kernel.org>
Subject: Re: 'perf top' broken on intel hybrid systems
Date: Mon, 11 Dec 2023 09:23:49 -0800 [thread overview]
Message-ID: <CAP-5=fWdJYu9pf7d5ytvV7FNJTOK5Hr2mYcbDDPWO9zJMbPtdQ@mail.gmail.com> (raw)
In-Reply-To: <CAP-5=fVKEU1SesA52_jdR1OFAKF6Q6mrXjS+3+cibO6c=Or=6g@mail.gmail.com>
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
next prev parent reply other threads:[~2023-12-11 17:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-08 18:57 'perf top' broken on intel hybrid systems Arnaldo Carvalho de Melo
2023-12-08 19:39 ` Liang, Kan
2023-12-09 17:14 ` Leah Neukirchen
[not found] ` <CAP-5=fVKEU1SesA52_jdR1OFAKF6Q6mrXjS+3+cibO6c=Or=6g@mail.gmail.com>
2023-12-11 17:23 ` Ian Rogers [this message]
2023-12-11 21:10 ` Arnaldo Carvalho de Melo
2023-12-11 21:25 ` Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAP-5=fWdJYu9pf7d5ytvV7FNJTOK5Hr2mYcbDDPWO9zJMbPtdQ@mail.gmail.com' \
--to=irogers@google.com \
--cc=kan.liang@linux.intel.com \
--cc=leah@vuxu.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).