mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Ian Rogers <irogers@google.com>
Cc: Marc Zyngier <maz@kernel.org>, Hector Martin <marcan@marcan.st>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	James Clark <james.clark@arm.com>,
	linux-perf-users@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	Asahi Linux <asahi@lists.linux.dev>
Subject: Re: [REGRESSION] Perf (userspace) broken on big.LITTLE systems since v6.5
Date: Tue, 21 Nov 2023 15:56:22 +0000	[thread overview]
Message-ID: <ZVzTJhMsgjPw2OE4@FVFF77S0Q05N.cambridge.arm.com> (raw)
In-Reply-To: <CAP-5=fWeyorotfVz_y16ibakSwbNa0fapZoxSZ1nbkt1s=uGbw@mail.gmail.com>

On Tue, Nov 21, 2023 at 07:41:17AM -0800, Ian Rogers wrote:
> Hi Marc,

Hi Ian,

> I'm unclear if you are running a newer perf tool on an older kernel or
> not. In any case I'll assume the kernel and perf tool versions match.
> In Linux 6.6 this patch was added to the ARM PMU:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/perf/arm_pmu.c?id=5c816728651ae425954542fed64d21d40cb75a9f
> 
> My guess is that the apple_icestorm_pmu requires a similar patch. 

The apple_icestorm_pmu PMU driver uses the arm_pmu framework, so it's using
that code (since v6.6).

> The perf tool is supposed to not use extended types when they aren't
> supported:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/pmus.c?h=perf-tools-next#n532

How does that is_event_supported() check actually work? I suspect that's giving
the wrong answer.

Regardless, I think the tool is doing something semantically wrong, see below.

> So I share your confusion as to why something broke.
> 
> PERF_TYPE_HARDWARE is a legacy type where there are hardcoded type and
> config values that correspond to an event. The PMU driver turns legacy
> events into the real types. On BIG.little systems if the legacy events
> are monitoring a task a different event is needed for each PMU (ie >1
> event). In your example you are monitoring 'ls', a task, and so
> different cycles events are necessary. In the high 32-bits (the
> extended type) the PMU is identified.

I think the interesting thing here is that the tool is mapping events with an
explicit PMU into legacy PERF_TYPE_HARDWARE events, which is the opposite
direction than intended. Regardless of whether PERF_TYPE_HARDWARE events can be
targetted to a specific PMU, if the user has requested to use a specific PMU we
should be using that PMU and related event namespace.

Marc's command line was:

	sudo taskset -c 0 ./perf stat -vvv \
		-e apple_icestorm_pmu/cycles/ \
		-e apple_firestorm_pmu/cycles/ \
		-e cycles \
	ls

... and so the apple_*_pmu events should target their respective PMUs, and the
plain 'cycles' event could legitimately be opened as a single
PERF_TYPE_HARDWARE event, or split into two directed PERF_TYPE_HARDWARE events
targetting the two PMUs.

However, thwe tool opens three (undirected?) PERF_TYPE_HARDWARE events:

Opening: apple_icestorm_pmu/cycles/
------------------------------------------------------------
perf_event_attr:
  type                             0 (PERF_TYPE_HARDWARE)
  size                             136
  config                           0 (PERF_COUNT_HW_CPU_CYCLES)
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  enable_on_exec                   1
  exclude_guest                    1
------------------------------------------------------------
sys_perf_event_open: pid 1045843  cpu -1  group_fd -1  flags 0x8 = 3
Opening: apple_firestorm_pmu/cycles/
------------------------------------------------------------
perf_event_attr:
  type                             0 (PERF_TYPE_HARDWARE)
  size                             136
  config                           0 (PERF_COUNT_HW_CPU_CYCLES)
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  enable_on_exec                   1
  exclude_guest                    1
