[PATCH 1/3] x86/intel_rdt: Fix kernfs_to_rdtgroup to know about "info/*" directories

[PATCH 1/3] x86/intel_rdt: Fix kernfs_to_rdtgroup to know about "info/*" directories

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

We saw a kernel oops on NULL pointer when we ran "rmdir info". This is
because the "info" directory and per-resource subdirectories of "info"
do not use "kn->priv" to point to a "struct rdtgroup".

Also change error code from -ENOENT to the more appropriate -EPERM
so the user sees "Operation not permitted".

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 4795880..cff286e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -644,16 +644,29 @@ static int parse_rdtgroupfs_options(char *data)
  */
 static struct rdtgroup *kernfs_to_rdtgroup(struct kernfs_node *kn)
 {
-	if (kernfs_type(kn) == KERNFS_DIR)
-		return kn->priv;
-	else
+	if (kernfs_type(kn) == KERNFS_DIR) {
+		/*
+		 * All the resource directories use "kn->priv"
+		 * to point to the "struct rdtgroup" for the
+		 * resource. "info" and its subdirectories don't
+		 * have rdtgroup structures, so return NULL here.
+		 */
+		if (kn == kn_info || kn->parent == kn_info)
+			return NULL;
+		else
+			return kn->priv;
+	} else {
 		return kn->parent->priv;
+	}
 }
 
 struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn)
 {
 	struct rdtgroup *rdtgrp = kernfs_to_rdtgroup(kn);
 
+	if (!rdtgrp)
+		return NULL;
+
 	atomic_inc(&rdtgrp->waitcount);
 	kernfs_break_active_protection(kn);
 
@@ -670,6 +683,9 @@ void rdtgroup_kn_unlock(struct kernfs_node *kn)
 {
 	struct rdtgroup *rdtgrp = kernfs_to_rdtgroup(kn);
 
+	if (!rdtgrp)
+		return;
+
 	mutex_unlock(&rdtgroup_mutex);
 
 	if (atomic_dec_and_test(&rdtgrp->waitcount) &&
@@ -918,7 +934,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 	rdtgrp = rdtgroup_kn_lock_live(kn);
 	if (!rdtgrp) {
 		rdtgroup_kn_unlock(kn);
-		return -ENOENT;
+		return -EPERM;
 	}
 
 	/* Give any tasks back to the default group */
-- 
2.5.0

[PATCH 2/3] x86/intel_rdt: Return state to default on umount

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

All CPUs in a rdtgroup are given back to the default rdtgroup
before the rdtgroup is removed during umount. After umount,
the default rdtgroup contains all online CPUs.

cpu_closid needs to be cleared for each cpu in root rdtgroup
during umount. Otherwise, PQR_ASSOC will be written to the
non-zero value stored in cpu_closid after next mount.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index cff286e..416b95e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -801,6 +801,7 @@ static void rmdir_all_sub(void)
 {
 	struct rdtgroup *rdtgrp, *tmp;
 	struct task_struct *p, *t;
+	int cpu;
 
 	/* move all tasks to default resource group */
 	read_lock(&tasklist_lock);
@@ -819,10 +820,19 @@ static void rmdir_all_sub(void)
 		/* Remove each rdtgroup other than root */
 		if (rdtgrp == &rdtgroup_default)
 			continue;
+
+		/* Give any CPUs back to the default group */
+		cpumask_or(&rdtgroup_default.cpu_mask,
+			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
+
 		kernfs_remove(rdtgrp->kn);
 		list_del(&rdtgrp->rdtgroup_list);
 		kfree(rdtgrp);
 	}
+
+	for_each_cpu(cpu, &rdtgroup_default.cpu_mask)
+		per_cpu(cpu_closid, cpu) = 0;
+
 	kernfs_remove(kn_info);
 }
 
-- 
2.5.0

Re: [PATCH 2/3] x86/intel_rdt: Return state to default on umount

[Thomas Gleixner]

On Fri, 11 Nov 2016, Fenghua Yu wrote:
> @@ -801,6 +801,7 @@ static void rmdir_all_sub(void)
>  {
>  	struct rdtgroup *rdtgrp, *tmp;
>  	struct task_struct *p, *t;
> +	int cpu;
>  
>  	/* move all tasks to default resource group */
>  	read_lock(&tasklist_lock);
> @@ -819,10 +820,19 @@ static void rmdir_all_sub(void)
>  		/* Remove each rdtgroup other than root */
>  		if (rdtgrp == &rdtgroup_default)
>  			continue;
> +
> +		/* Give any CPUs back to the default group */
> +		cpumask_or(&rdtgroup_default.cpu_mask,
> +			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);

That's a pointless exercise. We can simply copy cpu_online_mask to the
default group mask and be done with it. I'll fix that up.

Thanks,

	tglx

Re: [PATCH 2/3] x86/intel_rdt: Return state to default on umount

[Thomas Gleixner]

On Tue, 15 Nov 2016, Thomas Gleixner wrote:

> On Fri, 11 Nov 2016, Fenghua Yu wrote:
> > @@ -801,6 +801,7 @@ static void rmdir_all_sub(void)
> >  {
> >  	struct rdtgroup *rdtgrp, *tmp;
> >  	struct task_struct *p, *t;
> > +	int cpu;
> >  
> >  	/* move all tasks to default resource group */
> >  	read_lock(&tasklist_lock);
> > @@ -819,10 +820,19 @@ static void rmdir_all_sub(void)
> >  		/* Remove each rdtgroup other than root */
> >  		if (rdtgrp == &rdtgroup_default)
> >  			continue;
> > +
> > +		/* Give any CPUs back to the default group */
> > +		cpumask_or(&rdtgroup_default.cpu_mask,
> > +			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
> 
> That's a pointless exercise. We can simply copy cpu_online_mask to the
> default group mask and be done with it. I'll fix that up.

Actually we cannot. A CPU which already run the offline callback might
still be marked online. So the above is correct.

Thanks,

	tglx

[tip:x86/cache] x86/intel_rdt: Reset per cpu closids on unmount

[tip-bot for Fenghua Yu]

Commit-ID:  c7cc0cc10cdecc275211c8749defba6c41aaf5de
Gitweb:     http://git.kernel.org/tip/c7cc0cc10cdecc275211c8749defba6c41aaf5de
Author:     Fenghua Yu <fenghua.yu@intel.com>
AuthorDate: Fri, 11 Nov 2016 17:02:37 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 15 Nov 2016 18:35:50 +0100

x86/intel_rdt: Reset per cpu closids on unmount

All CPUs in a rdtgroup are given back to the default rdtgroup before the
rdtgroup is removed during umount. After umount, the default rdtgroup
contains all online CPUs, but the per cpu closids are not cleared. As a
result the stale closid value will be used immediately after the next
mount.

Move all cpus to the default group and update the percpu closid storage.

[ tglx: Massaged changelong ]

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Cc: "Ravi V Shankar" <ravi.v.shankar@intel.com>
Cc: "Tony Luck" <tony.luck@intel.com>
Cc: "Sai Prakhya" <sai.praneeth.prakhya@intel.com>
Cc: "Vikas Shivappa" <vikas.shivappa@linux.intel.com>
Cc: "Ingo Molnar" <mingo@elte.hu>
Cc: "H. Peter Anvin" <h.peter.anvin@intel.com>
Link: http://lkml.kernel.org/r/1478912558-55514-2-git-send-email-fenghua.yu@intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 2f54931..d6bad09 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -799,6 +799,7 @@ static void rmdir_all_sub(void)
 {
 	struct rdtgroup *rdtgrp, *tmp;
 	struct task_struct *p, *t;
+	int cpu;
 
 	/* move all tasks to default resource group */
 	read_lock(&tasklist_lock);
@@ -813,14 +814,29 @@ static void rmdir_all_sub(void)
 	smp_call_function_many(cpu_online_mask, rdt_reset_pqr_assoc_closid,
 			       NULL, 1);
 	put_cpu();
+
 	list_for_each_entry_safe(rdtgrp, tmp, &rdt_all_groups, rdtgroup_list) {
 		/* Remove each rdtgroup other than root */
 		if (rdtgrp == &rdtgroup_default)
 			continue;
+
+		/*
+		 * Give any CPUs back to the default group. We cannot copy
+		 * cpu_online_mask because a CPU might have executed the
+		 * offline callback already, but is still marked online.
+		 */
+		cpumask_or(&rdtgroup_default.cpu_mask,
+			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
+
 		kernfs_remove(rdtgrp->kn);
 		list_del(&rdtgrp->rdtgroup_list);
 		kfree(rdtgrp);
 	}
+
+	/* Reset all per cpu closids to the default value */
+	for_each_cpu(cpu, &rdtgroup_default.cpu_mask)
+		per_cpu(cpu_closid, cpu) = 0;
+
 	kernfs_remove(kn_info);
 }
 

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

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

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.

This patch updates closid in PQR_ASSOC synchronously when writing
cpus to avoid the above issue.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 53 ++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 416b95e..a45f2ba 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -191,6 +191,31 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+/*
+ * 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 ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 				   char *buf, size_t nbytes, loff_t off)
 {
@@ -238,6 +263,9 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 			   &rdtgroup_default.cpu_mask, tmpmask);
 		for_each_cpu(cpu, tmpmask)
 			per_cpu(cpu_closid, cpu) = 0;
+
+		/* Update PQR_ASSOC registers on the dropped cpus */
+		rdt_update_pqr_assoc(tmpmask, rdtgroup_default.closid);
 	}
 
 	/*
@@ -253,6 +281,9 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		}
 		for_each_cpu(cpu, tmpmask)
 			per_cpu(cpu_closid, cpu) = rdtgrp->closid;
+
+		/* Update PQR_ASSOC registers on the added cpus */
+		rdt_update_pqr_assoc(tmpmask, rdtgrp->closid);
 	}
 
 	/* Done pushing/pulling - update this group with new mask */
@@ -783,18 +814,6 @@ static int reset_all_cbms(struct rdt_resource *r)
 }
 
 /*
- * MSR_IA32_PQR_ASSOC is scoped per logical CPU, so all updates
- * are always in thread context.
- */
-static void rdt_reset_pqr_assoc_closid(void *v)
-{
-	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
-
-	state->closid = 0;
-	wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, 0);
-}
-
-/*
  * Forcibly remove all of subdirectories under root.
  */
 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);
+
 	list_for_each_entry_safe(rdtgrp, tmp, &rdt_all_groups, rdtgroup_list) {
 		/* Remove each rdtgroup other than root */
 		if (rdtgrp == &rdtgroup_default)
-- 
2.5.0

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

[Thomas Gleixner]

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

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

[Thomas Gleixner]

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);
> +}
> +
> +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 ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>  				   char *buf, size_t nbytes, loff_t off)
>  {
> @@ -238,6 +263,9 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>  			   &rdtgroup_default.cpu_mask, tmpmask);
>  		for_each_cpu(cpu, tmpmask)
>  			per_cpu(cpu_closid, cpu) = 0;
> +
> +		/* Update PQR_ASSOC registers on the dropped cpus */
> +		rdt_update_pqr_assoc(tmpmask, rdtgroup_default.closid);

