2013-03-08 23:24:34

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] perf, x86: Allow setting period 1

From: Andi Kleen <[email protected]>

I had some requests for setting period 1, so that every event of something
is caught. To my knowledge there is no limit to 1 on Intel hardware.
Just remove the check for minimum 2

If specific CPUs have problems we can black list them.

Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86/kernel/cpu/perf_event.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index bf0f01a..2b394ae 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -946,11 +946,6 @@ int x86_perf_event_set_period(struct perf_event *event)
hwc->last_period = period;
ret = 1;
}
- /*
- * Quirk: certain CPUs dont like it if just 1 hw_event is left:
- */
- if (unlikely(left < 2))
- left = 2;

if (left > x86_pmu.max_period)
left = x86_pmu.max_period;
--
1.7.7.6


2013-03-28 15:44:43

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] perf, x86: Allow setting period 1

Andi Kleen <[email protected]> writes:

> From: Andi Kleen <[email protected]>
>
> I had some requests for setting period 1, so that every event of something
> is caught. To my knowledge there is no limit to 1 on Intel hardware.
> Just remove the check for minimum 2

Ping! patch is missing review.

>
> If specific CPUs have problems we can black list them.
>
> Signed-off-by: Andi Kleen <[email protected]>
> ---
> arch/x86/kernel/cpu/perf_event.c | 5 -----
> 1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index bf0f01a..2b394ae 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -946,11 +946,6 @@ int x86_perf_event_set_period(struct perf_event *event)
> hwc->last_period = period;
> ret = 1;
> }
> - /*
> - * Quirk: certain CPUs dont like it if just 1 hw_event is left:
> - */
> - if (unlikely(left < 2))
> - left = 2;
>
> if (left > x86_pmu.max_period)
> left = x86_pmu.max_period;

--
[email protected] -- Speaking for myself only

2013-04-10 12:58:14

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf, x86: Allow setting period 1


* Andi Kleen <[email protected]> wrote:

> From: Andi Kleen <[email protected]>
>
> I had some requests for setting period 1, so that every event of something
> is caught. To my knowledge there is no limit to 1 on Intel hardware.
> Just remove the check for minimum 2
>
> If specific CPUs have problems we can black list them.

How have you tested this? The commit that added this quirk mentions very high perf
load triggering badness unless this quirk is added.

Thanks,

Ingo

2013-04-10 13:32:17

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] perf, x86: Allow setting period 1

On Wed, Apr 10, 2013 at 02:58:08PM +0200, Ingo Molnar wrote:
>
> * Andi Kleen <[email protected]> wrote:
>
> > From: Andi Kleen <[email protected]>
> >
> > I had some requests for setting period 1, so that every event of something
> > is caught. To my knowledge there is no limit to 1 on Intel hardware.
> > Just remove the check for minimum 2
> >
> > If specific CPUs have problems we can black list them.
>
> How have you tested this? The commit that added this quirk mentions very high perf
> load triggering badness unless this quirk is added.

Profiling a couple of simple loads on Westmere and IvyBridge: mostly AIM7 and
kernel builds. You can get throttling of course, but no excessive load.

The original quirk was done long ago before the modern event throttling
infrastructure may have been completely in place, right?

-Andi

--
[email protected] -- Speaking for myself only

2013-04-15 11:27:01

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf, x86: Allow setting period 1


* Andi Kleen <[email protected]> wrote:

> On Wed, Apr 10, 2013 at 02:58:08PM +0200, Ingo Molnar wrote:
> >
> > * Andi Kleen <[email protected]> wrote:
> >
> > > From: Andi Kleen <[email protected]>
> > >
> > > I had some requests for setting period 1, so that every event of something
> > > is caught. To my knowledge there is no limit to 1 on Intel hardware.
> > > Just remove the check for minimum 2
> > >
> > > If specific CPUs have problems we can black list them.
> >
> > How have you tested this? The commit that added this quirk mentions very high perf
> > load triggering badness unless this quirk is added.
>
> Profiling a couple of simple loads on Westmere and IvyBridge: mostly AIM7 and
> kernel builds. You can get throttling of course, but no excessive load.
>
> The original quirk was done long ago before the modern event throttling
> infrastructure may have been completely in place, right?

The failure IIRC wasn't about throttling or not, it was about extreme profiling
(lots of copies of perf record, perf top, perf stat running in parallel, mixed -a
and workload-local options), eventually resulting in a messed up PMU.

So before we can remove that a similar test should be repeated and made sure that
no badness happens, on a wide enough range of systems.

Thanks,

Ingo