[PATCH] perf, x86: Allow setting period 1

[PATCH] perf, x86: Allow setting period 1

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

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 <ak@linux.intel.com>
---
 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

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

[Andi Kleen]

Andi Kleen <andi@firstfloor.org> writes:

> From: Andi Kleen <ak@linux.intel.com>
>
> 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 <ak@linux.intel.com>
> ---
>  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;

-- 
ak@linux.intel.com -- Speaking for myself only

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

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> From: Andi Kleen <ak@linux.intel.com>
> 
> 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

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

[Andi Kleen]

On Wed, Apr 10, 2013 at 02:58:08PM +0200, Ingo Molnar wrote:
> 
> * Andi Kleen <andi@firstfloor.org> wrote:
> 
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > 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

-- 
ak@linux.intel.com -- Speaking for myself only

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

[Ingo Molnar]

* Andi Kleen <ak@linux.intel.com> wrote:

> On Wed, Apr 10, 2013 at 02:58:08PM +0200, Ingo Molnar wrote:
> > 
> > * Andi Kleen <andi@firstfloor.org> wrote:
> > 
> > > From: Andi Kleen <ak@linux.intel.com>
> > > 
> > > 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