Grr. You store the new closid now in the for_each_cpu() loop and in the smp
function call.

>  	}
>  
>  	/*
> @@ -253,6 +281,9 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>  		}
>  		for_each_cpu(cpu, tmpmask)
>  			per_cpu(cpu_closid, cpu) = rdtgrp->closid;
> +
> +		/* Update PQR_ASSOC registers on the added cpus */
> +		rdt_update_pqr_assoc(tmpmask, rdtgrp->closid);

Ditto.

>  	}
>  
>  	/* Done pushing/pulling - update this group with new mask */
> @@ -783,18 +814,6 @@ static int reset_all_cbms(struct rdt_resource *r)
>  }
>  
>  /*
> - * MSR_IA32_PQR_ASSOC is scoped per logical CPU, so all updates
> - * are always in thread context.
> - */
> -static void rdt_reset_pqr_assoc_closid(void *v)
> -{
> -	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
> -
> -	state->closid = 0;
> -	wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, 0);
> -}
> -
> -/*
>   * Forcibly remove all of subdirectories under root.
>   */
>  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);

And here you have the extra closid loop later on. Really useful.

Thanks,

	tglx

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

[Thomas Gleixner]

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

[tip:x86/cache] x86/intel_rdt: Update percpu closid immeditately on CPUs affected by changee

