[PATCH] perf, x86: Fix Intel shared extra MSR allocation

[PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Peter Zijlstra]

Subject: perf: Fix Intel shared extra MSR allocation

Zheng Yan reported that event group validation can wreck event state
when Intel extra_reg allocation changes event state.

Validation shouldn't change any persistent state. Cloning events in
validate_{event,group}() isn't really pretty either, so add a few
special cases to avoid modifying the event state.

The code is restructured to minimize the special case impact.

Reported-by: Zheng Yan <zheng.z.yan@linux.intel.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 arch/x86/kernel/cpu/perf_event.c       |    1 +
 arch/x86/kernel/cpu/perf_event.h       |    1 +
 arch/x86/kernel/cpu/perf_event_intel.c |   74 +++++++++++++++++++++++---------
 3 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index e049d6d..cb60838 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1496,6 +1496,7 @@ static struct cpu_hw_events *allocate_fake_cpuc(void)
 		if (!cpuc->shared_regs)
 			goto error;
 	}
+	cpuc->is_fake = 1;
 	return cpuc;
 error:
 	free_fake_cpuc(cpuc);
diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index 6638aaf..83794d8 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -117,6 +117,7 @@ struct cpu_hw_events {
 	struct perf_event	*event_list[X86_PMC_IDX_MAX]; /* in enabled order */
 
 	unsigned int		group_flag;
+	int			is_fake;
 
 	/*
 	 * Intel DebugStore bits
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index 166546e..6d00c42 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -1119,27 +1119,33 @@ intel_bts_constraints(struct perf_event *event)
 	return NULL;
 }
 
-static bool intel_try_alt_er(struct perf_event *event, int orig_idx)
+static int intel_alt_er(int idx)
 {
 	if (!(x86_pmu.er_flags & ERF_HAS_RSP_1))
-		return false;
+		return idx;
 
-	if (event->hw.extra_reg.idx == EXTRA_REG_RSP_0) {
+	if (idx == EXTRA_REG_RSP_0)
+		return EXTRA_REG_RSP_1;
+
+	if (idx == EXTRA_REG_RSP_1)
+		return EXTRA_REG_RSP_0;
+
+	return idx;
+}
+
+static void intel_fixup_er(struct perf_event *event, int idx)
+{
+	if (idx == EXTRA_REG_RSP_0) {
 		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
 		event->hw.config |= 0x01bb;
 		event->hw.extra_reg.idx = EXTRA_REG_RSP_1;
 		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
-	} else if (event->hw.extra_reg.idx == EXTRA_REG_RSP_1) {
+	} else if (idx == EXTRA_REG_RSP_1) {
 		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
 		event->hw.config |= 0x01b7;
 		event->hw.extra_reg.idx = EXTRA_REG_RSP_0;
 		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
 	}
-
-	if (event->hw.extra_reg.idx == orig_idx)
-		return false;
-
-	return true;
 }
 
 /*
@@ -1157,14 +1163,14 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
 	struct event_constraint *c = &emptyconstraint;
 	struct er_account *era;
 	unsigned long flags;
-	int orig_idx = reg->idx;
+	int idx = reg->idx;
 
 	/* already allocated shared msr */
 	if (reg->alloc)
 		return NULL; /* call x86_get_event_constraint() */
 
 again:
-	era = &cpuc->shared_regs->regs[reg->idx];
+	era = &cpuc->shared_regs->regs[idx];
 	/*
 	 * we use spin_lock_irqsave() to avoid lockdep issues when
 	 * passing a fake cpuc
@@ -1173,6 +1179,29 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
 
 	if (!atomic_read(&era->ref) || era->config == reg->config) {
 
+		/*
+		 * If its a fake cpuc -- as per validate_{group,event}() we
+		 * shouldn't touch event state and we can avoid doing so
+		 * since both will only call get_event_constraints() once
+		 * on each event, this avoids the need for reg->alloc.
+		 *
+		 * Not doing the ER fixup will only result in era->reg being
+		 * wrong, but since we won't actually try and program hardware
+		 * this isn't a problem either.
+		 */
+		if (!cpuc->is_fake) {
+			if (idx != reg->idx)
+				intel_fixup_er(event, idx);
+
+			/* 
+			 * x86_schedule_events() can call get_event_constraints()
+			 * multiple times on events in the case of incremental
+			 * scheduling(). reg->alloc ensures we only do the ER
+			 * allocation once.
+			 */
+			reg->alloc = 1;
+		}
+
 		/* lock in msr value */
 		era->config = reg->config;
 		era->reg = reg->reg;
@@ -1180,17 +1209,17 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
 		/* one more user */
 		atomic_inc(&era->ref);
 
-		/* no need to reallocate during incremental event scheduling */
-		reg->alloc = 1;
-
 		/*
 		 * need to call x86_get_event_constraint()
 		 * to check if associated event has constraints
 		 */
 		c = NULL;
-	} else if (intel_try_alt_er(event, orig_idx)) {
-		raw_spin_unlock_irqrestore(&era->lock, flags);
-		goto again;
+	} else {
+		idx = intel_alt_er(idx);
+		if (idx != reg->idx) {
+			raw_spin_unlock_irqrestore(&era->lock, flags);
+			goto again;
+		}
 	}
 	raw_spin_unlock_irqrestore(&era->lock, flags);
 
