From: Anshuman Khandual <anshuman.khandual@arm.com> To: Mark Brown <broonie@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, peterz@infradead.org, acme@kernel.org, mark.rutland@arm.com, will@kernel.org, catalin.marinas@arm.com, James Clark <james.clark@arm.com>, Rob Herring <robh@kernel.org>, Marc Zyngier <maz@kernel.org>, Ingo Molnar <mingo@redhat.com> Subject: Re: [PATCH V2 3/7] arm64/perf: Update struct pmu_hw_events for BRBE Date: Tue, 13 Sep 2022 11:03:45 +0530 [thread overview] Message-ID: <84683aa7-58ad-85f8-327b-daed2f704834@arm.com> (raw) In-Reply-To: <Yx8GBq1FJN49iJs9@sirena.org.uk> On 9/12/22 15:42, Mark Brown wrote: > On Thu, Sep 08, 2022 at 10:40:42AM +0530, Anshuman Khandual wrote: > >> + /* Captured BRBE buffer - copied as is into perf_sample_data */ >> + struct perf_branch_stack brbe_stack; >> + struct perf_branch_entry brbe_entries[BRBE_MAX_ENTRIES]; > > It looks like perf_branch_entry is intended to be the variably > sized entries array at the end of perf_branch_stack? That could That is right. Because max number of entries for brbe_entries[] array is platform dependent i.e BHRB_MAX_ENTRIES on powerpc, MAX_LBR_ENTRIES on x86 and BRBE_MAX_ENTRIES on arm64. The generic definition struct perf_branch_stack { __u64 nr; __u64 hw_idx; struct perf_branch_entry entries[]; }; On x86 platform #define MAX_LBR_ENTRIES 32 struct cpu_hw_events { .... struct perf_branch_stack lbr_stack; struct perf_branch_entry lbr_entries[MAX_LBR_ENTRIES]; .... } On powerpc platform #define BHRB_MAX_ENTRIES 32 struct cpu_hw_events { .... struct perf_branch_stack bhrb_stack; struct perf_branch_entry bhrb_entries[BHRB_MAX_ENTRIES]; .... } Followed same format on arm64 platform as well #define BRBE_MAX_ENTRIES 64 struct pmu_hw_events { .... .... struct perf_branch_stack brbe_stack; struct perf_branch_entry brbe_entries[BRBE_MAX_ENTRIES]; .... .... } > probably do with being called out if it's the case. It feels Right, we could add a comment in this regard. > like it would be clearer and safer to allocate these dynamically > when BRBE is used if that's possible, I'd expect that should also > deal with the stack frame size issues as well. That might not be possible because the generic 'struct perf_branch_stack' expects 'perf_branch_stack.entries' to be a variable array which is also contiguous in memory, with other elements in 'perf_branch_stack'. Besides that will be a deviation from similar implementations on x86 and powerpc platforms. The stack frame size came up because BRBE_MAX_ENTRIES is 64 compared to just 32 on other platforms, which follow the exact same method.
WARNING: multiple messages have this Message-ID (diff)
From: Anshuman Khandual <anshuman.khandual@arm.com> To: Mark Brown <broonie@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-arm-kernel@lists.infradead.org, peterz@infradead.org, acme@kernel.org, mark.rutland@arm.com, will@kernel.org, catalin.marinas@arm.com, James Clark <james.clark@arm.com>, Rob Herring <robh@kernel.org>, Marc Zyngier <maz@kernel.org>, Ingo Molnar <mingo@redhat.com> Subject: Re: [PATCH V2 3/7] arm64/perf: Update struct pmu_hw_events for BRBE Date: Tue, 13 Sep 2022 11:03:45 +0530 [thread overview] Message-ID: <84683aa7-58ad-85f8-327b-daed2f704834@arm.com> (raw) In-Reply-To: <Yx8GBq1FJN49iJs9@sirena.org.uk> On 9/12/22 15:42, Mark Brown wrote: > On Thu, Sep 08, 2022 at 10:40:42AM +0530, Anshuman Khandual wrote: > >> + /* Captured BRBE buffer - copied as is into perf_sample_data */ >> + struct perf_branch_stack brbe_stack; >> + struct perf_branch_entry brbe_entries[BRBE_MAX_ENTRIES]; > > It looks like perf_branch_entry is intended to be the variably > sized entries array at the end of perf_branch_stack? That could That is right. Because max number of entries for brbe_entries[] array is platform dependent i.e BHRB_MAX_ENTRIES on powerpc, MAX_LBR_ENTRIES on x86 and BRBE_MAX_ENTRIES on arm64. The generic definition struct perf_branch_stack { __u64 nr; __u64 hw_idx; struct perf_branch_entry entries[]; }; On x86 platform #define MAX_LBR_ENTRIES 32 struct cpu_hw_events { .... struct perf_branch_stack lbr_stack; struct perf_branch_entry lbr_entries[MAX_LBR_ENTRIES]; .... } On powerpc platform #define BHRB_MAX_ENTRIES 32 struct cpu_hw_events { .... struct perf_branch_stack bhrb_stack; struct perf_branch_entry bhrb_entries[BHRB_MAX_ENTRIES]; .... } Followed same format on arm64 platform as well #define BRBE_MAX_ENTRIES 64 struct pmu_hw_events { .... .... struct perf_branch_stack brbe_stack; struct perf_branch_entry brbe_entries[BRBE_MAX_ENTRIES]; .... .... } > probably do with being called out if it's the case. It feels Right, we could add a comment in this regard. > like it would be clearer and safer to allocate these dynamically > when BRBE is used if that's possible, I'd expect that should also > deal with the stack frame size issues as well. That might not be possible because the generic 'struct perf_branch_stack' expects 'perf_branch_stack.entries' to be a variable array which is also contiguous in memory, with other elements in 'perf_branch_stack'. Besides that will be a deviation from similar implementations on x86 and powerpc platforms. The stack frame size came up because BRBE_MAX_ENTRIES is 64 compared to just 32 on other platforms, which follow the exact same method. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-09-13 5:34 UTC|newest] Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-09-08 5:10 [PATCH V2 0/7] arm64/perf: Enable branch stack sampling Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-08 5:10 ` [PATCH V2 1/7] arm64/perf: Add register definitions for BRBE Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-12 9:57 ` Mark Brown 2022-09-12 9:57 ` Mark Brown 2022-09-13 6:24 ` Anshuman Khandual 2022-09-13 6:24 ` Anshuman Khandual 2022-09-13 11:30 ` Mark Brown 2022-09-13 11:30 ` Mark Brown 2022-09-08 5:10 ` [PATCH V2 2/7] arm64/perf: Update struct arm_pmu " Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-08 5:10 ` [PATCH V2 3/7] arm64/perf: Update struct pmu_hw_events " Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-08 12:32 ` kernel test robot 2022-09-09 3:11 ` Anshuman Khandual 2022-09-09 3:11 ` Anshuman Khandual 2022-09-09 3:11 ` Anshuman Khandual 2022-09-08 14:14 ` kernel test robot 2022-09-09 3:14 ` Anshuman Khandual 2022-09-09 3:14 ` Anshuman Khandual 2022-09-09 3:14 ` Anshuman Khandual 2022-09-12 10:12 ` Mark Brown 2022-09-12 10:12 ` Mark Brown 2022-09-13 5:33 ` Anshuman Khandual [this message] 2022-09-13 5:33 ` Anshuman Khandual 2022-09-13 11:43 ` Mark Brown 2022-09-13 11:43 ` Mark Brown 2022-09-14 3:39 ` Anshuman Khandual 2022-09-14 3:39 ` Anshuman Khandual 2022-09-14 9:35 ` Mark Brown 2022-09-14 9:35 ` Mark Brown 2022-09-08 5:10 ` [PATCH V2 4/7] driver/perf/arm_pmu_platform: Add support for BRBE attributes detection Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-08 5:10 ` [PATCH V2 5/7] arm64/perf: Drive BRBE from perf event states Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-08 15:31 ` kernel test robot 2022-09-08 15:31 ` kernel test robot 2022-09-08 5:10 ` [PATCH V2 6/7] arm64/perf: Add BRBE driver Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-08 9:23 ` kernel test robot 2022-09-08 9:23 ` kernel test robot 2022-09-08 10:16 ` Anshuman Khandual 2022-09-08 10:16 ` Anshuman Khandual 2022-09-08 10:16 ` Anshuman Khandual 2022-09-13 10:39 ` James Clark 2022-09-13 10:39 ` James Clark 2022-09-13 11:38 ` Anshuman Khandual 2022-09-13 11:38 ` Anshuman Khandual 2022-09-08 5:10 ` [PATCH V2 7/7] arm64/perf: Enable branch stack sampling Anshuman Khandual 2022-09-08 5:10 ` Anshuman Khandual 2022-09-13 10:55 ` [PATCH V2 0/7] " James Clark 2022-09-13 10:55 ` James Clark 2022-09-13 12:12 ` Anshuman Khandual 2022-09-13 12:12 ` Anshuman Khandual 2022-09-13 13:12 ` James Clark 2022-09-13 13:12 ` James Clark 2022-09-14 4:43 ` Anshuman Khandual 2022-09-14 4:43 ` Anshuman Khandual
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=84683aa7-58ad-85f8-327b-daed2f704834@arm.com \ --to=anshuman.khandual@arm.com \ --cc=acme@kernel.org \ --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=mark.rutland@arm.com \ --cc=maz@kernel.org \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=robh@kernel.org \ --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: linkBe 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.