[tip-bot for Fenghua Yu]

Commit-ID:  f410770293a1fbc08906474c24104a7a11943eb6
Gitweb:     http://git.kernel.org/tip/f410770293a1fbc08906474c24104a7a11943eb6
Author:     Fenghua Yu <fenghua.yu@intel.com>
AuthorDate: Fri, 11 Nov 2016 17:02:38 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 15 Nov 2016 18:35:50 +0100

x86/intel_rdt: Update percpu closid immeditately on CPUs affected by changee

If CPUs are moved to or removed from a rdtgroup, the percpu closid storage
is updated. If tasks running on an affected CPU use the percpu closid then
the PQR_ASSOC MSR is only updated when the task runs through a context
switch. Up to the context switch the CPUs operate on the wrong closid. This
state is potentially unbound.
    
Make the change immediately effective by invoking a smp function call on
the affected CPUs which stores the new closid in the perpu storage and
calls the rdt_sched_in() function which updates the MSR, if the current
task uses the percpu closid.

[ tglx: Made it work and massaged changelog once more ]

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Cc: "Ravi V Shankar" <ravi.v.shankar@intel.com>
Cc: "Tony Luck" <tony.luck@intel.com>
Cc: "Sai Prakhya" <sai.praneeth.prakhya@intel.com>
Cc: "Vikas Shivappa" <vikas.shivappa@linux.intel.com>
Cc: "Ingo Molnar" <mingo@elte.hu>
Cc: "H. Peter Anvin" <h.peter.anvin@intel.com>
Link: http://lkml.kernel.org/r/1478912558-55514-3-git-send-email-fenghua.yu@intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 72 ++++++++++++++++----------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index d6bad09..98edba4 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -191,12 +191,40 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+/*
+ * This is safe against intel_rdt_sched_in() called from __switch_to()
+ * because __switch_to() is executed with interrupts disabled. A local call
+ * from rdt_update_percpu_closid() is proteced against __switch_to() because
+ * preemption is disabled.
+ */
+static void rdt_update_cpu_closid(void *v)
+{
+	this_cpu_write(cpu_closid, *(int *)v);
+	/*
+	 * We cannot unconditionally write the MSR because the current
+	 * executing task might have its own closid selected. Just reuse
+	 * the context switch code.
+	 */
+	intel_rdt_sched_in();
+}
+
+/* Update the per cpu closid and eventually the PGR_ASSOC MSR */
+static void rdt_update_percpu_closid(const struct cpumask *cpu_mask, int closid)
+{
+	int cpu = get_cpu();
+
+	if (cpumask_test_cpu(cpu, cpu_mask))
+		rdt_update_cpu_closid(&closid);
+	smp_call_function_many(cpu_mask, rdt_update_cpu_closid, &closid, 1);
+	put_cpu();
+}
+
 static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 				   char *buf, size_t nbytes, loff_t off)
 {
 	cpumask_var_t tmpmask, newmask;
 	struct rdtgroup *rdtgrp, *r;
-	int ret, cpu;
+	int ret;
 
 	if (!buf)
 		return -EINVAL;
@@ -236,8 +264,7 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		/* Give any dropped cpus to rdtgroup_default */
 		cpumask_or(&rdtgroup_default.cpu_mask,
 			   &rdtgroup_default.cpu_mask, tmpmask);
-		for_each_cpu(cpu, tmpmask)
-			per_cpu(cpu_closid, cpu) = 0;
+		rdt_update_percpu_closid(tmpmask, rdtgroup_default.closid);
 	}
 
 	/*
@@ -251,8 +278,7 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 				continue;
 			cpumask_andnot(&r->cpu_mask, &r->cpu_mask, tmpmask);
 		}
-		for_each_cpu(cpu, tmpmask)
-			per_cpu(cpu_closid, cpu) = rdtgrp->closid;
+		rdt_update_percpu_closid(tmpmask, rdtgroup_default.closid);
 	}
 
 	/* Done pushing/pulling - update this group with new mask */