@@ -1204,11 +1233,14 @@ __intel_shared_reg_put_constraints(struct cpu_hw_events *cpuc,
 	struct er_account *era;
 
 	/*
-	 * only put constraint if extra reg was actually
-	 * allocated. Also takes care of event which do
-	 * not use an extra shared reg
+	 * Only put constraint if extra reg was actually allocated. Also takes
+	 * care of event which do not use an extra shared reg.
+	 *
+	 * Also, if this is a fake cpuc we shouldn't touch any event state
+	 * (reg->alloc) and we don't care about leaving inconsistent cpuc state
+	 * either since it'll be thrown out.
 	 */
-	if (!reg->alloc)
+	if (!reg->alloc || cpuc->is_fake)
 		return;
 
 	era = &cpuc->shared_regs->regs[reg->idx];

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Peter Zijlstra]

On Tue, 2012-06-05 at 15:30 +0200, Peter Zijlstra wrote:

> @@ -1157,14 +1163,14 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>  	struct event_constraint *c = &emptyconstraint;
>  	struct er_account *era;
>  	unsigned long flags;
> -	int orig_idx = reg->idx;
> +	int idx = reg->idx;
>  
>  	/* already allocated shared msr */
>  	if (reg->alloc)
>  		return NULL; /* call x86_get_event_constraint() */

I'm afraid that needs to be:

	/*
	 * reg->alloc can be set due to existing state, so for fake cpuc
	 * we need to ignore this, otherwise we might fail to allocate
	 * proper fake state for this extra reg constraint. Also see
	 * the comment below.
	 */
	if (reg->alloc && !cpuc->is_fake)
		return NULL; /* call x86_get_event_constraints() */

>  
>  again:
> -	era = &cpuc->shared_regs->regs[reg->idx];
> +	era = &cpuc->shared_regs->regs[idx];
>  	/*
>  	 * we use spin_lock_irqsave() to avoid lockdep issues when
>  	 * passing a fake cpuc
> @@ -1173,6 +1179,29 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>  
>  	if (!atomic_read(&era->ref) || era->config == reg->config) {
>  
> +		/*
> +		 * If its a fake cpuc -- as per validate_{group,event}() we
> +		 * shouldn't touch event state and we can avoid doing so
> +		 * since both will only call get_event_constraints() once
> +		 * on each event, this avoids the need for reg->alloc.
> +		 *
> +		 * Not doing the ER fixup will only result in era->reg being
> +		 * wrong, but since we won't actually try and program hardware
> +		 * this isn't a problem either.
> +		 */
> +		if (!cpuc->is_fake) {
> +			if (idx != reg->idx)
> +				intel_fixup_er(event, idx);
> +
> +			/* 
> +			 * x86_schedule_events() can call get_event_constraints()
> +			 * multiple times on events in the case of incremental
> +			 * scheduling(). reg->alloc ensures we only do the ER
> +			 * allocation once.
> +			 */
> +			reg->alloc = 1;
> +		}
> +
>  		/* lock in msr value */
>  		era->config = reg->config;
>  		era->reg = reg->reg;

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Stephane Eranian]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 3058 bytes --]

On Tue, Jun 5, 2012 at 3:56 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, 2012-06-05 at 15:30 +0200, Peter Zijlstra wrote:
>
>> @@ -1157,14 +1163,14 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>>       struct event_constraint *c = &emptyconstraint;
>>       struct er_account *era;
>>       unsigned long flags;
>> -     int orig_idx = reg->idx;
>> +     int idx = reg->idx;
>>
>>       /* already allocated shared msr */
>>       if (reg->alloc)
>>               return NULL; /* call x86_get_event_constraint() */
>
> I'm afraid that needs to be:
>
>        /*
>         * reg->alloc can be set due to existing state, so for fake cpuc
>         * we need to ignore this, otherwise we might fail to allocate
>         * proper fake state for this extra reg constraint. Also see
>         * the comment below.
>         */
>        if (reg->alloc && !cpuc->is_fake)
>                return NULL; /* call x86_get_event_constraints() */
>
>>
Yes.

>>  again:
>> -     era = &cpuc->shared_regs->regs[reg->idx];
>> +     era = &cpuc->shared_regs->regs[idx];
>>       /*
>>        * we use spin_lock_irqsave() to avoid lockdep issues when
>>        * passing a fake cpuc
>> @@ -1173,6 +1179,29 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>>
>>       if (!atomic_read(&era->ref) || era->config == reg->config) {
>>
>> +             /*
>> +              * If its a fake cpuc -- as per validate_{group,event}() we
>> +              * shouldn't touch event state and we can avoid doing so
>> +              * since both will only call get_event_constraints() once
>> +              * on each event, this avoids the need for reg->alloc.
>> +              *
>> +              * Not doing the ER fixup will only result in era->reg being
>> +              * wrong, but since we won't actually try and program hardware
>> +              * this isn't a problem either.
>> +              */
>> +             if (!cpuc->is_fake) {
>> +                     if (idx != reg->idx)
>> +                             intel_fixup_er(event, idx);
>> +
>> +                     /*
>> +                      * x86_schedule_events() can call get_event_constraints()
>> +                      * multiple times on events in the case of incremental
>> +                      * scheduling(). reg->alloc ensures we only do the ER
>> +                      * allocation once.
>> +                      */
>> +                     reg->alloc = 1;
>> +             }
>> +
>>               /* lock in msr value */
>>               era->config = reg->config;
>>               era->reg = reg->reg;
>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Stephane Eranian]

