* [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 ` [PATCH] " Scott Wood
0 siblings, 2 replies; 6+ 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] 6+ 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 ` [PATCH] " Scott Wood
1 sibling, 0 replies; 6+ 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] 6+ 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 10:45 ` [tip:perf/urgent] " tip-bot for Anton Blanchard
@ 2011-01-17 17:32 ` Scott Wood
2011-01-17 17:38 ` Peter Zijlstra
1 sibling, 1 reply; 6+ 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] 6+ messages in thread
* Re: [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters
2011-01-17 17:32 ` [PATCH] " Scott Wood
@ 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
0 siblings, 1 reply; 6+ 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] 6+ 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
2011-01-19 19:19 ` [tip:perf/urgent] powerpc, " tip-bot for Anton Blanchard
0 siblings, 1 reply; 6+ 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] 6+ messages in thread
* [tip:perf/urgent] powerpc, perf: Fix frequency calculation for overflowing counters (FSL version)
2011-01-18 10:44 ` [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters (FSL version) Anton Blanchard
@ 2011-01-19 19:19 ` tip-bot for Anton Blanchard
0 siblings, 0 replies; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2011-01-19 19:20 UTC | newest]
Thread overview: 6+ 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:38 ` Peter Zijlstra
2011-01-18 10:44 ` [PATCH] powerpc: perf: Fix frequency calculation for overflowing counters (FSL version) Anton Blanchard
2011-01-19 19:19 ` [tip:perf/urgent] powerpc, " tip-bot for Anton Blanchard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).