8-letter strings representing ARC perf events are stores in two
32-bit registers as ASCII characters like that: "IJMP", "IALL", "IJMPTAK" etc.
And the same order of bytes in the word is used regardless CPU endianness.
Which means in case of big-endian CPU core we need to swap bytes to get
the same order as if it was on little-endian CPU.
Otherwise we're seeing the following error message on boot:
------------------------->8----------------------
ARC perf : 8 counters (32 bits), 40 conditions, [overflow IRQ support]
sysfs: cannot create duplicate filename '/devices/arc_pct/events/pmji'
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.18 #3
Stack Trace:
arc_unwind_core+0xd4/0xfc
dump_stack+0x64/0x80
sysfs_warn_dup+0x46/0x58
sysfs_add_file_mode_ns+0xb2/0x168
create_files+0x70/0x2a0
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at kernel/events/core.c:12144 perf_event_sysfs_init+0x70/0xa0
Failed to register pmu: arc_pct, reason -17
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.18 #3
Stack Trace:
arc_unwind_core+0xd4/0xfc
dump_stack+0x64/0x80
__warn+0x9c/0xd4
warn_slowpath_fmt+0x22/0x2c
perf_event_sysfs_init+0x70/0xa0
---[ end trace a75fb9a9837bd1ec ]---
------------------------->8----------------------
What happens here we're trying to register more than one raw perf event
with the same name "PMJI". Why? Because ARC perf events are 4 to 8 letters
and encoded into two 32-bit words. In this particular case we deal with 2
events:
* "IJMP____" which counts all jump & branch instructions
* "IJMPC___" which counts only conditional jumps & branches
Those strings are split in two 32-bit words this way "IJMP" + "____" &
"IJMP" + "C___" correspondingly. Now if we read them swapped due to CPU core
being big-endian then we read "PMJI" + "____" & "PMJI" + "___C".
And since we interpret read array of ASCII letters as a null-terminated string
on big-endian CPU we end up with 2 events of the same name "PMJI".
Signed-off-by: Alexey Brodkin <[email protected]>
Cc: [email protected]
---
arch/arc/kernel/perf_event.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arc/kernel/perf_event.c b/arch/arc/kernel/perf_event.c
index 861a8aea51f9..661fd842ea97 100644
--- a/arch/arc/kernel/perf_event.c
+++ b/arch/arc/kernel/perf_event.c
@@ -614,8 +614,8 @@ static int arc_pmu_device_probe(struct platform_device *pdev)
/* loop thru all available h/w condition indexes */
for (i = 0; i < cc_bcr.c; i++) {
write_aux_reg(ARC_REG_CC_INDEX, i);
- cc_name.indiv.word0 = read_aux_reg(ARC_REG_CC_NAME0);
- cc_name.indiv.word1 = read_aux_reg(ARC_REG_CC_NAME1);
+ cc_name.indiv.word0 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME0));
+ cc_name.indiv.word1 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME1));
arc_pmu_map_hw_event(i, cc_name.str);
arc_pmu_add_raw_event_attr(i, cc_name.str);
--
2.16.2
Hi Sasha, Greg,
> -----Original Message-----
> From: Sasha Levin <[email protected]>
> Sent: Saturday, October 26, 2019 4:11 PM
> To: Sasha Levin <[email protected]>; Alexey Brodkin <[email protected]>; linux-snps-
> [email protected]
> Cc: [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH] ARC: perf: Accommodate big-endian CPU
>
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.3.7, v4.19.80, v4.14.150, v4.9.197, v4.4.197.
>
> v5.3.7: Build OK!
> v4.19.80: Failed to apply! Possible dependencies:
> 0e956150fe09f ("ARC: perf: introduce Kernel PMU events support")
> 14f81a91ad29a ("ARC: perf: trivial code cleanup")
> baf9cc85ba01f ("ARC: perf: move HW events mapping to separate function")
> v4.14.150: Failed to apply! Possible dependencies:
> v4.9.197: Failed to apply! Possible dependencies:
> v4.4.197: Failed to apply! Possible dependencies:
Indeed the clash is due to
commit baf9cc85ba01f ("ARC: perf: move HW events mapping to separate function") as tmp variable "j" was changed on "i". So that's a fixed hunk:
-------------------------------->8------------------------------
diff --git a/arch/arc/kernel/perf_event.c b/arch/arc/kernel/perf_event.c
index 8aec462d90fb..30f66b123541 100644
--- a/arch/arc/kernel/perf_event.c
+++ b/arch/arc/kernel/perf_event.c
@@ -490,8 +490,8 @@ static int arc_pmu_device_probe(struct platform_device *pdev)
/* loop thru all available h/w condition indexes */
for (j = 0; j < cc_bcr.c; j++) {
write_aux_reg(ARC_REG_CC_INDEX, j);
- cc_name.indiv.word0 = read_aux_reg(ARC_REG_CC_NAME0);
- cc_name.indiv.word1 = read_aux_reg(ARC_REG_CC_NAME1);
+ cc_name.indiv.word0 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME0));
+ cc_name.indiv.word1 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME1));
/* See if it has been mapped to a perf event_id */
for (i = 0; i < ARRAY_SIZE(arc_pmu_ev_hw_map); i++) {
-------------------------------->8------------------------------
Should I send a formal patch with it or it's OK for now?
-Alexey
On Tue, Nov 05, 2019 at 07:52:16PM +0000, Alexey Brodkin wrote:
> Hi Sasha, Greg,
>
> > -----Original Message-----
> > From: Sasha Levin <[email protected]>
> > Sent: Saturday, October 26, 2019 4:11 PM
> > To: Sasha Levin <[email protected]>; Alexey Brodkin <[email protected]>; linux-snps-
> > [email protected]
> > Cc: [email protected]; [email protected]; [email protected]
> > Subject: Re: [PATCH] ARC: perf: Accommodate big-endian CPU
> >
> > Hi,
> >
> > [This is an automated email]
> >
> > This commit has been processed because it contains a -stable tag.
> > The stable tag indicates that it's relevant for the following trees: all
> >
> > The bot has tested the following trees: v5.3.7, v4.19.80, v4.14.150, v4.9.197, v4.4.197.
> >
> > v5.3.7: Build OK!
> > v4.19.80: Failed to apply! Possible dependencies:
> > 0e956150fe09f ("ARC: perf: introduce Kernel PMU events support")
> > 14f81a91ad29a ("ARC: perf: trivial code cleanup")
> > baf9cc85ba01f ("ARC: perf: move HW events mapping to separate function")
> > v4.14.150: Failed to apply! Possible dependencies:
> > v4.9.197: Failed to apply! Possible dependencies:
> > v4.4.197: Failed to apply! Possible dependencies:
>
> Indeed the clash is due to
> commit baf9cc85ba01f ("ARC: perf: move HW events mapping to separate function") as tmp variable "j" was changed on "i". So that's a fixed hunk:
> -------------------------------->8------------------------------
> diff --git a/arch/arc/kernel/perf_event.c b/arch/arc/kernel/perf_event.c
> index 8aec462d90fb..30f66b123541 100644
> --- a/arch/arc/kernel/perf_event.c
> +++ b/arch/arc/kernel/perf_event.c
> @@ -490,8 +490,8 @@ static int arc_pmu_device_probe(struct platform_device *pdev)
> /* loop thru all available h/w condition indexes */
> for (j = 0; j < cc_bcr.c; j++) {
> write_aux_reg(ARC_REG_CC_INDEX, j);
> - cc_name.indiv.word0 = read_aux_reg(ARC_REG_CC_NAME0);
> - cc_name.indiv.word1 = read_aux_reg(ARC_REG_CC_NAME1);
> + cc_name.indiv.word0 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME0));
> + cc_name.indiv.word1 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME1));
>
> /* See if it has been mapped to a perf event_id */
> for (i = 0; i < ARRAY_SIZE(arc_pmu_ev_hw_map); i++) {
> -------------------------------->8------------------------------
>
> Should I send a formal patch with it or it's OK for now?
We need a "formal" patch that we can apply if you want it applied.
thanks,
greg k-h
Hi Greg,
> -----Original Message-----
> From: linux-snps-arc <[email protected]> On Behalf Of Greg Kroah-Hartman
> Sent: Thursday, November 21, 2019 11:40 PM
> To: Alexey Brodkin <[email protected]>
> Cc: Sasha Levin <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] ARC: perf: Accommodate big-endian CPU
>
> On Tue, Nov 05, 2019 at 07:52:16PM +0000, Alexey Brodkin wrote:
> > Hi Sasha, Greg,
> >
> > > -----Original Message-----
> > > From: Sasha Levin <[email protected]>
> > > Sent: Saturday, October 26, 2019 4:11 PM
> > > To: Sasha Levin <[email protected]>; Alexey Brodkin <[email protected]>; linux-snps-
> > > [email protected]
> > > Cc: [email protected]; [email protected]; [email protected]
> > > Subject: Re: [PATCH] ARC: perf: Accommodate big-endian CPU
> > >
> > > Hi,
> > >
> > > [This is an automated email]
> > >
> > > This commit has been processed because it contains a -stable tag.
> > > The stable tag indicates that it's relevant for the following trees: all
> > >
> > > The bot has tested the following trees: v5.3.7, v4.19.80, v4.14.150, v4.9.197, v4.4.197.
> > >
> > > v5.3.7: Build OK!
> > > v4.19.80: Failed to apply! Possible dependencies:
> > > 0e956150fe09f ("ARC: perf: introduce Kernel PMU events support")
> > > 14f81a91ad29a ("ARC: perf: trivial code cleanup")
> > > baf9cc85ba01f ("ARC: perf: move HW events mapping to separate function")
> > > v4.14.150: Failed to apply! Possible dependencies:
> > > v4.9.197: Failed to apply! Possible dependencies:
> > > v4.4.197: Failed to apply! Possible dependencies:
> >
> > Indeed the clash is due to
> > commit baf9cc85ba01f ("ARC: perf: move HW events mapping to separate function") as tmp variable "j"
> was changed on "i". So that's a fixed hunk:
> > -------------------------------->8------------------------------
> > diff --git a/arch/arc/kernel/perf_event.c b/arch/arc/kernel/perf_event.c
> > index 8aec462d90fb..30f66b123541 100644
> > --- a/arch/arc/kernel/perf_event.c
> > +++ b/arch/arc/kernel/perf_event.c
> > @@ -490,8 +490,8 @@ static int arc_pmu_device_probe(struct platform_device *pdev)
> > /* loop thru all available h/w condition indexes */
> > for (j = 0; j < cc_bcr.c; j++) {
> > write_aux_reg(ARC_REG_CC_INDEX, j);
> > - cc_name.indiv.word0 = read_aux_reg(ARC_REG_CC_NAME0);
> > - cc_name.indiv.word1 = read_aux_reg(ARC_REG_CC_NAME1);
> > + cc_name.indiv.word0 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME0));
> > + cc_name.indiv.word1 = le32_to_cpu(read_aux_reg(ARC_REG_CC_NAME1));
> >
> > /* See if it has been mapped to a perf event_id */
> > for (i = 0; i < ARRAY_SIZE(arc_pmu_ev_hw_map); i++) {
> > -------------------------------->8------------------------------
> >
> > Should I send a formal patch with it or it's OK for now?
>
> We need a "formal" patch that we can apply if you want it applied.
Done, see https://patchwork.ozlabs.org/patch/1201398/ and your inbox.
-Alexey