Zheng Yan reported that event group validation can wreck event state
when Intel extra_reg allocation changes event state.

Validation shouldn't change any persistent state. Cloning events in
validate_{event,group}() isn't really pretty either, so add a few
special cases to avoid modifying the event state.

The code is restructured to minimize the special case impact.

Reported-by: Zheng Yan <zheng.z.yan@linux.intel.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Stephane Eranian <eranian@google.com>
---

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index e049d6d..cb60838 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1496,6 +1496,7 @@ static struct cpu_hw_events *allocate_fake_cpuc(void)
 		if (!cpuc->shared_regs)
 			goto error;
 	}
+	cpuc->is_fake = 1;
 	return cpuc;
 error:
 	free_fake_cpuc(cpuc);
diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index 6638aaf..83794d8 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -117,6 +117,7 @@ struct cpu_hw_events {
 	struct perf_event	*event_list[X86_PMC_IDX_MAX]; /* in enabled order */
 
 	unsigned int		group_flag;
+	int			is_fake;
 
 	/*
 	 * Intel DebugStore bits
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index 166546e..76a2bd2 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -1119,27 +1119,33 @@ intel_bts_constraints(struct perf_event *event)
 	return NULL;
 }
 
-static bool intel_try_alt_er(struct perf_event *event, int orig_idx)
+static int intel_alt_er(int idx)
 {
 	if (!(x86_pmu.er_flags & ERF_HAS_RSP_1))
-		return false;
+		return idx;
 
-	if (event->hw.extra_reg.idx == EXTRA_REG_RSP_0) {
+	if (idx == EXTRA_REG_RSP_0)
+		return EXTRA_REG_RSP_1;
+
+	if (idx == EXTRA_REG_RSP_1)
+		return EXTRA_REG_RSP_0;
+
+	return idx;
+}
+
+static void intel_fixup_er(struct perf_event *event, int idx)
+{
+	if (idx == EXTRA_REG_RSP_0) {
 		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
 		event->hw.config |= 0x01bb;
 		event->hw.extra_reg.idx = EXTRA_REG_RSP_1;
 		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
-	} else if (event->hw.extra_reg.idx == EXTRA_REG_RSP_1) {
+	} else if (idx == EXTRA_REG_RSP_1) {
 		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
 		event->hw.config |= 0x01b7;
 		event->hw.extra_reg.idx = EXTRA_REG_RSP_0;
 		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
 	}
-
-	if (event->hw.extra_reg.idx == orig_idx)
-		return false;
-
-	return true;
 }
 
 /*
@@ -1157,14 +1163,19 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
 	struct event_constraint *c = &emptyconstraint;
 	struct er_account *era;
 	unsigned long flags;
-	int orig_idx = reg->idx;
+	int idx = reg->idx;
 
-	/* already allocated shared msr */
-	if (reg->alloc)
-		return NULL; /* call x86_get_event_constraint() */
+	/*
+	 * reg->alloc can be set due to existing state, so for fake cpuc
+	 * we need to ignore this, otherwise we might fail to allocate
+	 * proper fake state for this extra reg constraint. Also see
+	 * the comment below.
+	*/
+	if (reg->alloc && !cpuc->is_fake)
+		return NULL; /* call x86_get_event_constraints() */
 
 again:
-	era = &cpuc->shared_regs->regs[reg->idx];
+	era = &cpuc->shared_regs->regs[idx];
 	/*
 	 * we use spin_lock_irqsave() to avoid lockdep issues when
 	 * passing a fake cpuc
@@ -1172,6 +1183,27 @@ again:
 	raw_spin_lock_irqsave(&era->lock, flags);
 
 	if (!atomic_read(&era->ref) || era->config == reg->config) {
+		/*
+		 * If its a fake cpuc -- as per validate_{group,event}() we
+		 * shouldn't touch event state and we can avoid doing so
+		 * since both will only call get_event_constraints() once
+		 * on each event, this avoids the need for reg->alloc.
+		 *
+		 * Not doing the ER fixup will only result in era->reg being
+		 * wrong, but since we won't actually try and program hardware
+		 * this isn't a problem either.
+		 */
+		if (!cpuc->is_fake) {
+			if (idx != reg->idx)
+				intel_fixup_er(event, idx);
+			/*
+			 * x86_schedule_events() calls get_event_constraints()
+			 * multiple times on events in the case of incremental
+			 * scheduling(). reg->alloc ensures we only do the ER
+			 * allocation once.
+			 */
+			reg->alloc = 1;
+		}
 
 		/* lock in msr value */
 		era->config = reg->config;
@@ -1180,18 +1212,19 @@ again:
 		/* one more user */
 		atomic_inc(&era->ref);
 
-		/* no need to reallocate during incremental event scheduling */
-		reg->alloc = 1;
-
 		/*
 		 * need to call x86_get_event_constraint()
 		 * to check if associated event has constraints
 		 */
 		c = NULL;
-	} else if (intel_try_alt_er(event, orig_idx)) {
-		raw_spin_unlock_irqrestore(&era->lock, flags);
-		goto again;
+	} else {
+		idx = intel_alt_er(idx);
+		if (idx != reg->idx) {
+			raw_spin_unlock_irqrestore(&era->lock, flags);
+			goto again;
+		}
 	}
