[PATCH] powerpc: perf: Fix frequency calculation for overflowing counters

When profiling a benchmark that is almost 100% userspace, I noticed some
wildly inaccurate profiles that showed almost all time spent in the kernel.
Closer examination shows we were programming a tiny number of cycles into
the PMU after each overflow (about ~200 away from the next overflow). This
gets us stuck in a loop which we eventually break out of by throttling the
PMU (there are regular throttle/unthrottle events in the log).

It looks like we aren't setting event->hw.last_period to something same
and the frequency to period calculations in perf are going haywire. With
the following patch we find the correct period after a few interrupts and
stay there. I also see no more throttle events.

Signed-off-by: Anton Blanchard <redacted>
---

diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/perf_event.c
index 5674807..ab6f6be 100644
--- a/arch/powerpc/kernel/perf_event.c
+++ b/arch/powerpc/kernel/perf_event.c

@@ -1212,6 +1212,7 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
 			if (left <= 0)
 				left = period;
 			record = 1;
+			event->hw.last_period = event->hw.sample_period;
 		}
 		if (left < 0x80000000LL)
 			val = 0x80000000LL - left;

On Mon, 17 Jan 2011 16:17:42 +1100
Anton Blanchard [off-list ref] wrote:

Does perf_event_fsl_emb.c need this as well (it has almost the same
record_and_restart code)?

-Scott

On Mon, 2011-01-17 at 11:32 -0600, Scott Wood wrote:

quoted

diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/per=

f_event.c

quoted

index 5674807..ab6f6be 100644

--- a/arch/powerpc/kernel/perf_event.c
+++ b/arch/powerpc/kernel/perf_event.c

@@ -1212,6 +1212,7 @@ static void record_and_restart(struct perf_event =

*event, unsigned long val,

quoted

                      if (left <=3D 0)
                              left =3D period;
                      record =3D 1;
+                     event->hw.last_period =3D event->hw.sample_period=

;

quoted

              }
              if (left < 0x80000000LL)
                      val =3D 0x80000000LL - left;

Does perf_event_fsl_emb.c need this as well (it has almost the same
record_and_restart code)?=20

I would think so.

quoted

Does perf_event_fsl_emb.c need this as well (it has almost the same
record_and_restart code)? 

I would think so.

Good point:


When fixing the frequency calculations for perf on powerpc I forgot
to fix the FSL version.

If we dont set event->hw.last_period the frequency to period calculations
in perf go haywire and we continually throttle/unthrottle the PMU.

Signed-off-by: Anton Blanchard <redacted>
---

This is only compile tested.

Index: powerpc.git/arch/powerpc/kernel/perf_event_fsl_emb.c
===================================================================

--- powerpc.git.orig/arch/powerpc/kernel/perf_event_fsl_emb.c	2011-01-18 21:23:57.761644115 +1100
+++ powerpc.git/arch/powerpc/kernel/perf_event_fsl_emb.c	2011-01-18 21:25:28.994477247 +1100

@@ -596,6 +596,7 @@ static void record_and_restart(struct pe
 			if (left <= 0)
 				left = period;
 			record = 1;
+			event->hw.last_period = event->hw.sample_period;
 		}
 		if (left < 0x80000000LL)
 			val = 0x80000000LL - left;