From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>, James Clark <james.clark@linaro.org>
Cc: linux-perf-users@vger.kernel.org, irogers@google.com,
thomas.falcon@intel.com, Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
"Liang, Kan" <kan.liang@linux.intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] libperf: evlist: Fix --cpu argument on hybrid platform
Date: Tue, 10 Dec 2024 10:51:47 -0300 [thread overview]
Message-ID: <Z1hHc-Q-TKgagORu@x1> (raw)
In-Reply-To: <20241114160450.295844-2-james.clark@linaro.org>
On Thu, Nov 14, 2024 at 04:04:48PM +0000, James Clark wrote:
> Since the linked fixes: commit, specifying a CPU on hybrid platforms
> results in an error because Perf tries to open an extended type event
> on "any" CPU which isn't valid. Extended type events can only be opened
> on CPUs that match the type.
> Before (working):
> $ perf record --cpu 1 -- true
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 2.385 MB perf.data (7 samples) ]
> After (not working):
> $ perf record -C 1 -- true
> WARNING: A requested CPU in '1' is not supported by PMU 'cpu_atom' (CPUs 16-27) for event 'cycles:P'
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cpu_atom/cycles:P/).
> /bin/dmesg | grep -i perf may provide additional information.
>
> (Ignore the warning message, that's expected and not particularly
> relevant to this issue).
>
> This is because perf_cpu_map__intersect() of the user specified CPU (1)
> and one of the PMU's CPUs (16-27) correctly results in an empty (NULL)
> CPU map. However for the purposes of opening an event, libperf converts
> empty CPU maps into an any CPU (-1) which the kernel rejects.
>
> Fix it by deleting evsels with empty CPU maps in the specific case where
> user requested CPU maps are evaluated.
Namhyung, I think this should go via perf-tools,
Before, on an hybrid Intel processor:
⬢ [acme@toolbox perf-tools]$ grep -m1 'model name' /proc/cpuinfo
model name : 13th Gen Intel(R) Core(TM) i7-1365U
⬢ [acme@toolbox perf-tools]$
root@x1:~# perf record -C 1 -- true
WARNING: A requested CPU in '1' is not supported by PMU 'cpu_atom' (CPUs 4-11) for event 'cycles:P'
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (cpu_atom/cycles/P).
"dmesg | grep -i perf" may provide additional information.
root@x1:~#
After the patch:
root@x1:~# perf record -C 1 -- true
WARNING: A requested CPU in '1' is not supported by PMU 'cpu_atom' (CPUs 4-11) for event 'cycles:P'
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.369 MB perf.data (8 samples) ]
root@x1:~#
root@x1:~# perf evlist -v
cpu_core/cycles/P: type: 0 (PERF_TYPE_HARDWARE), size: 136, config: 0x400000000, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CPU|PERIOD|IDENTIFIER, read_format: ID|LOST, disabled: 1, freq: 1, precise_ip: 3, sample_id_all: 1
dummy:u: type: 1 (software), size: 136, config: 0x9 (PERF_COUNT_SW_DUMMY), { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU|IDENTIFIER, read_format: ID|LOST, exclude_kernel: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
root@x1:~#
All on CPU 1:
root@x1:~# perf report -D | grep PERF_RECORD_SAMPLE
1 1809541166457609 0x15e410 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9b1ce418 period: 1 addr: 0
1 1809541166470019 0x15e448 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9a0206b1 period: 1 addr: 0
1 1809541166475543 0x15e480 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9a0a9158 period: 5 addr: 0
1 1809541166478316 0x15e4b8 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9a0a9158 period: 53 addr: 0
1 1809541166480879 0x15e4f0 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9a0a9158 period: 667 addr: 0
1 1809541166483895 0x15e528 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9b1bcbcc period: 8537 addr: 0
1 1809541166489116 0x15e560 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 719180/719180: 0xffffffff9a009b3b period: 98429 addr: 0
1 1809541168375508 0x15e620 [0x38]: PERF_RECORD_SAMPLE(IP, 0x4001): 0/0: 0xffffffff9b0e2a34 period: 732596 addr: 0
root@x1:~#
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
James, the second patch isn't applying to perf-tools/perf-tools.
- Arnaldo
> Fixes: 251aa040244a ("perf parse-events: Wildcard most "numeric" events")
> Reviewed-by: Ian Rogers <irogers@google.com>
> Tested-by: Thomas Falcon <thomas.falcon@intel.com>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> tools/lib/perf/evlist.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index c6d67fc9e57e..83c43dc13313 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -47,6 +47,20 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
> */
> perf_cpu_map__put(evsel->cpus);
> evsel->cpus = perf_cpu_map__intersect(evlist->user_requested_cpus, evsel->own_cpus);
> +
> + /*
> + * Empty cpu lists would eventually get opened as "any" so remove
> + * genuinely empty ones before they're opened in the wrong place.
> + */
> + if (perf_cpu_map__is_empty(evsel->cpus)) {
> + struct perf_evsel *next = perf_evlist__next(evlist, evsel);
> +
> + perf_evlist__remove(evlist, evsel);
> + /* Keep idx contiguous */
> + if (next)
> + list_for_each_entry_from(next, &evlist->entries, node)
> + next->idx--;
> + }
> } else if (!evsel->own_cpus || evlist->has_user_cpus ||
> (!evsel->requires_cpu && perf_cpu_map__has_any_cpu(evlist->user_requested_cpus))) {
> /*
> @@ -80,11 +94,11 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
>
> static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> {
> - struct perf_evsel *evsel;
> + struct perf_evsel *evsel, *n;
>
> evlist->needs_map_propagation = true;
>
> - perf_evlist__for_each_evsel(evlist, evsel)
> + list_for_each_entry_safe(evsel, n, &evlist->entries, node)
> __perf_evlist__propagate_maps(evlist, evsel);
> }
>
> --
> 2.34.1
next prev parent reply other threads:[~2024-12-10 13:51 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-14 16:04 [PATCH v2 0/2] libperf: evlist: Fix --cpu argument on hybrid platform James Clark
2024-11-14 16:04 ` [PATCH v2 1/2] " James Clark
2024-12-10 13:51 ` Arnaldo Carvalho de Melo [this message]
2024-12-10 13:56 ` James Clark
2024-12-10 14:07 ` Arnaldo Carvalho de Melo
2024-12-10 18:10 ` Namhyung Kim
2024-12-12 7:10 ` Namhyung Kim
2024-11-14 16:04 ` [PATCH v2 2/2] libperf: evlist: Keep evsel idx contiguous on removal James Clark
2024-11-14 23:35 ` 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=Z1hHc-Q-TKgagORu@x1 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=thomas.falcon@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.