public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: James Clark <james.clark@arm.com>, Peter Zijlstra <peterz@infradead.org>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	Marc Zyngier <maz@kernel.org>,
	Suzuki Poulose <suzuki.poulose@arm.com>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-perf-users@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, will@kernel.org,
	catalin.marinas@arm.com
Subject: Re: [PATCH V17 0/9] arm64/perf: Enable branch stack sampling
Date: Fri, 31 May 2024 14:01:08 +0100	[thread overview]
Message-ID: <ZlnKFFwv9612V81h@J2N7QTR9R3> (raw)
In-Reply-To: <Zli6Ot0TwK3Qy-ee@J2N7QTR9R3>

On Thu, May 30, 2024 at 06:41:14PM +0100, Mark Rutland wrote:
> On Thu, May 30, 2024 at 10:47:34AM +0100, James Clark wrote:
> > On 05/04/2024 03:46, Anshuman Khandual wrote:
> > > ------------------ Possible 'branch_sample_type' Mismatch -----------------
> > > 
> > > Branch stack sampling attributes 'event->attr.branch_sample_type' generally
> > > remain the same for all the events during a perf record session.
> > > 
> > > $perf record -e <event_1> -e <event_2> -j <branch_filters> [workload]
> > > 
> > > event_1->attr.branch_sample_type == event_2->attr.branch_sample_type
> > > 
> > > This 'branch_sample_type' is used to configure the BRBE hardware, when both
> > > events i.e <event_1> and <event_2> get scheduled on a given PMU. But during
> > > PMU HW event's privilege filter inheritance, 'branch_sample_type' does not
> > > remain the same for all events. Let's consider the following example
> > > 
> > > $perf record -e cycles:u -e instructions:k -j any,save_type ls
> > > 
> > > cycles->attr.branch_sample_type != instructions->attr.branch_sample_type
> > > 
> > > Because cycles event inherits PERF_SAMPLE_BRANCH_USER and instruction event
> > > inherits PERF_SAMPLE_BRANCH_KERNEL. The proposed solution here configures
> > > BRBE hardware with 'branch_sample_type' from last event to be added in the
> > > PMU and hence captured branch records only get passed on to matching events
> > > during a PMU interrupt.
> > > 
> > 
> > Hi Anshuman,
> > 
> > Surely because of this example we should merge? At least we have to try
> > to make the most common basic command lines work. Unless we expect all
> > tools to know whether the branch buffer is shared between PMUs on each
> > architecture or not. The driver knows though, so can merge the settings
> > because it all has to go into one BRBE.
> 
> The difficulty here is that these are opened as independent events (not
> in the same event group), and so from the driver's PoV, this is no
> different two two users independently doing:
> 
> 	perf record -e event:u -j any,save_type -p ${SOME_PID}
> 
> 	perf record -e event:k -j any,save_type -p ${SOME_PID}
> 
> .. where either would be surprised to get the merged result.

I took a look at how x86 handles this, and it looks like they may have the
problem we'd like to avoid. AFAICT, intel_pmu_lbr_add() blats cpuc->br_sel with
the branch selection of the last event added, and 

So I took a look at what happens on my x86-64 desktop running v5.10.0-9-amd64
from Debian 11.

Running the following program:

| int main (int argc, char *argv[])
| {
|         for (;;) {
|                 asm volatile("" ::: "memory");
|         }
| 
|         return 0;
| }	

I set /proc/sys/kernel/perf_event_paranoid to 2 and started two independent
perf sessions:

	perf record -e cycles:u -j any -o perf-user.data -p 1320224

	sudo perf record -e cycles:k -j any -o perf-kernel.data -p 1320224

... after ~10 seconds, I killed both sessions with ^C.

When i susbsequently do 'perf report -i perf-kernel.data, I see:

| Samples: 295  of event 'cycles:k', Event count (approx.): 295
| Overhead  Command  Source Shared Object  Source Symbol               Target Symbol  Basic Block Cycles
|   99.66%  loop     loop                  [k] main                    [k] main       -
|    0.34%  loop     [kernel.kallsyms]     [k] native_irq_return_iret  [k] main       -

... where the user symbols are surprising.

Similarly for 'perf report -i perf-user.data', I see:

| Samples: 198K of event 'cycles:u', Event count (approx.): 198739
| Overhead  Command  Source Shared Object  Source Symbol           Target Symbol           Basic Block Cycles
|   99.99%  loop     loop                  [.] main                [.] main                -
|    0.00%  loop     [unknown]             [.] 0xffffffff87801007  [.] main                -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e05626  [.] 0xffffffff86e05629  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e0563d  [.] 0xffffffff86e0c850  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e0c86f  [.] 0xffffffff86e6b3f0  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e0c884  [.] 0xffffffff86e11ed0  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e0c88a  [.] 0xffffffff86e13850  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e11eee  [.] 0xffffffff86e0c889  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e13885  [.] 0xffffffff86e13888  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e13889  [.] 0xffffffff86e138a1  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e138a9  [.] 0xffffffff86e6b320  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e138c3  [.] 0xffffffff86e6b3f0  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e6b33a  [.] 0xffffffff86e138ae  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86e6b3fb  [.] 0xffffffff86e0c874  -
|    0.00%  loop     [unknown]             [.] 0xffffffff86ff6c91  [.] 0xffffffff87a01ca0  -
|    0.00%  loop     [unknown]             [.] 0xffffffff87a01ca0  [.] 0xffffffff87a01ca5  -
|    0.00%  loop     [unknown]             [.] 0xffffffff87a01ca5  [.] 0xffffffff87a01cb1  -
|    0.00%  loop     [unknown]             [.] 0xffffffff87a01cb5  [.] 0xffffffff86e05600  -

Where the unknown (kernel!) samples are surprising.

Peter, do you have any opinion on this?

My thinking is that the "last scheduled event branch selection wins"
isn't the behaviour we actually want, and either:

(a) Conflicting events shouldn't be scheduled concurrently (e.g. treat
    that like running out of counters).

(b) The HW filters should be configured to allow anything permited by
    any of the events, and SW filtering should remove the unexpected
    records on a per-event basis.

... but I imagine (b) is hard maybe? I don't know if LBR tells you which
CPU mode the src/dst were in.

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-05-31 13:01 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-05  2:46 [PATCH V17 0/9] arm64/perf: Enable branch stack sampling Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 1/9] arm64/sysreg: Add BRBE registers and fields Anshuman Khandual
2024-05-21 13:24   ` Mark Rutland
2024-06-03  5:12     ` Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 2/9] KVM: arm64: Explicitly handle BRBE traps as UNDEFINED Anshuman Khandual
2024-05-21 13:26   ` Mark Rutland
2024-06-03  5:31     ` Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 3/9] drivers: perf: arm_pmu: Add infrastructure for branch stack sampling Anshuman Khandual
2024-05-21 13:44   ` Mark Rutland
2024-06-03  6:40     ` Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 4/9] arm64/boot: Enable EL2 requirements for BRBE Anshuman Khandual
2024-05-29 10:51   ` Mark Rutland
2024-06-03  9:11     ` Anshuman Khandual
2024-06-03  9:38       ` Mark Rutland
2024-04-05  2:46 ` [PATCH V17 5/9] drivers: perf: arm_pmuv3: Enable branch stack sampling via FEAT_BRBE Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 6/9] KVM: arm64: nvhe: Disable branch generation in nVHE guests Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 7/9] perf: test: Speed up running brstack test on an Arm model Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 8/9] perf: test: Remove empty lines from branch filter test output Anshuman Khandual
2024-04-05  2:46 ` [PATCH V17 9/9] perf: test: Extend branch stack sampling test for Arm64 BRBE Anshuman Khandual
2024-04-05  3:54 ` [PATCH V17 0/9] arm64/perf: Enable branch stack sampling Adam Young
2024-04-08  2:26   ` Anshuman Khandual
2024-05-30  9:47 ` James Clark
2024-05-30 17:41   ` Mark Rutland
2024-05-31 13:01     ` Mark Rutland [this message]
2024-06-06  4:58     ` Anshuman Khandual
2024-06-06  6:27       ` Anshuman Khandual
2024-06-06 11:01       ` James Clark
2024-06-06  3:57   ` Anshuman Khandual
2024-05-30 10:03 ` James Clark
2024-06-03  9:18   ` Anshuman Khandual
2024-06-03  9:39     ` Mark Rutland

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=ZlnKFFwv9612V81h@J2N7QTR9R3 \
    --to=mark.rutland@arm.com \
    --cc=acme@kernel.org \
    --cc=anshuman.khandual@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=james.clark@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=robh@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@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