On Fri, 11 Nov 2016, Fenghua Yu wrote:
> x86/intel_rdt: Update closid in PQR_ASSOC registers in synchronous mode when
> changing "cpus"
Subject lines are supposed to be short and precise.
> When "cpus" is changed in a rdtgroup, the current code doesn't update
> PQR_ASSOC registers with new closid in the cpus write operation.
> The PQR_ASSOC registers are updated asynchronously later when new
> processes are scheduled in on the CPUs.
>
> A process may run on a CPU for long time without being switched to
> another process, e.g. high performance computing or real time cases.
> Then closid in the PQR_ASSOC register on the CPU is not updated for
> long time until a new process is switched in.
Can you please be a bit more careful about your changelogs? The above is
reads like a fairy tale.
> This patch updates closid in PQR_ASSOC synchronously when writing
'This patch' - Grrr! I told you more than once that 'This patch' is
crap. We all know that this is a PATCH. See
Documentation/SubmittingPatches.
> +/*
> + * MSR_IA32_PQR_ASSOC is scoped per logical CPU, so all updates
> + * are always in thread context.
> + */
> +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);
> +}
> +
> +static void rdt_update_pqr_assoc(const struct cpumask *cpu_mask, int closid)
> +{
> + int cpu = get_cpu();
> +
> + /* Update PQR_ASSOC MSR on this cpu if it's in cpu_mask. */
> + if (cpumask_test_cpu(cpu, cpu_mask))
> + rdt_update_pqr_assoc_closid(&closid);
> + /* Update PQR_ASSOC MSR on the rest of cpus. */
> + smp_call_function_many(cpu_mask, rdt_update_pqr_assoc_closid,
> + &closid, 1);
> + put_cpu();
> +}
> static void rmdir_all_sub(void)
> @@ -809,13 +828,9 @@ static void rmdir_all_sub(void)
> t->closid = 0;
> read_unlock(&tasklist_lock);
>
> - get_cpu();
> - /* Reset PQR_ASSOC MSR on this cpu. */
> - rdt_reset_pqr_assoc_closid(NULL);
> - /* Reset PQR_ASSOC MSR on the rest of cpus. */
> - smp_call_function_many(cpu_online_mask, rdt_reset_pqr_assoc_closid,
> - NULL, 1);
> - put_cpu();
> + /* Reset PQR_ASSOC MSR on all online cpus */
> + rdt_update_pqr_assoc(cpu_online_mask, 0);
cpu_online_mask is not protected against changing before you hit the
get_cpu() in rdt_update_pqr_assoc().
Thanks,
tglx