@@ -781,25 +807,12 @@ static int reset_all_cbms(struct rdt_resource *r)
 }
 
 /*
- * MSR_IA32_PQR_ASSOC is scoped per logical CPU, so all updates
- * are always in thread context.
- */
-static void rdt_reset_pqr_assoc_closid(void *v)
-{
-	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
-
-	state->closid = 0;
-	wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, 0);
-}
-
-/*
  * Forcibly remove all of subdirectories under root.
  */
 static void rmdir_all_sub(void)
 {
 	struct rdtgroup *rdtgrp, *tmp;
 	struct task_struct *p, *t;
-	int cpu;
 
 	/* move all tasks to default resource group */
 	read_lock(&tasklist_lock);
@@ -807,14 +820,6 @@ 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();
-
 	list_for_each_entry_safe(rdtgrp, tmp, &rdt_all_groups, rdtgroup_list) {
 		/* Remove each rdtgroup other than root */
 		if (rdtgrp == &rdtgroup_default)
@@ -828,15 +833,13 @@ static void rmdir_all_sub(void)
 		cpumask_or(&rdtgroup_default.cpu_mask,
 			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
 
+		rdt_update_percpu_closid(&rdtgrp->cpu_mask,
+					 rdtgroup_default.closid);
+
 		kernfs_remove(rdtgrp->kn);
 		list_del(&rdtgrp->rdtgroup_list);
 		kfree(rdtgrp);
 	}
-
-	/* Reset all per cpu closids to the default value */
-	for_each_cpu(cpu, &rdtgroup_default.cpu_mask)
-		per_cpu(cpu_closid, cpu) = 0;
-
 	kernfs_remove(kn_info);
 }
 
@@ -943,7 +946,6 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 {
 	struct task_struct *p, *t;
 	struct rdtgroup *rdtgrp;
-	int cpu, ret = 0;
 
 	rdtgrp = rdtgroup_kn_lock_live(kn);
 	if (!rdtgrp) {
@@ -962,8 +964,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 	/* Give any CPUs back to the default group */
 	cpumask_or(&rdtgroup_default.cpu_mask,
 		   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
-	for_each_cpu(cpu, &rdtgrp->cpu_mask)
-		per_cpu(cpu_closid, cpu) = 0;
+	rdt_update_percpu_closid(&rdtgrp->cpu_mask, rdtgroup_default.closid);
 
 	rdtgrp->flags = RDT_DELETED;
 	closid_free(rdtgrp->closid);
@@ -977,8 +978,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 	kernfs_remove(rdtgrp->kn);
 
 	rdtgroup_kn_unlock(kn);
-
-	return ret;
+	return 0;
 }
 
 static struct kernfs_syscall_ops rdtgroup_kf_syscall_ops = {

[tip:x86/cache] x86/intel_rdt: Protect info directory from removal

[tip-bot for Fenghua Yu]

Commit-ID:  f57b308728902d9ffade53466e9201e999a870e4
Gitweb:     http://git.kernel.org/tip/f57b308728902d9ffade53466e9201e999a870e4
Author:     Fenghua Yu <fenghua.yu@intel.com>
AuthorDate: Fri, 11 Nov 2016 17:02:36 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 15 Nov 2016 18:35:49 +0100

x86/intel_rdt: Protect info directory from removal

The info directory and the per-resource subdirectories of the info
directory have no reference to a struct rdtgroup in kn->priv. An attempt to
remove one of those directories results in a NULL pointer dereference.

Protect the directories from removal and return -EPERM instead of -ENOENT.

[ tglx: Massaged changelog ]

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Cc: "Ravi V Shankar" <ravi.v.shankar@intel.com>
Cc: "Tony Luck" <tony.luck@intel.com>
Cc: "Sai Prakhya" <sai.praneeth.prakhya@intel.com>
Cc: "Vikas Shivappa" <vikas.shivappa@linux.intel.com>
Cc: "Ingo Molnar" <mingo@elte.hu>
Cc: "H. Peter Anvin" <h.peter.anvin@intel.com>
Link: http://lkml.kernel.org/r/1478912558-55514-1-git-send-email-fenghua.yu@intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 4795880..cff286e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -644,16 +644,29 @@ static int parse_rdtgroupfs_options(char *data)
  */
 static struct rdtgroup *kernfs_to_rdtgroup(struct kernfs_node *kn)
 {
-	if (kernfs_type(kn) == KERNFS_DIR)
-		return kn->priv;
-	else
+	if (kernfs_type(kn) == KERNFS_DIR) {
+		/*
+		 * All the resource directories use "kn->priv"
+		 * to point to the "struct rdtgroup" for the
+		 * resource. "info" and its subdirectories don't
+		 * have rdtgroup structures, so return NULL here.
+		 */
+		if (kn == kn_info || kn->parent == kn_info)
+			return NULL;
+		else
+			return kn->priv;
+	} else {
 		return kn->parent->priv;
+	}
 }
 
 struct rdtgroup *rdtgroup_kn_lock_live(struct kernfs_node *kn)
 {
 	struct rdtgroup *rdtgrp = kernfs_to_rdtgroup(kn);
 
+	if (!rdtgrp)
+		return NULL;
+
 	atomic_inc(&rdtgrp->waitcount);
 	kernfs_break_active_protection(kn);
 
@@ -670,6 +683,9 @@ void rdtgroup_kn_unlock(struct kernfs_node *kn)
 {
 	struct rdtgroup *rdtgrp = kernfs_to_rdtgroup(kn);
 
+	if (!rdtgrp)
+		return;
+
 	mutex_unlock(&rdtgroup_mutex);
 
 	if (atomic_dec_and_test(&rdtgrp->waitcount) &&
@@ -918,7 +934,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 	rdtgrp = rdtgroup_kn_lock_live(kn);
 	if (!rdtgrp) {
 		rdtgroup_kn_unlock(kn);
-		return -ENOENT;
+		return -EPERM;
 	}
 
 	/* Give any tasks back to the default group */