+
 	raw_spin_unlock_irqrestore(&era->lock, flags);
 
 	return c;
@@ -1204,11 +1237,14 @@ __intel_shared_reg_put_constraints(struct cpu_hw_events *cpuc,
 	struct er_account *era;
 
 	/*
-	 * only put constraint if extra reg was actually
-	 * allocated. Also takes care of event which do
-	 * not use an extra shared reg
+	 * Only put constraint if extra reg was actually allocated. Also takes
+	 * care of event which do not use an extra shared reg.
+	 *
+	 * Also, if this is a fake cpuc we shouldn't touch any event state
+	 * (reg->alloc) and we don't care about leaving inconsistent cpuc state
+	 * either since it'll be thrown out.
 	 */
-	if (!reg->alloc)
+	if (!reg->alloc || cpuc->is_fake)
 		return;
 
 	era = &cpuc->shared_regs->regs[reg->idx];

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Yan, Zheng]

On 06/05/2012 09:56 PM, Peter Zijlstra wrote:
>> @@ -1157,14 +1163,14 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>> >  	struct event_constraint *c = &emptyconstraint;
>> >  	struct er_account *era;
>> >  	unsigned long flags;
>> > -	int orig_idx = reg->idx;
>> > +	int idx = reg->idx;
>> >  
>> >  	/* already allocated shared msr */
>> >  	if (reg->alloc)
>> >  		return NULL; /* call x86_get_event_constraint() */
> I'm afraid that needs to be:
> 
> 	/*
> 	 * reg->alloc can be set due to existing state, so for fake cpuc
> 	 * we need to ignore this, otherwise we might fail to allocate
> 	 * proper fake state for this extra reg constraint. Also see
> 	 * the comment below.
> 	 */
> 	if (reg->alloc && !cpuc->is_fake)
> 		return NULL; /* call x86_get_event_constraints() */
> 
Agree

Regards
Yan, Zheng

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Stephane Eranian]

Ok, I found the problem. It was in intel_fixup_er().
Unlike in the original code, this routine must update
the event->extra_reg.idx to the idx parameter instead
of trying to swap out from it.

I will repost an updated version (v2).


