perf: odd event scheduling issue

perf: odd event scheduling issue

[Vince Weaver]

Hello

so I'm running 4.1-rc4 on my Haswell machine and a lot of my perf_event 
testsuite is failing.  There seems to be weird event scheduling issues.

I can reproduce it with plain perf; simple event groups that should work 
fine can't be scheduled.

vince@haswell:~$ perf stat -e \{cycles,instructions\} sleep 1

 Performance counter stats for 'sleep 1':

     <not counted>      cycles                   
     <not counted>      instructions             

       1.003414661 seconds time elapsed

I just wanted to check to see if this was a known problem before I started 
bisecting.  I was also possibly seeing this with 4.1-rc2.

Vince

Re: perf: odd event scheduling issue

[Peter Zijlstra]

On Tue, May 19, 2015 at 11:07:09PM -0400, Vince Weaver wrote:
> Hello
> 
> so I'm running 4.1-rc4 on my Haswell machine and a lot of my perf_event 
> testsuite is failing.  There seems to be weird event scheduling issues.
> 
> I can reproduce it with plain perf; simple event groups that should work 
> fine can't be scheduled.
> 
> vince@haswell:~$ perf stat -e \{cycles,instructions\} sleep 1
> 

Hmm indeed, my ivb-ep does the same thing. Lemme go poke at that.

Re: perf: odd event scheduling issue

[Jiri Olsa]

On Wed, May 20, 2015 at 09:00:57AM +0200, Peter Zijlstra wrote:
> On Tue, May 19, 2015 at 11:07:09PM -0400, Vince Weaver wrote:
> > Hello
> > 
> > so I'm running 4.1-rc4 on my Haswell machine and a lot of my perf_event 
> > testsuite is failing.  There seems to be weird event scheduling issues.
> > 
> > I can reproduce it with plain perf; simple event groups that should work 
> > fine can't be scheduled.
> > 
> > vince@haswell:~$ perf stat -e \{cycles,instructions\} sleep 1
> > 
> 
> Hmm indeed, my ivb-ep does the same thing. Lemme go poke at that.

snb is ok

jirka

Re: perf: odd event scheduling issue

[Peter Zijlstra]

On Wed, May 20, 2015 at 09:00:57AM +0200, Peter Zijlstra wrote:
> On Tue, May 19, 2015 at 11:07:09PM -0400, Vince Weaver wrote:
> > Hello
> > 
> > so I'm running 4.1-rc4 on my Haswell machine and a lot of my perf_event 
> > testsuite is failing.  There seems to be weird event scheduling issues.
> > 
> > I can reproduce it with plain perf; simple event groups that should work 
> > fine can't be scheduled.
> > 
> > vince@haswell:~$ perf stat -e \{cycles,instructions\} sleep 1
> > 
> 
> Hmm indeed, my ivb-ep does the same thing. Lemme go poke at that.

I've gotta go pick the kid up from school, but the bisect just finished
and threw up the below commit.

I'll go stare at it in an hour or so.

---

c02cdbf60b51b8d98a49185535f5d527a2965142 is the first bad commit
commit c02cdbf60b51b8d98a49185535f5d527a2965142
Author: Stephane Eranian <eranian@google.com>
Date:   Mon Nov 17 20:07:02 2014 +0100

    perf/x86/intel: Limit to half counters when the HT workaround is enabled, to avoid exclusive mode starvation
    
    This patch limits the number of counters available to each CPU when
    the HT bug workaround is enabled.
    
    This is necessary to avoid situation of counter starvation. Such can
    arise from configuration where one HT thread, HT0, is using all 4 counters
    with corrupting events which require exclusion the the sibling HT, HT1.
    
    In such case, HT1 would not be able to schedule any event until HT0
    is done. To mitigate this problem, this patch artificially limits
    the number of counters to 2.
    
    That way, we can gurantee that at least 2 counters are not in exclusive
    mode and therefore allow the sibling thread to schedule events of the
    same type (system vs. per-thread). The 2 counters are not determined
    in advance. We simply set the limit to two events per HT.
    
    This helps mitigate starvation in case of events with specific counter
    constraints such a PREC_DIST.
    
    Note that this does not elimintate the starvation is all cases. But
    it is better than not having it.
    
    (Solution suggested by Peter Zjilstra.)
    
    Signed-off-by: Stephane Eranian <eranian@google.com>
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Cc: bp@alien8.de
    Cc: jolsa@redhat.com
    Cc: kan.liang@intel.com
    Cc: maria.n.dimakopoulou@gmail.com
    Link: http://lkml.kernel.org/r/1416251225-17721-11-git-send-email-eranian@google.com
    Signed-off-by: Ingo Molnar <mingo@kernel.org>

