* [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters
@ 2011-01-17 5:17 Anton Blanchard
2011-01-17 10:45 ` [tip:perf/urgent] " tip-bot for Anton Blanchard
2011-01-17 17:32 ` Scott Wood
0 siblings, 2 replies; 9+ messages in thread
From: Anton Blanchard @ 2011-01-17 5:17 UTC (permalink / raw)
To: Paul Mackerras, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-kernel
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 <anton@samba.org>
---
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;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [tip:perf/urgent] powerpc: perf: Fix frequency calculation for overflowing counters
2011-01-17 5:17 [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters Anton Blanchard
@ 2011-01-17 10:45 ` tip-bot for Anton Blanchard
2011-01-17 17:32 ` Scott Wood
1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Anton Blanchard @ 2011-01-17 10:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, anton, hpa, mingo, a.p.zijlstra, acme,
benh, tglx, mingo
Commit-ID: 4bca770ede796a1ef7af9c983166d5608d9ccfaf
Gitweb: http://git.kernel.org/tip/4bca770ede796a1ef7af9c983166d5608d9ccfaf
Author: Anton Blanchard <anton@samba.org>
AuthorDate: Mon, 17 Jan 2011 16:17:42 +1100
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 17 Jan 2011 11:43:02 +0100
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 <anton@samba.org>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
LKML-Reference: <20110117161742.5feb3761@kryten>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/powerpc/kernel/perf_event.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
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;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters
2011-01-17 5:17 [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters Anton Blanchard
@ 2011-01-17 17:32 ` Scott Wood
2011-01-17 17:32 ` Scott Wood
1 sibling, 0 replies; 9+ messages in thread
From: Scott Wood @ 2011-01-17 17:32 UTC (permalink / raw)
To: Anton Blanchard
Cc: Paul Mackerras, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Benjamin Herrenschmidt, linuxppc-dev,
linux-kernel
On Mon, 17 Jan 2011 16:17:42 +1100
Anton Blanchard <anton@samba.org> wrote:
>
> 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 <anton@samba.org>
> ---
>
> 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;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Does perf_event_fsl_emb.c need this as well (it has almost the same
record_and_restart code)?
-Scott
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters
@ 2011-01-17 17:32 ` Scott Wood
0 siblings, 0 replies; 9+ messages in thread
From: Scott Wood @ 2011-01-17 17:32 UTC (permalink / raw)
To: Anton Blanchard
Cc: Peter Zijlstra, linux-kernel, Paul Mackerras,
Arnaldo Carvalho de Melo, Ingo Molnar, linuxppc-dev
On Mon, 17 Jan 2011 16:17:42 +1100
Anton Blanchard <anton@samba.org> wrote:
>
> 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 <anton@samba.org>
> ---
>
> 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;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Does perf_event_fsl_emb.c need this as well (it has almost the same
record_and_restart code)?
-Scott
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters
2011-01-17 17:32 ` Scott Wood
@ 2011-01-17 17:38 ` Peter Zijlstra
-1 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2011-01-17 17:38 UTC (permalink / raw)
To: Scott Wood
Cc: Anton Blanchard, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo, Benjamin Herrenschmidt, linuxppc-dev,
linux-kernel
On Mon, 2011-01-17 at 11:32 -0600, Scott Wood wrote:
> > 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;
> Does perf_event_fsl_emb.c need this as well (it has almost the same
> record_and_restart code)?
I would think so.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters
@ 2011-01-17 17:38 ` Peter Zijlstra
0 siblings, 0 replies; 9+ messages in thread
From: Peter Zijlstra @ 2011-01-17 17:38 UTC (permalink / raw)
To: Scott Wood
Cc: linux-kernel, Paul Mackerras, Anton Blanchard,
Arnaldo Carvalho de Melo, Ingo Molnar, linuxppc-dev
On Mon, 2011-01-17 at 11:32 -0600, Scott Wood wrote:
> > diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/per=
f_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 <=3D 0)
> > left =3D period;
> > record =3D 1;
> > + event->hw.last_period =3D event->hw.sample_period=
;
> > }
> > 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters (FSL version)
2011-01-17 17:38 ` Peter Zijlstra
@ 2011-01-18 10:44 ` Anton Blanchard
-1 siblings, 0 replies; 9+ messages in thread
From: Anton Blanchard @ 2011-01-18 10:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Scott Wood, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo, Benjamin Herrenschmidt, linuxppc-dev,
linux-kernel
> > 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 <anton@samba.org>
---
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;
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters (FSL version)
@ 2011-01-18 10:44 ` Anton Blanchard
0 siblings, 0 replies; 9+ messages in thread
From: Anton Blanchard @ 2011-01-18 10:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Paul Mackerras, Arnaldo Carvalho de Melo,
Scott Wood, Ingo Molnar, linuxppc-dev
> > 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 <anton@samba.org>
---
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;
^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip:perf/urgent] powerpc, perf: Fix frequency calculation for overflowing counters (FSL version)
2011-01-18 10:44 ` Anton Blanchard
(?)
@ 2011-01-19 19:19 ` tip-bot for Anton Blanchard
-1 siblings, 0 replies; 9+ messages in thread
From: tip-bot for Anton Blanchard @ 2011-01-19 19:19 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, anton, hpa, mingo, a.p.zijlstra, acme,
scottwood, benh, tglx, mingo
Commit-ID: 8c8a9b25b5de3f1eeac721cf34f4379e56d5d694
Gitweb: http://git.kernel.org/tip/8c8a9b25b5de3f1eeac721cf34f4379e56d5d694
Author: Anton Blanchard <anton@samba.org>
AuthorDate: Tue, 18 Jan 2011 21:44:04 +1100
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 19 Jan 2011 20:05:42 +0100
powerpc, perf: Fix frequency calculation for overflowing counters (FSL version)
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 <anton@samba.org>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110118214404.2f42e634@kryten>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/powerpc/kernel/perf_event_fsl_emb.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/perf_event_fsl_emb.c b/arch/powerpc/kernel/perf_event_fsl_emb.c
index 4dcf5f8..b0dc8f7 100644
--- a/arch/powerpc/kernel/perf_event_fsl_emb.c
+++ b/arch/powerpc/kernel/perf_event_fsl_emb.c
@@ -596,6 +596,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;
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-01-19 19:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-17 5:17 [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters Anton Blanchard
2011-01-17 10:45 ` [tip:perf/urgent] " tip-bot for Anton Blanchard
2011-01-17 17:32 ` [PATCH] " Scott Wood
2011-01-17 17:32 ` Scott Wood
2011-01-17 17:38 ` Peter Zijlstra
2011-01-17 17:38 ` Peter Zijlstra
2011-01-18 10:44 ` [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters (FSL version) Anton Blanchard
2011-01-18 10:44 ` Anton Blanchard
2011-01-19 19:19 ` [tip:perf/urgent] powerpc, " tip-bot for Anton Blanchard
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.