On Tue, Jun 5, 2012 at 11:35 PM, Stephane Eranian <eranian@google.com> wrote:
>
> Zheng Yan reported that event group validation can wreck event state
> when Intel extra_reg allocation changes event state.
>
> Validation shouldn't change any persistent state. Cloning events in
> validate_{event,group}() isn't really pretty either, so add a few
> special cases to avoid modifying the event state.
>
> The code is restructured to minimize the special case impact.
>
> Reported-by: Zheng Yan <zheng.z.yan@linux.intel.com>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Acked-by: Stephane Eranian <eranian@google.com>
> ---
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index e049d6d..cb60838 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1496,6 +1496,7 @@ static struct cpu_hw_events *allocate_fake_cpuc(void)
>                if (!cpuc->shared_regs)
>                        goto error;
>        }
> +       cpuc->is_fake = 1;
>        return cpuc;
>  error:
>        free_fake_cpuc(cpuc);
> diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
> index 6638aaf..83794d8 100644
> --- a/arch/x86/kernel/cpu/perf_event.h
> +++ b/arch/x86/kernel/cpu/perf_event.h
> @@ -117,6 +117,7 @@ struct cpu_hw_events {
>        struct perf_event       *event_list[X86_PMC_IDX_MAX]; /* in enabled order */
>
>        unsigned int            group_flag;
> +       int                     is_fake;
>
>        /*
>         * Intel DebugStore bits
> diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
> index 166546e..76a2bd2 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel.c
> @@ -1119,27 +1119,33 @@ intel_bts_constraints(struct perf_event *event)
>        return NULL;
>  }
>
> -static bool intel_try_alt_er(struct perf_event *event, int orig_idx)
> +static int intel_alt_er(int idx)
>  {
>        if (!(x86_pmu.er_flags & ERF_HAS_RSP_1))
> -               return false;
> +               return idx;
>
> -       if (event->hw.extra_reg.idx == EXTRA_REG_RSP_0) {
> +       if (idx == EXTRA_REG_RSP_0)
> +               return EXTRA_REG_RSP_1;
> +
> +       if (idx == EXTRA_REG_RSP_1)
> +               return EXTRA_REG_RSP_0;
> +
> +       return idx;
> +}
> +
> +static void intel_fixup_er(struct perf_event *event, int idx)
> +{
> +       if (idx == EXTRA_REG_RSP_0) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01bb;
>                event->hw.extra_reg.idx = EXTRA_REG_RSP_1;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
> -       } else if (event->hw.extra_reg.idx == EXTRA_REG_RSP_1) {
> +       } else if (idx == EXTRA_REG_RSP_1) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01b7;
>                event->hw.extra_reg.idx = EXTRA_REG_RSP_0;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
>        }
> -
> -       if (event->hw.extra_reg.idx == orig_idx)
> -               return false;
> -
> -       return true;
>  }
>
>  /*
> @@ -1157,14 +1163,19 @@ __intel_shared_reg_get_constraints(struct cpu_hw_events *cpuc,
>        struct event_constraint *c = &emptyconstraint;
>        struct er_account *era;
>        unsigned long flags;
> -       int orig_idx = reg->idx;
> +       int idx = reg->idx;
>
> -       /* already allocated shared msr */
> -       if (reg->alloc)
> -               return NULL; /* call x86_get_event_constraint() */
> +       /*
> +        * reg->alloc can be set due to existing state, so for fake cpuc
> +        * we need to ignore this, otherwise we might fail to allocate
> +        * proper fake state for this extra reg constraint. Also see
> +        * the comment below.
> +       */
> +       if (reg->alloc && !cpuc->is_fake)
> +               return NULL; /* call x86_get_event_constraints() */
>
>  again:
> -       era = &cpuc->shared_regs->regs[reg->idx];
> +       era = &cpuc->shared_regs->regs[idx];
>        /*
>         * we use spin_lock_irqsave() to avoid lockdep issues when
>         * passing a fake cpuc
> @@ -1172,6 +1183,27 @@ again:
>        raw_spin_lock_irqsave(&era->lock, flags);
>
>        if (!atomic_read(&era->ref) || era->config == reg->config) {
> +               /*
> +                * If its a fake cpuc -- as per validate_{group,event}() we
> +                * shouldn't touch event state and we can avoid doing so
> +                * since both will only call get_event_constraints() once
> +                * on each event, this avoids the need for reg->alloc.
> +                *
> +                * Not doing the ER fixup will only result in era->reg being
> +                * wrong, but since we won't actually try and program hardware
> +                * this isn't a problem either.
> +                */
> +               if (!cpuc->is_fake) {
> +                       if (idx != reg->idx)
> +                               intel_fixup_er(event, idx);
> +                       /*
> +                        * x86_schedule_events() calls get_event_constraints()
> +                        * multiple times on events in the case of incremental
> +                        * scheduling(). reg->alloc ensures we only do the ER
> +                        * allocation once.
> +                        */
> +                       reg->alloc = 1;
> +               }
>
>                /* lock in msr value */
>                era->config = reg->config;
> @@ -1180,18 +1212,19 @@ again:
>                /* one more user */
>                atomic_inc(&era->ref);
>
> -               /* no need to reallocate during incremental event scheduling */
> -               reg->alloc = 1;
> -
>                /*
>                 * need to call x86_get_event_constraint()
>                 * to check if associated event has constraints
>                 */
>                c = NULL;
> -       } else if (intel_try_alt_er(event, orig_idx)) {
> -               raw_spin_unlock_irqrestore(&era->lock, flags);
> -               goto again;
> +       } else {
> +               idx = intel_alt_er(idx);
> +               if (idx != reg->idx) {
> +                       raw_spin_unlock_irqrestore(&era->lock, flags);
> +                       goto again;
> +               }
>        }
> +
>        raw_spin_unlock_irqrestore(&era->lock, flags);
>
>        return c;
> @@ -1204,11 +1237,14 @@ __intel_shared_reg_put_constraints(struct cpu_hw_events *cpuc,
>        struct er_account *era;
>
>        /*
> -        * only put constraint if extra reg was actually
> -        * allocated. Also takes care of event which do
> -        * not use an extra shared reg
> +        * Only put constraint if extra reg was actually allocated. Also takes
> +        * care of event which do not use an extra shared reg.
> +        *
> +        * Also, if this is a fake cpuc we shouldn't touch any event state
> +        * (reg->alloc) and we don't care about leaving inconsistent cpuc state
> +        * either since it'll be thrown out.
>         */
> -       if (!reg->alloc)
> +       if (!reg->alloc || cpuc->is_fake)
>                return;
>
>        era = &cpuc->shared_regs->regs[reg->idx];

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Peter Zijlstra]

On Wed, 2012-06-06 at 12:35 +0200, Stephane Eranian wrote:
> Ok, I found the problem. It was in intel_fixup_er().
> Unlike in the original code, this routine must update
> the event->extra_reg.idx to the idx parameter instead
> of trying to swap out from it. 

Ah indeed. Thanks!

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Peter Zijlstra]

On Wed, 2012-06-06 at 12:36 +0200, Peter Zijlstra wrote:
> On Wed, 2012-06-06 at 12:35 +0200, Stephane Eranian wrote:
> > Ok, I found the problem. It was in intel_fixup_er().
> > Unlike in the original code, this routine must update
> > the event->extra_reg.idx to the idx parameter instead
> > of trying to swap out from it. 
> 
> Ah indeed. Thanks!


static void intel_fixup_er(struct perf_event *event, int idx)
{
        event->hw.extra_reg.idx = idx;

        if (idx == EXTRA_REG_RSP_0) {
                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
                event->hw.config |= 0x01b7;
                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
        } else if (idx == EXTRA_REG_RSP_1) {
                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
                event->hw.config |= 0x01bb;
                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
        }
}

Like that then?

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Stephane Eranian]

