[PATCH] perf/x86: fix bug in event constraint end marker macro

[PATCH] perf/x86: fix bug in event constraint end marker macro

[Stephane Eranian]

The EVENT_CONSTRAINT_END macro defines the end marker as
a constraint with a weight of zero. This was all fine
until we blacklisted the corrupting memory events on
Intel IvyBridge. These events are blacklisted by using
a counter bitmask of zero. Thus, they also get a constraint
weight of zero.

The iteration macro: for_each_constraint tests the weight==0.
Therefore, it was stopping at the first blacklisted event, i.e.,
0xd0. The corrupting events were therefore considered as
unconstrained and were scheduled on any of the generic counters.

This patch fixes the end marker to have a weight of -1. With this,
the blacklisted events get an empty constraint and cannot be
scheduled which is what we want for now.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/kernel/cpu/perf_event.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index fd00bb2..7c900c9 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -263,10 +263,16 @@ struct cpu_hw_events {
 			  HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_ST_HSW)
 
 #define EVENT_CONSTRAINT_END		\
-	EVENT_CONSTRAINT(0, 0, 0)
+	{ .idxmsk64 = 0,		\
+	  .code = 0,			\
+	  .cmask = 0,			\
+	  .weight = -1,			\
+	  .overlap = 0,			\
+	  .flags = 0,			\
+}
 
 #define for_each_event_constraint(e, c)	\
-	for ((e) = (c); (e)->weight; (e)++)
+	for ((e) = (c); (e)->weight != -1; (e)++)
 
 /*
  * Extra registers for specific events.
-- 
1.8.1.2

[PATCH] perf/x86: fix bug in event constraint end marker macro

[Stephane Eranian]

[repost because typo in Ingo's email address]

The EVENT_CONSTRAINT_END macro defines the end marker as
a constraint with a weight of zero. This was all fine
until we blacklisted the corrupting memory events on
Intel IvyBridge. These events are blacklisted by using
a counter bitmask of zero. Thus, they also get a constraint
weight of zero.

The iteration macro: for_each_constraint tests the weight==0.
Therefore, it was stopping at the first blacklisted event, i.e.,
0xd0. The corrupting events were therefore considered as
unconstrained and were scheduled on any of the generic counters.

This patch fixes the end marker to have a weight of -1. With this,
the blacklisted events get an empty constraint and cannot be
scheduled which is what we want for now.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/kernel/cpu/perf_event.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index fd00bb2..7c900c9 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -263,10 +263,16 @@ struct cpu_hw_events {
 			  HWEIGHT(n), 0, PERF_X86_EVENT_PEBS_ST_HSW)
 
 #define EVENT_CONSTRAINT_END		\
-	EVENT_CONSTRAINT(0, 0, 0)
+	{ .idxmsk64 = 0,		\
+	  .code = 0,			\
+	  .cmask = 0,			\
+	  .weight = -1,			\
+	  .overlap = 0,			\
+	  .flags = 0,			\
+}
 
 #define for_each_event_constraint(e, c)	\
-	for ((e) = (c); (e)->weight; (e)++)
+	for ((e) = (c); (e)->weight != -1; (e)++)
 
 /*
  * Extra registers for specific events.
-- 
1.8.1.2

Re: [PATCH] perf/x86: fix bug in event constraint end marker macro

[Ingo Molnar]

* Stephane Eranian <eranian@google.com> wrote:

>  #define EVENT_CONSTRAINT_END		\
> -	EVENT_CONSTRAINT(0, 0, 0)
> +	{ .idxmsk64 = 0,		\
> +	  .code = 0,			\
> +	  .cmask = 0,			\
> +	  .weight = -1,			\
> +	  .overlap = 0,			\
> +	  .flags = 0,			\
> +}

A nit, simply writing:

#define EVENT_CONSTRAINT_END { .weight = -1, }

... should be enough as initialization to zero is guaranteed for the 
other fields.

Thanks,

	Ingo

Re: [PATCH] perf/x86: fix bug in event constraint end marker macro

[Stephane Eranian]

Ingo,

Ok, I have asked Maria to clean the patch up add more comments and
repost a new version of the
patch. She will repost ASAP.

Thanks.

On Wed, Dec 4, 2013 at 10:03 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Stephane Eranian <eranian@google.com> wrote:
>
>>  #define EVENT_CONSTRAINT_END         \
>> -     EVENT_CONSTRAINT(0, 0, 0)
>> +     { .idxmsk64 = 0,                \
>> +       .code = 0,                    \
>> +       .cmask = 0,                   \
>> +       .weight = -1,                 \
>> +       .overlap = 0,                 \
>> +       .flags = 0,                   \
>> +}
>
> A nit, simply writing:
>
> #define EVENT_CONSTRAINT_END { .weight = -1, }
>
> ... should be enough as initialization to zero is guaranteed for the
> other fields.
>
> Thanks,
>
>         Ingo

Re: [PATCH] perf/x86: fix bug in event constraint end marker macro

[Ingo Molnar]

* Stephane Eranian <eranian@google.com> wrote:

> Ingo,
> 
> Ok, I have asked Maria to clean the patch up add more comments and
> repost a new version of the
> patch. She will repost ASAP.

Ok!

Thanks,

	Ingo