:040000 040000 d78b3927f7fb17a34079012f7184aeef6120e411 84a0237880834d5b58b357fb58ff816124718545 M      arch

Re: perf: odd event scheduling issue

[Peter Zijlstra]

On Wed, May 20, 2015 at 11:28:42AM +0200, Peter Zijlstra wrote:
 
> ---
> 
> c02cdbf60b51b8d98a49185535f5d527a2965142 is the first bad commit
> commit c02cdbf60b51b8d98a49185535f5d527a2965142
> Author: Stephane Eranian <eranian@google.com>
> Date:   Mon Nov 17 20:07:02 2014 +0100
> 
>     perf/x86/intel: Limit to half counters when the HT workaround is enabled, to avoid exclusive mode starvation
>     
>     This patch limits the number of counters available to each CPU when
>     the HT bug workaround is enabled.
>     
>     This is necessary to avoid situation of counter starvation. Such can
>     arise from configuration where one HT thread, HT0, is using all 4 counters
>     with corrupting events which require exclusion the the sibling HT, HT1.
>     
>     In such case, HT1 would not be able to schedule any event until HT0
>     is done. To mitigate this problem, this patch artificially limits
>     the number of counters to 2.
>     
>     That way, we can gurantee that at least 2 counters are not in exclusive
>     mode and therefore allow the sibling thread to schedule events of the
>     same type (system vs. per-thread). The 2 counters are not determined
>     in advance. We simply set the limit to two events per HT.
>     
>     This helps mitigate starvation in case of events with specific counter
>     constraints such a PREC_DIST.
>     
>     Note that this does not elimintate the starvation is all cases. But
>     it is better than not having it.
>     
>     (Solution suggested by Peter Zjilstra.)
>     
>     Signed-off-by: Stephane Eranian <eranian@google.com>
>     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>     Cc: bp@alien8.de
>     Cc: jolsa@redhat.com
>     Cc: kan.liang@intel.com
>     Cc: maria.n.dimakopoulou@gmail.com
>     Link: http://lkml.kernel.org/r/1416251225-17721-11-git-send-email-eranian@google.com
>     Signed-off-by: Ingo Molnar <mingo@kernel.org>

OK, so if you have the watchdog enabled, that's 1 event, and having a
max of 2 GP events, adding another 2 events is fail.

Jiri, did you SNB have the watchdog disabled?

Re: perf: odd event scheduling issue

[Peter Zijlstra]

On Wed, May 20, 2015 at 03:52:09PM +0200, Peter Zijlstra wrote:
> >     
> >     This patch limits the number of counters available to each CPU when
> >     the HT bug workaround is enabled.
> >     
> >     This is necessary to avoid situation of counter starvation. Such can
> >     arise from configuration where one HT thread, HT0, is using all 4 counters
> >     with corrupting events which require exclusion the the sibling HT, HT1.
> >     

> 
> OK, so if you have the watchdog enabled, that's 1 event, and having a
> max of 2 GP events, adding another 2 events is fail.

Hmm, so we count all events, including those scheduled on fixed purpose
counters.

Lemme see if I can cure that.

Re: perf: odd event scheduling issue

[Peter Zijlstra]

On Wed, May 20, 2015 at 05:25:34PM +0200, Peter Zijlstra wrote:

> > 
> > OK, so if you have the watchdog enabled, that's 1 event, and having a
> > max of 2 GP events, adding another 2 events is fail.
> 
> Hmm, so we count all events, including those scheduled on fixed purpose
> counters.
> 
> Lemme see if I can cure that.


Stephane, does something like the below make sense?

---
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index 3998131..7e779e9 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -2008,27 +2039,15 @@ intel_get_excl_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
 	xl = &excl_cntrs->states[tid];
 	xlo = &excl_cntrs->states[o_tid];
 