On Wed, Jun 6, 2012 at 12:53 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2012-06-06 at 12:36 +0200, Peter Zijlstra wrote:
>> On Wed, 2012-06-06 at 12:35 +0200, Stephane Eranian wrote:
>> > Ok, I found the problem. It was in intel_fixup_er().
>> > Unlike in the original code, this routine must update
>> > the event->extra_reg.idx to the idx parameter instead
>> > of trying to swap out from it.
>>
>> Ah indeed. Thanks!
>
>
> static void intel_fixup_er(struct perf_event *event, int idx)
> {
>        event->hw.extra_reg.idx = idx;
>
>        if (idx == EXTRA_REG_RSP_0) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01b7;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
>        } else if (idx == EXTRA_REG_RSP_1) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01bb;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
>        }
> }
>
> Like that then?

Yes.

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Stephane Eranian]

On Wed, Jun 6, 2012 at 12:53 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2012-06-06 at 12:36 +0200, Peter Zijlstra wrote:
>> On Wed, 2012-06-06 at 12:35 +0200, Stephane Eranian wrote:
>> > Ok, I found the problem. It was in intel_fixup_er().
>> > Unlike in the original code, this routine must update
>> > the event->extra_reg.idx to the idx parameter instead
>> > of trying to swap out from it.
>>
>> Ah indeed. Thanks!
>
>
> static void intel_fixup_er(struct perf_event *event, int idx)
> {
>        event->hw.extra_reg.idx = idx;
>
>        if (idx == EXTRA_REG_RSP_0) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01b7;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
>        } else if (idx == EXTRA_REG_RSP_1) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01bb;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
>        }
> }
>
> Like that then?


Are you going to repost the update patch, or shall I?

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Peter Zijlstra]

On Wed, 2012-06-06 at 13:57 +0200, Stephane Eranian wrote:

> Are you going to repost the update patch, or shall I?



---
Subject: perf, x86: Fix Intel shared extra MSR allocation
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 05 Jun 2012 15:30:31 +0200

Zheng Yan reported that event group validation can wreck event state
when Intel extra_reg allocation changes event state.

Validation shouldn't change any persistent state. Cloning events in
validate_{event,group}() isn't really pretty either, so add a few
special cases to avoid modifying the event state.

The code is restructured to minimize the special case impact.

Reported-by: Zheng Yan <zheng.z.yan@linux.intel.com>
Acked-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1338903031.28282.175.camel@twins
---
 arch/x86/kernel/cpu/perf_event.c       |    1 
 arch/x86/kernel/cpu/perf_event.h       |    1 
 arch/x86/kernel/cpu/perf_event_intel.c |   92 ++++++++++++++++++++++-----------
 3 files changed, 66 insertions(+), 28 deletions(-)

--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1496,6 +1496,7 @@ static struct cpu_hw_events *allocate_fa
 		if (!cpuc->shared_regs)
 			goto error;
 	}
