RE: [patch 3/6] x86, perf, cqm: Remove pointless spinlock from state cache

[Juvva, Kanaka D]

Tested-by: Kanaka Juvva<kanaka.d.juvva@xxxxxxxxx>

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx]
> Sent: Monday, May 18, 2015 5:01 PM
> To: LKML
> Cc: Peter Zijlstra; Vikas Shivappa; x86@xxxxxxxxxx; Fleming, Matt; Auld, Will;
> Juvva, Kanaka D
> Subject: [patch 3/6] x86, perf, cqm: Remove pointless spinlock from state cache
> 
> struct intel_cqm_state is a strict per cpu cache of the rmid and the usage
> counter. It can never be modified from a remote cpu.
> 
> The 3 functions which modify the content: start, stop and del (del maps to stop)
> are called from the perf core with interrupts disabled which is enough protection
> for the per cpu state values.
> 
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/perf_event_intel_cqm.c |   17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> Index: linux/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> =================================================================
> ==
> --- linux.orig/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> +++ linux/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> @@ -17,11 +17,16 @@ static unsigned int cqm_max_rmid = -1;  static unsigned
> int cqm_l3_scale; /* supposedly cacheline size */
> 
>  struct intel_cqm_state {
> -	raw_spinlock_t		lock;
>  	u32			rmid;
>  	int			cnt;
>  };
> 
> +/*
> + * The cached intel_cqm_state is strictly per cpu and can never be
> + * updated from a remote cpu. Both functions which modify the state
> + * (intel_cqm_event_start and intel_cqm_event_stop) are called with
> + * interrupts disabled, which is sufficient for the protection.
> + */
>  static DEFINE_PER_CPU(struct intel_cqm_state, cqm_state);
> 
>  /*
> @@ -963,15 +968,12 @@ static void intel_cqm_event_start(struct  {
>  	struct intel_cqm_state *state = this_cpu_ptr(&cqm_state);
>  	u32 rmid = event->hw.cqm_rmid;
> -	unsigned long flags;
> 
>  	if (!(event->hw.cqm_state & PERF_HES_STOPPED))
>  		return;
> 
>  	event->hw.cqm_state &= ~PERF_HES_STOPPED;
> 
> -	raw_spin_lock_irqsave(&state->lock, flags);
> -
>  	if (state->cnt++)
>  		WARN_ON_ONCE(state->rmid != rmid);
>  	else
> @@ -984,21 +986,17 @@ static void intel_cqm_event_start(struct
>  	 * Technology component.
>  	 */
>  	wrmsr(MSR_IA32_PQR_ASSOC, rmid, 0);
> -
> -	raw_spin_unlock_irqrestore(&state->lock, flags);
>  }
> 
>  static void intel_cqm_event_stop(struct perf_event *event, int mode)  {
>  	struct intel_cqm_state *state = this_cpu_ptr(&cqm_state);
> -	unsigned long flags;
> 
>  	if (event->hw.cqm_state & PERF_HES_STOPPED)
>  		return;
> 
>  	event->hw.cqm_state |= PERF_HES_STOPPED;
> 
> -	raw_spin_lock_irqsave(&state->lock, flags);
>  	intel_cqm_event_read(event);
> 
>  	if (!--state->cnt) {
> @@ -1013,8 +1011,6 @@ static void intel_cqm_event_stop(struct
>  	} else {
>  		WARN_ON_ONCE(!state->rmid);
>  	}
> -
> -	raw_spin_unlock_irqrestore(&state->lock, flags);
>  }
> 
>  static int intel_cqm_event_add(struct perf_event *event, int mode) @@ -
> 1257,7 +1253,6 @@ static void intel_cqm_cpu_prepare(unsign
>  	struct intel_cqm_state *state = &per_cpu(cqm_state, cpu);
>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
> 
> -	raw_spin_lock_init(&state->lock);
>  	state->rmid = 0;
>  	state->cnt  = 0;
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/