-	/*
-	 * do not allow scheduling of more than max_alloc_cntrs
-	 * which is set to half the available generic counters.
-	 * this helps avoid counter starvation of sibling thread
-	 * by ensuring at most half the counters cannot be in
-	 * exclusive mode. There is not designated counters for the
-	 * limits. Any N/2 counters can be used. This helps with
-	 * events with specifix counter constraints
-	 */
-	if (xl->num_alloc_cntrs++ == xl->max_alloc_cntrs)
-		return &emptyconstraint;
-
 	cx = c;
 
 	/*
-	 * because we modify the constraint, we need
+	 * Because we modify the constraint, we need
 	 * to make a copy. Static constraints come
 	 * from static const tables.
 	 *
-	 * only needed when constraint has not yet
-	 * been cloned (marked dynamic)
+	 * Only needed when constraint has not yet
+	 * been cloned (marked dynamic).
 	 */
 	if (!(c->flags & PERF_X86_EVENT_DYNAMIC)) {
 
@@ -2062,6 +2081,22 @@ intel_get_excl_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
 	 */
 
 	/*
+	 * Do not allow scheduling of more than max_alloc_cntrs
+	 * which is set to half the available generic counters.
+	 *
+	 * This helps avoid counter starvation of sibling thread
+	 * by ensuring at most half the counters cannot be in
+	 * exclusive mode. There is not designated counters for the
+	 * limits. Any N/2 counters can be used. This helps with
+	 * events with specifix counter constraints
+	 */
+	if (xl->num_alloc_cntrs++ >= xl->max_alloc_cntrs) {
+		/* wipe the GP counters */
+		cx->idxmsk64 &= ~((1ULL << INTEL_PMC_IDX_FIXED) - 1);
+		goto done;
+	}
+
+	/*
 	 * Modify static constraint with current dynamic
 	 * state of thread
 	 *
@@ -2086,6 +2121,7 @@ intel_get_excl_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
 			__clear_bit(i, cx->idxmsk);
 	}
 
+done:
 	/*
 	 * recompute actual bit weight for scheduling algorithm
 	 */

Re: perf: odd event scheduling issue

[Jiri Olsa]

On Wed, May 20, 2015 at 03:52:09PM +0200, Peter Zijlstra wrote:
> On Wed, May 20, 2015 at 11:28:42AM +0200, Peter Zijlstra wrote:
>  
> > ---
> > 
> > c02cdbf60b51b8d98a49185535f5d527a2965142 is the first bad commit
> > commit c02cdbf60b51b8d98a49185535f5d527a2965142
> > Author: Stephane Eranian <eranian@google.com>
> > Date:   Mon Nov 17 20:07:02 2014 +0100
> > 
> >     perf/x86/intel: Limit to half counters when the HT workaround is enabled, to avoid exclusive mode starvation
> >     
> >     This patch limits the number of counters available to each CPU when
> >     the HT bug workaround is enabled.
> >     
> >     This is necessary to avoid situation of counter starvation. Such can
> >     arise from configuration where one HT thread, HT0, is using all 4 counters
> >     with corrupting events which require exclusion the the sibling HT, HT1.
> >     
> >     In such case, HT1 would not be able to schedule any event until HT0
> >     is done. To mitigate this problem, this patch artificially limits
> >     the number of counters to 2.
> >     
> >     That way, we can gurantee that at least 2 counters are not in exclusive
> >     mode and therefore allow the sibling thread to schedule events of the
> >     same type (system vs. per-thread). The 2 counters are not determined
> >     in advance. We simply set the limit to two events per HT.
> >     
> >     This helps mitigate starvation in case of events with specific counter
> >     constraints such a PREC_DIST.
> >     
> >     Note that this does not elimintate the starvation is all cases. But
> >     it is better than not having it.
> >     
> >     (Solution suggested by Peter Zjilstra.)
> >     
> >     Signed-off-by: Stephane Eranian <eranian@google.com>
> >     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >     Cc: bp@alien8.de
> >     Cc: jolsa@redhat.com
> >     Cc: kan.liang@intel.com
> >     Cc: maria.n.dimakopoulou@gmail.com
> >     Link: http://lkml.kernel.org/r/1416251225-17721-11-git-send-email-eranian@google.com
> >     Signed-off-by: Ingo Molnar <mingo@kernel.org>
> 
> OK, so if you have the watchdog enabled, that's 1 event, and having a
> max of 2 GP events, adding another 2 events is fail.
> 
> Jiri, did you SNB have the watchdog disabled?

watchdog was enabled

jirka

Re: perf: odd event scheduling issue

[Peter Zijlstra]

On Wed, May 20, 2015 at 06:08:02PM +0200, Peter Zijlstra wrote:
> @@ -2062,6 +2081,22 @@ intel_get_excl_constraints(struct cpu_hw_events *cpuc, struct perf_event *event,
>  	 */
>  
>  	/*
> +	 * Do not allow scheduling of more than max_alloc_cntrs
> +	 * which is set to half the available generic counters.
> +	 *
> +	 * This helps avoid counter starvation of sibling thread
> +	 * by ensuring at most half the counters cannot be in
> +	 * exclusive mode. There is not designated counters for the
> +	 * limits. Any N/2 counters can be used. This helps with
> +	 * events with specifix counter constraints
> +	 */
> +	if (xl->num_alloc_cntrs++ >= xl->max_alloc_cntrs) {
> +		/* wipe the GP counters */
> +		cx->idxmsk64 &= ~((1ULL << INTEL_PMC_IDX_FIXED) - 1);
> +		goto done;
> +	}
> +
> +	/*
>  	 * Modify static constraint with current dynamic
>  	 * state of thread
>  	 *

While this improves things, its still sub optimal because we should only
increase num_alloc_cntrs when we actually allocate a GP register, but we
do that at commit time and that callback is too late to back out / retry.

So ideally we'd move the callback into scheduling code, but that means
we also have to move the xlo array into the sched_state etc.

[ which brings me to the whole xl vs xlo thing, I think we done that the
  wrong way around. It would be more natural to account to xl and create
  constraints based on xlo. ]

Secondly, we should only enforce this limit if and when there are
exclusive events on the system I suppose.

I have some ideas on how to go do this, but I need a break..

Re: perf: odd event scheduling issue

[Andi Kleen]

Peter Zijlstra <peterz@infradead.org> writes:
>
> Secondly, we should only enforce this limit if and when there are
> exclusive events on the system I suppose.

That's the key part really and absolutely needed.

-Andi

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

Re: perf: odd event scheduling issue

[Stephane Eranian]

On Wed, May 20, 2015 at 4:10 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
>>
>> Secondly, we should only enforce this limit if and when there are
>> exclusive events on the system I suppose.
>
> That's the key part really and absolutely needed.
>
Are you talking about the N/2 counters?
If so, they, yes I agree.

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

Re: perf: odd event scheduling issue

[Stephane Eranian]

On Wed, May 20, 2015 at 9:25 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, May 20, 2015 at 03:52:09PM +0200, Peter Zijlstra wrote:
>> On Wed, May 20, 2015 at 11:28:42AM +0200, Peter Zijlstra wrote:
>>
>> > ---
>> >
>> > c02cdbf60b51b8d98a49185535f5d527a2965142 is the first bad commit
>> > commit c02cdbf60b51b8d98a49185535f5d527a2965142
>> > Author: Stephane Eranian <eranian@google.com>
>> > Date:   Mon Nov 17 20:07:02 2014 +0100
>> >
>> >     perf/x86/intel: Limit to half counters when the HT workaround is enabled, to avoid exclusive mode starvation
>> >
>> >     This patch limits the number of counters available to each CPU when
>> >     the HT bug workaround is enabled.
>> >
>> >     This is necessary to avoid situation of counter starvation. Such can
>> >     arise from configuration where one HT thread, HT0, is using all 4 counters
>> >     with corrupting events which require exclusion the the sibling HT, HT1.
>> >
>> >     In such case, HT1 would not be able to schedule any event until HT0
>> >     is done. To mitigate this problem, this patch artificially limits
>> >     the number of counters to 2.
>> >
>> >     That way, we can gurantee that at least 2 counters are not in exclusive
>> >     mode and therefore allow the sibling thread to schedule events of the
>> >     same type (system vs. per-thread). The 2 counters are not determined
>> >     in advance. We simply set the limit to two events per HT.
>> >
>> >     This helps mitigate starvation in case of events with specific counter
>> >     constraints such a PREC_DIST.
>> >
>> >     Note that this does not elimintate the starvation is all cases. But
>> >     it is better than not having it.
>> >
>> >     (Solution suggested by Peter Zjilstra.)
>> >
>> >     Signed-off-by: Stephane Eranian <eranian@google.com>
>> >     Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> >     Cc: bp@alien8.de
>> >     Cc: jolsa@redhat.com
>> >     Cc: kan.liang@intel.com
>> >     Cc: maria.n.dimakopoulou@gmail.com
>> >     Link: http://lkml.kernel.org/r/1416251225-17721-11-git-send-email-eranian@google.com
>> >     Signed-off-by: Ingo Molnar <mingo@kernel.org>
>>
>> OK, so if you have the watchdog enabled, that's 1 event, and having a
>> max of 2 GP events, adding another 2 events is fail.
>>
>> Jiri, did you SNB have the watchdog disabled?
>
> watchdog was enabled
>
But maybe HT was disabled on your snb machine.

> jirka