+	cpuc->is_fake = 1;
 	return cpuc;
 error:
 	free_fake_cpuc(cpuc);
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -117,6 +117,7 @@ struct cpu_hw_events {
 	struct perf_event	*event_list[X86_PMC_IDX_MAX]; /* in enabled order */
 
 	unsigned int		group_flag;
+	int			is_fake;
 
 	/*
 	 * Intel DebugStore bits
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -1119,27 +1119,33 @@ intel_bts_constraints(struct perf_event
 	return NULL;
 }
 
-static bool intel_try_alt_er(struct perf_event *event, int orig_idx)
+static int intel_alt_er(int idx)
 {
 	if (!(x86_pmu.er_flags & ERF_HAS_RSP_1))
-		return false;
+		return idx;
 
-	if (event->hw.extra_reg.idx == EXTRA_REG_RSP_0) {
-		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
-		event->hw.config |= 0x01bb;
-		event->hw.extra_reg.idx = EXTRA_REG_RSP_1;
-		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
-	} else if (event->hw.extra_reg.idx == EXTRA_REG_RSP_1) {
+	if (idx == EXTRA_REG_RSP_0)
+		return EXTRA_REG_RSP_1;
+
+	if (idx == EXTRA_REG_RSP_1)
+		return EXTRA_REG_RSP_0;
+
+	return idx;
+}
+
+static void intel_fixup_er(struct perf_event *event, int idx)
+{
+	event->hw.extra_reg.idx = idx;
+
+	if (idx == EXTRA_REG_RSP_0) {
 		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
 		event->hw.config |= 0x01b7;
-		event->hw.extra_reg.idx = EXTRA_REG_RSP_0;
 		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
+	} else if (idx == EXTRA_REG_RSP_1) {
+		event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
+		event->hw.config |= 0x01bb;
+		event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
 	}
-
-	if (event->hw.extra_reg.idx == orig_idx)
-		return false;
-
-	return true;
 }
 
 /*
@@ -1157,14 +1163,18 @@ __intel_shared_reg_get_constraints(struc
 	struct event_constraint *c = &emptyconstraint;
 	struct er_account *era;
 	unsigned long flags;
-	int orig_idx = reg->idx;
+	int idx = reg->idx;
 
-	/* already allocated shared msr */
-	if (reg->alloc)
+	/*
+	 * reg->alloc can be set due to existing state, so for fake cpuc we
+	 * need to ignore this, otherwise we might fail to allocate proper fake
+	 * state for this extra reg constraint. Also see the comment below.
+	 */
+	if (reg->alloc && !cpuc->is_fake)
 		return NULL; /* call x86_get_event_constraint() */
 
 again:
-	era = &cpuc->shared_regs->regs[reg->idx];
+	era = &cpuc->shared_regs->regs[idx];
 	/*
 	 * we use spin_lock_irqsave() to avoid lockdep issues when
 	 * passing a fake cpuc
@@ -1173,6 +1183,29 @@ __intel_shared_reg_get_constraints(struc
 
 	if (!atomic_read(&era->ref) || era->config == reg->config) {
 
+		/*
+		 * If its a fake cpuc -- as per validate_{group,event}() we
+		 * shouldn't touch event state and we can avoid doing so
+		 * since both will only call get_event_constraints() once
+		 * on each event, this avoids the need for reg->alloc.
+		 *
+		 * Not doing the ER fixup will only result in era->reg being
+		 * wrong, but since we won't actually try and program hardware
+		 * this isn't a problem either.
+		 */
+		if (!cpuc->is_fake) {
+			if (idx != reg->idx)
+				intel_fixup_er(event, idx);
+
+			/*
+			 * x86_schedule_events() can call get_event_constraints()
+			 * multiple times on events in the case of incremental
+			 * scheduling(). reg->alloc ensures we only do the ER
+			 * allocation once.
+			 */
+			reg->alloc = 1;
+		}
+
 		/* lock in msr value */
 		era->config = reg->config;
 		era->reg = reg->reg;
@@ -1180,17 +1213,17 @@ __intel_shared_reg_get_constraints(struc
 		/* one more user */
 		atomic_inc(&era->ref);
 
-		/* no need to reallocate during incremental event scheduling */
-		reg->alloc = 1;
-
 		/*
 		 * need to call x86_get_event_constraint()
 		 * to check if associated event has constraints
 		 */
 		c = NULL;
-	} else if (intel_try_alt_er(event, orig_idx)) {
-		raw_spin_unlock_irqrestore(&era->lock, flags);
-		goto again;
+	} else {
+		idx = intel_alt_er(idx);
+		if (idx != reg->idx) {
+			raw_spin_unlock_irqrestore(&era->lock, flags);
+			goto again;
+		}
 	}
 	raw_spin_unlock_irqrestore(&era->lock, flags);
 
@@ -1204,11 +1237,14 @@ __intel_shared_reg_put_constraints(struc
 	struct er_account *era;
 
 	/*
-	 * only put constraint if extra reg was actually
-	 * allocated. Also takes care of event which do
-	 * not use an extra shared reg
+	 * Only put constraint if extra reg was actually allocated. Also takes
+	 * care of event which do not use an extra shared reg.
+	 *
+	 * Also, if this is a fake cpuc we shouldn't touch any event state
+	 * (reg->alloc) and we don't care about leaving inconsistent cpuc state
+	 * either since it'll be thrown out.
 	 */
-	if (!reg->alloc)
+	if (!reg->alloc || cpuc->is_fake)
 		return;
 
 	era = &cpuc->shared_regs->regs[reg->idx];

Re: [PATCH] perf, x86: Fix Intel shared extra MSR allocation

[Stephane Eranian]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 8595 bytes --]

Looks good.
thanks.

On Wed, Jun 6, 2012 at 2:06 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2012-06-06 at 13:57 +0200, Stephane Eranian wrote:
>
>> Are you going to repost the update patch, or shall I?
>
>
>
> ---
> Subject: perf, x86: Fix Intel shared extra MSR allocation
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Tue, 05 Jun 2012 15:30:31 +0200
>
> Zheng Yan reported that event group validation can wreck event state
> when Intel extra_reg allocation changes event state.
>
> Validation shouldn't change any persistent state. Cloning events in
> validate_{event,group}() isn't really pretty either, so add a few
> special cases to avoid modifying the event state.
>
> The code is restructured to minimize the special case impact.
>
> Reported-by: Zheng Yan <zheng.z.yan@linux.intel.com>
> Acked-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Link: http://lkml.kernel.org/r/1338903031.28282.175.camel@twins
> ---
>  arch/x86/kernel/cpu/perf_event.c       |    1
>  arch/x86/kernel/cpu/perf_event.h       |    1
>  arch/x86/kernel/cpu/perf_event_intel.c |   92 ++++++++++++++++++++++-----------
>  3 files changed, 66 insertions(+), 28 deletions(-)
>
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1496,6 +1496,7 @@ static struct cpu_hw_events *allocate_fa
>                if (!cpuc->shared_regs)
>                        goto error;
>        }
> +       cpuc->is_fake = 1;
>        return cpuc;
>  error:
>        free_fake_cpuc(cpuc);
> --- a/arch/x86/kernel/cpu/perf_event.h
> +++ b/arch/x86/kernel/cpu/perf_event.h
> @@ -117,6 +117,7 @@ struct cpu_hw_events {
>        struct perf_event       *event_list[X86_PMC_IDX_MAX]; /* in enabled order */
>
>        unsigned int            group_flag;
> +       int                     is_fake;
>
>        /*
>         * Intel DebugStore bits
> --- a/arch/x86/kernel/cpu/perf_event_intel.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel.c
> @@ -1119,27 +1119,33 @@ intel_bts_constraints(struct perf_event
>        return NULL;
>  }
>
> -static bool intel_try_alt_er(struct perf_event *event, int orig_idx)
> +static int intel_alt_er(int idx)
>  {
>        if (!(x86_pmu.er_flags & ERF_HAS_RSP_1))
> -               return false;
> +               return idx;
>
> -       if (event->hw.extra_reg.idx == EXTRA_REG_RSP_0) {
> -               event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
> -               event->hw.config |= 0x01bb;
> -               event->hw.extra_reg.idx = EXTRA_REG_RSP_1;
> -               event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
> -       } else if (event->hw.extra_reg.idx == EXTRA_REG_RSP_1) {
> +       if (idx == EXTRA_REG_RSP_0)
> +               return EXTRA_REG_RSP_1;
> +
> +       if (idx == EXTRA_REG_RSP_1)
> +               return EXTRA_REG_RSP_0;
> +
> +       return idx;
> +}
> +
> +static void intel_fixup_er(struct perf_event *event, int idx)
> +{
> +       event->hw.extra_reg.idx = idx;
> +
> +       if (idx == EXTRA_REG_RSP_0) {
>                event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
>                event->hw.config |= 0x01b7;
> -               event->hw.extra_reg.idx = EXTRA_REG_RSP_0;
>                event->hw.extra_reg.reg = MSR_OFFCORE_RSP_0;
> +       } else if (idx == EXTRA_REG_RSP_1) {
> +               event->hw.config &= ~INTEL_ARCH_EVENT_MASK;
> +               event->hw.config |= 0x01bb;
> +               event->hw.extra_reg.reg = MSR_OFFCORE_RSP_1;
>        }
> -
> -       if (event->hw.extra_reg.idx == orig_idx)
> -               return false;
> -
> -       return true;
>  }
>
>  /*
> @@ -1157,14 +1163,18 @@ __intel_shared_reg_get_constraints(struc
>        struct event_constraint *c = &emptyconstraint;
>        struct er_account *era;
>        unsigned long flags;
> -       int orig_idx = reg->idx;
> +       int idx = reg->idx;
>
> -       /* already allocated shared msr */
> -       if (reg->alloc)
> +       /*
> +        * reg->alloc can be set due to existing state, so for fake cpuc we
> +        * need to ignore this, otherwise we might fail to allocate proper fake
> +        * state for this extra reg constraint. Also see the comment below.
> +        */
> +       if (reg->alloc && !cpuc->is_fake)
>                return NULL; /* call x86_get_event_constraint() */
>
>  again:
> -       era = &cpuc->shared_regs->regs[reg->idx];
> +       era = &cpuc->shared_regs->regs[idx];
>        /*
>         * we use spin_lock_irqsave() to avoid lockdep issues when
>         * passing a fake cpuc
> @@ -1173,6 +1183,29 @@ __intel_shared_reg_get_constraints(struc
>
>        if (!atomic_read(&era->ref) || era->config == reg->config) {
>
> +               /*
> +                * If its a fake cpuc -- as per validate_{group,event}() we
> +                * shouldn't touch event state and we can avoid doing so
> +                * since both will only call get_event_constraints() once
> +                * on each event, this avoids the need for reg->alloc.
> +                *
> +                * Not doing the ER fixup will only result in era->reg being
> +                * wrong, but since we won't actually try and program hardware
> +                * this isn't a problem either.
> +                */
> +               if (!cpuc->is_fake) {
> +                       if (idx != reg->idx)
> +                               intel_fixup_er(event, idx);
> +
> +                       /*
> +                        * x86_schedule_events() can call get_event_constraints()
> +                        * multiple times on events in the case of incremental
> +                        * scheduling(). reg->alloc ensures we only do the ER
> +                        * allocation once.
> +                        */
> +                       reg->alloc = 1;
> +               }
> +
>                /* lock in msr value */
>                era->config = reg->config;
>                era->reg = reg->reg;
> @@ -1180,17 +1213,17 @@ __intel_shared_reg_get_constraints(struc
>                /* one more user */
>                atomic_inc(&era->ref);
>
> -               /* no need to reallocate during incremental event scheduling */
> -               reg->alloc = 1;
> -
>                /*
>                 * need to call x86_get_event_constraint()
>                 * to check if associated event has constraints
>                 */
>                c = NULL;
> -       } else if (intel_try_alt_er(event, orig_idx)) {
> -               raw_spin_unlock_irqrestore(&era->lock, flags);
> -               goto again;
> +       } else {
> +               idx = intel_alt_er(idx);
> +               if (idx != reg->idx) {
> +                       raw_spin_unlock_irqrestore(&era->lock, flags);
> +                       goto again;
> +               }
>        }
>        raw_spin_unlock_irqrestore(&era->lock, flags);
>
> @@ -1204,11 +1237,14 @@ __intel_shared_reg_put_constraints(struc
>        struct er_account *era;
>
>        /*
> -        * only put constraint if extra reg was actually
> -        * allocated. Also takes care of event which do
> -        * not use an extra shared reg
> +        * Only put constraint if extra reg was actually allocated. Also takes
> +        * care of event which do not use an extra shared reg.
> +        *
> +        * Also, if this is a fake cpuc we shouldn't touch any event state
> +        * (reg->alloc) and we don't care about leaving inconsistent cpuc state
> +        * either since it'll be thrown out.
>         */
> -       if (!reg->alloc)
> +       if (!reg->alloc || cpuc->is_fake)
>                return;
>
>        era = &cpuc->shared_regs->regs[reg->idx];
>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