------------------------------------------------------------
sys_perf_event_open: pid 1045843  cpu -1  group_fd -1  flags 0x8 = 4
Opening: cycles
------------------------------------------------------------
perf_event_attr:
  type                             0 (PERF_TYPE_HARDWARE)
  size                             136
  config                           0 (PERF_COUNT_HW_CPU_CYCLES)
  sample_type                      IDENTIFIER
  read_format                      TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
  disabled                         1
  inherit                          1
  enable_on_exec                   1
  exclude_guest                    1
------------------------------------------------------------

Mark.

  reply	other threads:[~2023-11-21 15:56 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-21 12:08 Hector Martin
2023-11-21 13:40 ` Marc Zyngier
2023-11-21 15:24   ` Marc Zyngier
2023-11-21 15:40     ` Mark Rutland
2023-11-21 15:46       ` Ian Rogers
2023-11-21 16:02         ` Mark Rutland
2023-11-21 16:09           ` Ian Rogers
2023-11-21 16:15             ` Mark Rutland
2023-11-21 16:38               ` Ian Rogers
2023-11-22  3:23                 ` Hector Martin
2023-11-22 13:06                   ` Arnaldo Carvalho de Melo
2023-11-22 15:33                     ` Ian Rogers
2023-11-22 15:49                     ` Mark Rutland
2023-11-22 16:04                       ` Ian Rogers
2023-11-22 16:26                         ` Arnaldo Carvalho de Melo
2023-11-22 16:33                           ` Ian Rogers
2023-11-22 16:19                       ` Arnaldo Carvalho de Melo
2023-11-22 13:03                 ` Mark Rutland
2023-11-22 15:29                   ` Ian Rogers
2023-11-22 16:08                     ` Mark Rutland
2023-11-22 16:29                       ` Ian Rogers
2023-11-22 16:55                         ` Arnaldo Carvalho de Melo
2023-11-22 16:59                           ` Ian Rogers
2023-11-23  4:33                             ` Ian Rogers
2023-11-21 15:41     ` Ian Rogers
2023-11-21 15:56       ` Mark Rutland [this message]
2023-11-21 16:03         ` Ian Rogers
2023-11-21 16:08           ` Mark Rutland
2023-11-23 14:23     ` Mark Rutland
2023-11-23 14:45       ` Marc Zyngier
2023-11-23 15:14       ` Ian Rogers
2023-11-23 16:48         ` Mark Rutland
2023-11-23 17:08           ` James Clark
2023-11-23 17:15             ` Mark Rutland
2023-11-21 23:43 ` Bagas Sanjaya
2023-12-06 12:09   ` Linux regression tracking #update (Thorsten Leemhuis)
2024-08-01 19:05     ` Ian Rogers
2024-08-07  8:54       ` Thorsten Leemhuis
2024-08-14 16:28         ` James Clark
2024-08-14 16:41           ` Arnaldo Carvalho de Melo
2024-08-15 15:15             ` James Clark
2024-08-15 15:20               ` James Clark
2024-08-15 15:27               ` Arnaldo Carvalho de Melo
2024-08-15 15:53                 ` Arnaldo Carvalho de Melo
2024-08-16  8:57                   ` James Clark
2024-08-15 17:29           ` Ian Rogers
2024-08-16  9:22             ` James Clark
2024-08-16 15:30               ` Ian Rogers
2024-08-17  1:38                 ` Atish Kumar Patra
2024-08-20  8:58                   ` James Clark
2024-08-19 14:56                 ` James Clark
2024-08-19 15:44                   ` Ian Rogers
2025-03-09 21:19       ` 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=ZVzTJhMsgjPw2OE4@FVFF77S0Q05N.cambridge.arm.com \
    --to=mark.rutland@arm.com \
    --cc=acme@redhat.com \
    --cc=asahi@lists.linux.dev \
    --cc=irogers@google.com \
    --cc=james.clark@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=marcan@marcan.st \
    --cc=maz@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

all inboxes | Powered by JetHome®