[PATCH V3 1/2] powerpc/perf: Fix the power9 event alternatives array to have correct sort order

[PATCH V3 1/2] powerpc/perf: Fix the power9 event alternatives array to have correct sort order

[Athira Rajeev]

When scheduling a group of events, there are constraint checks
done to make sure all events can go in a group. Example, one of
the criteria is that events in a group cannot use same PMC.
But platform specific PMU supports alternative event for some
of the event codes. During perf_event_open, if any event
group doesn't match constraint check criteria, further lookup
is done to find alternative event.

By current design, the array of alternatives events in PMU
code is expected to be sorted by column 0. This is because in
find_alternative() function, the return criteria is based on
event code comparison. ie "event < ev_alt[i][0])". This
optimisation is there since find_alternative() can get called
multiple times. In power9 PMU code, the alternative event array
is not sorted list and hence there is breakage in finding
alternative event.

To work with existing logic, fix the alternative event array
to be sorted by column 0 for power9-pmu.c

Results:

With alternative events, multiplexing can be avoided. That is, 
for example, in power9 PM_LD_MISS_L1 (0x3e054) has alternative
event, PM_LD_MISS_L1_ALT (0x400f0). This is an identical event
which can be programmed in a different PMC.

<< Before patch >>

 # perf stat -e r3e054,r300fc
^C
 Performance counter stats for 'system wide':

           1057860      r3e054              (50.21%)
               379      r300fc              (49.79%)

       0.944329741 seconds time elapsed
       
Since both the events are using PMC3 in this case, they are
multiplexed here.

<<With patch>>

 # perf stat -e r3e054,r300fc
^C
 Performance counter stats for 'system wide':

           1006948      r3e054
               182      r300fc
<<>>

Fixes: 91e0bd1e6251 ("powerpc/perf: Add PM_LD_MISS_L1 and PM_BR_2PATH to power9 event list")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Changelog:
 v1 -> v2:
 Added Fixes tag and reworded commit message.
 Added Reviewed-by from Maddy.
 v2 -> v3:
 Added info about what is the breakage with current
 code.

 arch/powerpc/perf/power9-pmu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index c9eb5232e68b..c393e837648e 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -133,11 +133,11 @@ int p9_dd22_bl_ev[] = {
 
 /* Table of alternatives, sorted by column 0 */
 static const unsigned int power9_event_alternatives[][MAX_ALT] = {
-	{ PM_INST_DISP,			PM_INST_DISP_ALT },
-	{ PM_RUN_CYC_ALT,		PM_RUN_CYC },
-	{ PM_RUN_INST_CMPL_ALT,		PM_RUN_INST_CMPL },
-	{ PM_LD_MISS_L1,		PM_LD_MISS_L1_ALT },
 	{ PM_BR_2PATH,			PM_BR_2PATH_ALT },
+	{ PM_INST_DISP,			PM_INST_DISP_ALT },
+	{ PM_RUN_CYC_ALT,               PM_RUN_CYC },
+	{ PM_LD_MISS_L1,                PM_LD_MISS_L1_ALT },
+	{ PM_RUN_INST_CMPL_ALT,         PM_RUN_INST_CMPL },
 };
 
 static int power9_get_alternatives(u64 event, unsigned int flags, u64 alt[])
-- 
2.35.1

[PATCH V3 2/2] powerpc/perf: Fix the power10 event alternatives array to have correct sort order

[Athira Rajeev]

When scheduling a group of events, there are constraint checks
done to make sure all events can go in a group. Example, one of
the criteria is that events in a group cannot use same PMC.
But platform specific PMU supports alternative event for some
of the event codes. During perf_event_open, if any event
group doesn't match constraint check criteria, further lookup
is done to find alternative event.

By current design, the array of alternatives events in PMU 
code is expected to be sorted by column 0. This is because in
find_alternative() function, the return criteria is based on
event code comparison. ie "event < ev_alt[i][0])". This
optimisation is there since find_alternative() can get called
multiple times. In power10 PMU code, the alternative event array
is not sorted list and hence there is breakage in finding 
alternative event.

To work with existing logic, fix the alternative event array
to be sorted by column 0 for power10-pmu.c

Results:

In case where an alternative event is not chosen when we could,
events will be multiplexed. ie, time sliced where it could
actually run concurrently.
Example, in power10 PM_INST_CMPL_ALT(0x00002) has alternative
event, PM_INST_CMPL(0x500fa). Without the fix, if a group of
events with PMC1 to PMC4 is used along with PM_INST_CMPL_ALT,
it will be time sliced since all programmable PMC's are
consumed already. But with the fix, when it picks alternative
event on PMC5, all events will run concurrently.

<< Before Patch >>
 # perf stat -e r00002,r100fc,r200fa,r300fc,r400fc
^C
 Performance counter stats for 'system wide':

         328668935      r00002               (79.94%)
          56501024      r100fc               (79.95%)
          49564238      r200fa               (79.95%)
               376      r300fc               (80.19%)
               660      r400fc               (79.97%)

       4.039150522 seconds time elapsed

With the fix, since alternative event is chosen to run
on PMC6, events will be run concurrently.

<< After Patch >>
 # perf stat -e r00002,r100fc,r200fa,r300fc,r400fc
^C
 Performance counter stats for 'system wide':

          23596607      r00002
           4907738      r100fc
           2283608      r200fa
               135      r300fc
               248      r400fc

       1.664671390 seconds time elapsed

Fixes: a64e697cef23 ("powerpc/perf: power10 Performance Monitoring support")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Changelog:
 v1 -> v2:
 Added Fixes tag and reworded commit message
 Added Reviewed-by from Maddy
 v2 -> v3:
 Added info about what is the breakage with current 
 code.

 arch/powerpc/perf/power10-pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
index d3398100a60f..c6d51e7093cf 100644
--- a/arch/powerpc/perf/power10-pmu.c
+++ b/arch/powerpc/perf/power10-pmu.c
@@ -91,8 +91,8 @@ extern u64 PERF_REG_EXTENDED_MASK;
 
 /* Table of alternatives, sorted by column 0 */
 static const unsigned int power10_event_alternatives[][MAX_ALT] = {
-	{ PM_CYC_ALT,			PM_CYC },
 	{ PM_INST_CMPL_ALT,		PM_INST_CMPL },
+	{ PM_CYC_ALT,			PM_CYC },
 };
 
 static int power10_get_alternatives(u64 event, unsigned int flags, u64 alt[])
-- 
2.35.1

Re: [PATCH V3 1/2] powerpc/perf: Fix the power9 event alternatives array to have correct sort order

[Michael Ellerman]

On Tue, 19 Apr 2022 17:18:27 +0530, Athira Rajeev wrote:
> When scheduling a group of events, there are constraint checks
> done to make sure all events can go in a group. Example, one of
> the criteria is that events in a group cannot use same PMC.
> But platform specific PMU supports alternative event for some
> of the event codes. During perf_event_open, if any event
> group doesn't match constraint check criteria, further lookup
> is done to find alternative event.
> 
> [...]

Applied to powerpc/fixes.

[1/2] powerpc/perf: Fix the power9 event alternatives array to have correct sort order
      https://git.kernel.org/powerpc/c/0dcad700bb2776e3886fe0a645a4bf13b1e747cd
[2/2] powerpc/perf: Fix the power10 event alternatives array to have correct sort order
      https://git.kernel.org/powerpc/c/c6cc9a852f123301d5271f1484df8e961b2b64f1

cheers