Re: [PATCH 3/3] x86/intel_rdt: Update closid in PQR_ASSOC registers in synchronous mode when changing "cpus"

From: Thomas Gleixner
Date: Tue Nov 15 2016 - 11:43:00 EST


On Tue, 15 Nov 2016, Thomas Gleixner wrote:

> On Fri, 11 Nov 2016, Fenghua Yu wrote:
> > +/*
> > + * MSR_IA32_PQR_ASSOC is scoped per logical CPU, so all updates
> > + * are always in thread context.
>
> And this comment tells us what? Nothing useful. It lacks the most important
> information why this is safe against a logical CPU switching the MSR right
> at this moment during context switch. It's safe because the pqr_switch_to
> function is called with interrupts disabled.
>
> > + */
> > +static void rdt_update_pqr_assoc_closid(void *v)
> > +{
> > + struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
> > +
> > + state->closid = *(int *)v;
> > + wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, state->closid);

Furthermore, unconditionally writting the MSR is Just Wrong because the
current executing task might have it's own closid set which gets
overwritten by this.

No cookies!

tglx