Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1501214 > unrolled thread

[PATCH v4 00/18] Intel Cache Allocation Technology

Started by"Fenghua Yu" <fenghua.yu@intel.com>
First post2016-10-15 01:20 +0200
Last post2016-10-17 13:10 +0200
Articles 9 on this page of 49 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v4 00/18] Intel Cache Allocation Technology "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
    [PATCH v4 02/18] cacheinfo: Introduce cache id "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 02/18] cacheinfo: Introduce cache id Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 12:40 +0200
    [PATCH v4 16/18] x86/intel_rdt: Add schemata file "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 16/18] x86/intel_rdt: Add schemata file Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 00:40 +0200
    [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 23:20 +0200
        Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 00:00 +0200
          Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:00 +0200
            RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:10 +0200
              RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 01:20 +0200
                RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:30 +0200
            RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 01:10 +0200
        Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Fenghua Yu <fenghua.yu@intel.com> - 2016-10-18 00:20 +0200
          Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:30 +0200
            Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 01:40 +0200
              Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Fenghua Yu <fenghua.yu@intel.com> - 2016-10-18 02:00 +0200
              Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 12:50 +0200
    [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 15:50 +0200
        Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID Fenghua Yu <fenghua.yu@intel.com> - 2016-10-17 17:10 +0200
          Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID "Luck, Tony" <tony.luck@intel.com> - 2016-10-17 18:40 +0200
            RE: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID "Yu, Fenghua" <fenghua.yu@intel.com> - 2016-10-17 18:50 +0200
              Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID "Luck, Tony" <tony.luck@intel.com> - 2016-10-17 22:30 +0200
            Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 19:10 +0200
          Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 19:10 +0200
          Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 19:10 +0200
            Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters  from CPUID Fenghua Yu <fenghua.yu@intel.com> - 2016-10-17 20:20 +0200
    [PATCH v4 01/18] Documentation, ABI: Add a document entry for cache id "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 01/18] Documentation, ABI: Add a document entry for  cache id Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 12:40 +0200
    [PATCH v4 18/18] MAINTAINERS: Add maintainer for Intel RDT resource allocation "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
    [PATCH v4 04/18] x86/intel_rdt: Feature discovery "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
    [PATCH v4 11/18] x86/intel_rdt: Add basic resctrl filesystem support "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 11/18] x86/intel_rdt: Add basic resctrl filesystem  support Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 21:40 +0200
    [PATCH v4 05/18] Documentation, x86: Documentation for Intel resource allocation user interface "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
    [PATCH v4 17/18] x86/intel_rdt: Add scheduler hook "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
    [PATCH v4 14/18] x86/intel_rdt: Add cpus file "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 14/18] x86/intel_rdt: Add cpus file Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 23:40 +0200
    [PATCH v4 12/18] x86/intel_rdt: Add "info" files to resctrl file system "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 12/18] x86/intel_rdt: Add "info" files to resctrl file  system Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 21:50 +0200
    [PATCH v4 15/18] x86/intel_rdt: Add tasks files "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 15/18] x86/intel_rdt: Add tasks files Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 00:10 +0200
        Re: [PATCH v4 15/18] x86/intel_rdt: Add tasks files "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 00:20 +0200
    [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource  based on cache topology Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 16:50 +0200
    [PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86 "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86 Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 13:00 +0200
    [PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
      Re: [PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 13:10 +0200

Page 3 of 3 — ← Prev page 1 2 [3]


#1501227 — [PATCH v4 15/18] x86/intel_rdt: Add tasks files

From"Fenghua Yu" <fenghua.yu@intel.com>
Date2016-10-15 01:20 +0200
Subject[PATCH v4 15/18] x86/intel_rdt: Add tasks files
Message-ID<ssjVE-7Za-35@gated-at.bofh.it>
In reply to#1501214
From: Fenghua Yu <fenghua.yu@intel.com>

The root directory all subdirectories are automatically populated
with a read/write (mode 0644) file named "tasks". When read it will
show all the task IDs assigned to the resource group. Tasks can be
added (one at a time) to a group by writing the task ID to the file.
E.g.

Membership in a resource group is indicated by a new field in the
task_struct "int closid" which holds the CLOSID for each task. The
default resource group uses CLOSID=0 which means that all existing
tasks when the resctrl file system is mounted belong to the default
group.

A resource group cannot be removed while there are tasks assigned
to it.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 173 +++++++++++++++++++++++++++++++
 include/linux/sched.h                    |   3 +
 2 files changed, 176 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index f2d7a3a..bdbe2d1 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -28,6 +28,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/cpu.h>
+#include <linux/task_work.h>
 
 #include <uapi/linux/magic.h>
 
@@ -278,6 +279,152 @@ unlock:
 	return ret ?: nbytes;
 }
 
+struct task_move_callback {
+	struct callback_head work;
+	struct rdtgroup *rdtgrp;
+};
+
+static void move_myself(struct callback_head *head)
+{
+	struct task_move_callback *callback;
+	struct rdtgroup *rdtgrp;
+
+	callback = container_of(head, struct task_move_callback, work);
+	rdtgrp = callback->rdtgrp;
+
+	/* Resource group might have been deleted before process runs */
+	if (atomic_dec_and_test(&rdtgrp->waitcount) &&
+	    (rdtgrp->flags & RDT_DELETED)) {
+		current->closid = 0;
+		kfree(rdtgrp);
+	}
+
+	kfree(callback);
+}
+
+static int __rdtgroup_move_task(struct task_struct *tsk,
+				struct rdtgroup *rdtgrp)
+{
+	struct task_move_callback *callback;
+	int ret;
+
+	callback = kzalloc(sizeof(*callback), GFP_KERNEL);
+	if (!callback)
+		return -ENOMEM;
+	callback->work.func = move_myself;
+	callback->rdtgrp = rdtgrp;
+	atomic_inc(&rdtgrp->waitcount);
+	ret = task_work_add(tsk, &callback->work, true);
+	if (ret) {
+		atomic_dec(&rdtgrp->waitcount);
+		kfree(callback);
+	} else {
+		tsk->closid = rdtgrp->closid;
+	}
+	return ret;
+}
+
+static int rdtgroup_task_write_permission(struct task_struct *task,
+					  struct kernfs_open_file *of)
+{
+	const struct cred *cred = current_cred();
+	const struct cred *tcred = get_task_cred(task);
+	int ret = 0;
+
+	/*
+	 * even if we're attaching all tasks in the thread group, we only
+	 * need to check permissions on one of them.
+	 */
+	if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
+	    !uid_eq(cred->euid, tcred->uid) &&
+	    !uid_eq(cred->euid, tcred->suid))
+		ret = -EPERM;
+
+	put_cred(tcred);
+	return ret;
+}
+
+static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
+			      struct kernfs_open_file *of)
+{
+	struct task_struct *tsk;
+	int ret;
+
+	rcu_read_lock();
+	if (pid) {
+		tsk = find_task_by_vpid(pid);
+		if (!tsk) {
+			ret = -ESRCH;
+			goto out_unlock_rcu;
+		}
+	} else {
+		tsk = current;
+	}
+
+	get_task_struct(tsk);
+	rcu_read_unlock();
+
+	ret = rdtgroup_task_write_permission(tsk, of);
+	if (!ret)
+		ret = __rdtgroup_move_task(tsk, rdtgrp);
+
+	put_task_struct(tsk);
+	return ret;
+
+out_unlock_rcu:
+	rcu_read_unlock();
+	return ret;
+}
+
+static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
+				    char *buf, size_t nbytes, loff_t off)
+{
+	struct rdtgroup *rdtgrp;
+	pid_t pid;
+	int ret = 0;
+
+	if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0)
+		return -EINVAL;
+	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+
+	if (rdtgrp)
+		ret = rdtgroup_move_task(pid, rdtgrp, of);
+	else
+		ret = -ENOENT;
+
+	rdtgroup_kn_unlock(of->kn);
+
+	return ret ?: nbytes;
+}
+
+static void show_rdt_tasks(struct rdtgroup *r, struct seq_file *s)
+{
+	struct task_struct *p;
+
+	rcu_read_lock();
+	for_each_process(p) {
+		if (p->closid == r->closid)
+			seq_printf(s, "%d\n", p->pid);
+	}
+	rcu_read_unlock();
+}
+
+static int rdtgroup_tasks_show(struct kernfs_open_file *of,
+			       struct seq_file *s, void *v)
+{
+	struct rdtgroup *rdtgrp;
+	int ret = 0;
+
+	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+	if (rdtgrp)
+		show_rdt_tasks(rdtgrp, s);
+	else
+		ret = -ENOENT;
+	rdtgroup_kn_unlock(of->kn);
+
+	return ret;
+}
+
 /* Files in each rdtgroup */
 static struct rftype rdtgroup_base_files[] = {
 	{
@@ -288,6 +435,13 @@ static struct rftype rdtgroup_base_files[] = {
 		.seq_show	= rdtgroup_cpus_show,
 	},
 	{
+		.name		= "tasks",
+		.mode		= 0644,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.write		= rdtgroup_tasks_write,
+		.seq_show	= rdtgroup_tasks_show,
+	},
+	{
 		/* NULL terminated */
 	}
 };
@@ -559,6 +713,13 @@ static void rmdir_all_sub(void)
 {
 	struct rdtgroup *rdtgrp;
 	struct list_head *l, *next;
+	struct task_struct *p;
+
+	/* move all tasks to default resource group */
+	read_lock(&tasklist_lock);
+	for_each_process(p)
+		p->closid = 0;
+	read_unlock(&tasklist_lock);
 
 	get_cpu();
 	/* Reset PQR_ASSOC MSR on this cpu. */
@@ -681,6 +842,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 {
 	struct rdtgroup *rdtgrp;
 	int ret = 0;
+	struct task_struct *p;
 
 	rdtgrp = rdtgroup_kn_lock_live(kn);
 	if (!rdtgrp) {
@@ -698,6 +860,17 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
 		return -EPERM;
 	}
 
+	/* Don't allow if there are processes in this group */
+	read_lock(&tasklist_lock);
+	for_each_process(p) {
+		if (p->closid == rdtgrp->closid) {
+			read_unlock(&tasklist_lock);
+			rdtgroup_kn_unlock(kn);
+			return -EBUSY;
+		}
+	}
+	read_unlock(&tasklist_lock);
+
 	/* Give any CPUs back to the default group */
 	cpumask_or(&rdtgroup_default.cpu_mask,
 		   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 62c68e5..8a05c46 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1766,6 +1766,9 @@ struct task_struct {
 	/* cg_list protected by css_set_lock and tsk->alloc_lock */
 	struct list_head cg_list;
 #endif
+#ifdef CONFIG_INTEL_RDT_A
+	int closid;
+#endif
 #ifdef CONFIG_FUTEX
 	struct robust_list_head __user *robust_list;
 #ifdef CONFIG_COMPAT
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1502505 — Re: [PATCH v4 15/18] x86/intel_rdt: Add tasks files

FromThomas Gleixner <tglx@linutronix.de>
Date2016-10-18 00:10 +0200
SubjectRe: [PATCH v4 15/18] x86/intel_rdt: Add tasks files
Message-ID<stogy-1lR-3@gated-at.bofh.it>
In reply to#1501227
On Fri, 14 Oct 2016, Fenghua Yu wrote:
> +struct task_move_callback {
> +	struct callback_head work;
> +	struct rdtgroup *rdtgrp;

Please align the struct members as you did everywhere else already.

> +};
> +
> +static void move_myself(struct callback_head *head)
> +{
> +	struct task_move_callback *callback;
> +	struct rdtgroup *rdtgrp;
> +
> +	callback = container_of(head, struct task_move_callback, work);
> +	rdtgrp = callback->rdtgrp;
> +
> +	/* Resource group might have been deleted before process runs */

  	/*
	 * If resource group was deleted before this task work callback
	 * was invoked, then assign the task to root group and free the
	 * resource group,
	 */

> +	if (atomic_dec_and_test(&rdtgrp->waitcount) &&
> +	    (rdtgrp->flags & RDT_DELETED)) {
> +		current->closid = 0;
> +		kfree(rdtgrp);
> +	}
> +
> +	kfree(callback);
> +}
> +
> +static int __rdtgroup_move_task(struct task_struct *tsk,
> +				struct rdtgroup *rdtgrp)
> +{
> +	struct task_move_callback *callback;
> +	int ret;
> +
> +	callback = kzalloc(sizeof(*callback), GFP_KERNEL);
> +	if (!callback)
> +		return -ENOMEM;
> +	callback->work.func = move_myself;
> +	callback->rdtgrp = rdtgrp;

Lacks a comment:

  	/*
	 * Take a refcount, so rdtgrp cannot be freed before the
	 * callback has been invoked
	 */

> +	atomic_inc(&rdtgrp->waitcount);
> +	ret = task_work_add(tsk, &callback->work, true);
> +	if (ret) {


Lacks a comment as well:

      		/*
		 * Task is exiting. Drop the refcount and free the callback.
		 * No need to check the refcount as the group cannot be
		 * deleted before the write function unlocks rdtgroup_mutex.
		 */

For you the comment might be obvious, but I had to lookup the world and
some more.

> +		atomic_dec(&rdtgrp->waitcount);
> +		kfree(callback);
> +	} else {
> +		tsk->closid = rdtgrp->closid;
> +	}
> +	return ret;

> +static int rdtgroup_task_write_permission(struct task_struct *task,
> +					  struct kernfs_open_file *of)
> +{
> +	const struct cred *cred = current_cred();
> +	const struct cred *tcred = get_task_cred(task);
> +	int ret = 0;
> +
> +	/*
> +	 * even if we're attaching all tasks in the thread group, we only

Sentences start with an uppercase letter.

> +	 * need to check permissions on one of them.
> +	 */
> +	if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
> +	    !uid_eq(cred->euid, tcred->uid) &&
> +	    !uid_eq(cred->euid, tcred->suid))
> +		ret = -EPERM;
> +
> +	put_cred(tcred);
> +	return ret;
> +}
> +
> +static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp,
> +			      struct kernfs_open_file *of)
> +{
> +	struct task_struct *tsk;
> +	int ret;
> +
> +	rcu_read_lock();
> +	if (pid) {
> +		tsk = find_task_by_vpid(pid);
> +		if (!tsk) {
> +			ret = -ESRCH;
> +			goto out_unlock_rcu;

This goto is pointless as this is the only user,

     	     	        rcu_read_unlock()l
			return -ESRCH;

> +		}
> +	} else {
> +		tsk = current;
> +	}

> @@ -559,6 +713,13 @@ static void rmdir_all_sub(void)
>  {
>  	struct rdtgroup *rdtgrp;
>  	struct list_head *l, *next;
> +	struct task_struct *p;
> +
> +	/* move all tasks to default resource group */
> +	read_lock(&tasklist_lock);
> +	for_each_process(p)
> +		p->closid = 0;
> +	read_unlock(&tasklist_lock);

Ok.

> +	/* Don't allow if there are processes in this group */
> +	read_lock(&tasklist_lock);
> +	for_each_process(p) {
> +		if (p->closid == rdtgrp->closid) {
> +			read_unlock(&tasklist_lock);
> +			rdtgroup_kn_unlock(kn);
> +			return -EBUSY;
> +		}
> +	}
> +	read_unlock(&tasklist_lock);

I wonder, whether we should simply give those tasks back to the default
group, same as we do with the cpus.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1502537 — Re: [PATCH v4 15/18] x86/intel_rdt: Add tasks files

From"Luck, Tony" <tony.luck@intel.com>
Date2016-10-18 00:20 +0200
SubjectRe: [PATCH v4 15/18] x86/intel_rdt: Add tasks files
Message-ID<stoqe-1pY-45@gated-at.bofh.it>
In reply to#1502505
On Tue, Oct 18, 2016 at 12:01:01AM +0200, Thomas Gleixner wrote:
> > +	/* Don't allow if there are processes in this group */
> > +	read_lock(&tasklist_lock);
> > +	for_each_process(p) {
> > +		if (p->closid == rdtgrp->closid) {
> > +			read_unlock(&tasklist_lock);
> > +			rdtgroup_kn_unlock(kn);
> > +			return -EBUSY;
> > +		}
> > +	}
> > +	read_unlock(&tasklist_lock);
> 
> I wonder, whether we should simply give those tasks back to the default
> group, same as we do with the cpus.

Leftover inherited semantics from the cgroup version. It would
simplify the code here (one less error case to handle) if we
did drop this.  I can't come up with a good reason why we'd
want to make the rmdir fail. I can imagine that a sysadmin
dealing with an application that is a fork bomb being happy
about not having to race to move things out so they can do the
rmdir.

-Tony

[toc] | [prev] | [next] | [standalone]


#1501228 — [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology

From"Fenghua Yu" <fenghua.yu@intel.com>
Date2016-10-15 01:20 +0200
Subject[PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology
Message-ID<ssjVE-7Za-31@gated-at.bofh.it>
In reply to#1501214
From: Tony Luck <tony.luck@intel.com>

We use the cpu hotplug notifier to catch each cpu in turn and look at
its cache topology w.r.t each of the resource groups. As we discover
new resources, we initialize the bitmask array for each to the default
(full access) value.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/intel_rdt.h |  31 ++++++++++
 arch/x86/kernel/cpu/intel_rdt.c  | 128 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index 8c61d83..b3df691 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -43,6 +43,35 @@ struct rdt_resource {
 		if (r->enabled)
 
 #define IA32_L3_CBM_BASE	0xc90
+
+/**
+ * struct rdt_domain - group of cpus sharing an RDT resource
+ * @list:	all instances of this resource
+ * @id:		unique id for this instance
+ * @cpu_mask:	which cpus share this resource
+ * @cbm:	array of cache bit masks (indexed by CLOSID)
+ */
+struct rdt_domain {
+	struct list_head	list;
+	int			id;
+	struct cpumask		cpu_mask;
+	u32			*cbm;
+};
+
+/**
+ * struct msr_param - set a range of MSRs from a domain
+ * @res:       The resource to use
+ * @low:       Beginning index from base MSR
+ * @high:      End index
+ */
+struct msr_param {
+	struct rdt_resource	*res;
+	int			low;
+	int			high;
+};
+
+extern struct mutex rdtgroup_mutex;
+
 extern struct rdt_resource rdt_resources_all[];
 
 enum {
@@ -65,4 +94,6 @@ union cpuid_0x10_1_edx {
 	} split;
 	unsigned int full;
 };
+
+void rdt_cbm_update(void *arg);
 #endif /* _ASM_X86_INTEL_RDT_H */
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 87f9650..0b47ea9 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -26,11 +26,16 @@
 
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/cacheinfo.h>
+#include <linux/cpuhotplug.h>
 
 #include <asm/intel_rdt_common.h>
 #include <asm/intel-family.h>
 #include <asm/intel_rdt.h>
 
+/* Mutex to protect rdtgroup access. */
+DEFINE_MUTEX(rdtgroup_mutex);
+
 #define domain_init(name) LIST_HEAD_INIT(rdt_resources_all[name].domains)
 
 struct rdt_resource rdt_resources_all[] = {
@@ -142,13 +147,136 @@ static inline bool get_rdt_resources(void)
 	return ret;
 }
 
+static int get_cache_id(int cpu, int level)
+{
+	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
+	int i;
+
+	for (i = 0; i < ci->num_leaves; i++)
+		if (ci->info_list[i].level == level)
+			return ci->info_list[i].id;
+	return -1;
+}
+
+void rdt_cbm_update(void *arg)
+{
+	struct msr_param *m = (struct msr_param *)arg;
+	struct rdt_resource *r = m->res;
+	struct rdt_domain *d;
+	struct list_head *l;
+	int i, cpu = smp_processor_id();
+
+	list_for_each(l, &r->domains) {
+		d = list_entry(l, struct rdt_domain, list);
+		if (cpumask_test_cpu(cpu, &d->cpu_mask))
+			goto found;
+	}
+	pr_info_once("cpu %d not found in any domain for resource %s\n",
+		     cpu, r->name);
+
+found:
+	for (i = m->low; i < m->high; i++)
+		wrmsrl(r->msr_base + i, d->cbm[i]);
+}
+
+static void update_domain(int cpu, struct rdt_resource *r, int add)
+{
+	struct list_head *l;
+	struct rdt_domain *d;
+	int i, cache_id;
+
+	cache_id = get_cache_id(cpu, r->cache_level);
+
+	if (cache_id == -1) {
+		pr_info_once("Could't find cache id for cpu %d\n", cpu);
+		return;
+	}
+	list_for_each(l, &r->domains) {
+		d = list_entry(l, struct rdt_domain, list);
+		if (cache_id == d->id)
+			goto found;
+		if (cache_id < d->id)
+			break;
+	}
+	if (!add) {
+		pr_info_once("removed unknown cpu %d\n", cpu);
+		return;
+	}
+	d = kzalloc(sizeof(*d), GFP_KERNEL);
+	if (!d)
+		return;
+
+	d->id = cache_id;
+	d->cbm = kmalloc_array(r->max_closid, sizeof(*d->cbm), GFP_KERNEL);
+	if (!d->cbm) {
+		pr_info("Failed to alloc CBM array for cpu %d\n", cpu);
+		kfree(d);
+		return;
+	}
+	cpumask_set_cpu(cpu, &d->cpu_mask);
+	for (i = 0; i < r->max_closid; i++) {
+		d->cbm[i] = r->max_cbm;
+		wrmsrl(r->msr_base + i, d->cbm[i]);
+	}
+	list_add_tail(&d->list, l);
+	r->num_domains++;
+	return;
+
+found:
+	if (add) {
+		cpumask_set_cpu(cpu, &d->cpu_mask);
+	} else {
+		cpumask_clear_cpu(cpu, &d->cpu_mask);
+		if (cpumask_empty(&d->cpu_mask)) {
+			r->num_domains--;
+			kfree(d->cbm);
+			list_del(&d->list);
+			kfree(d);
+		}
+	}
+}
+
+static int intel_rdt_online_cpu(unsigned int cpu)
+{
+	struct rdt_resource *r;
+	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
+
+	mutex_lock(&rdtgroup_mutex);
+	for_each_rdt_resource(r)
+		update_domain(cpu, r, 1);
+	state->closid = 0;
+	wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, 0);
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
+static int intel_rdt_offline_cpu(unsigned int cpu)
+{
+	struct rdt_resource *r;
+
+	mutex_lock(&rdtgroup_mutex);
+	for_each_rdt_resource(r)
+		update_domain(cpu, r, 0);
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
 static int __init intel_rdt_late_init(void)
 {
 	struct rdt_resource *r;
+	int state;
 
 	if (!get_rdt_resources())
 		return -ENODEV;
 
+	state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+				"AP_INTEL_RDT_ONLINE",
+				intel_rdt_online_cpu, intel_rdt_offline_cpu);
+	if (state < 0)
+		return state;
+
 	for_each_rdt_resource(r)
 		pr_info("Intel RDT %s allocation %s detected\n", r->name,
 			r->cdp_capable ? " (with CDP)" : "");
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1502056 — Re: [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology

FromThomas Gleixner <tglx@linutronix.de>
Date2016-10-17 16:50 +0200
SubjectRe: [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology
Message-ID<sthoK-4JB-25@gated-at.bofh.it>
In reply to#1501228
On Fri, 14 Oct 2016, Fenghua Yu wrote:
>  
> +static int get_cache_id(int cpu, int level)
> +{
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
> +	int i;
> +
> +	for (i = 0; i < ci->num_leaves; i++)
> +		if (ci->info_list[i].level == level)
> +			return ci->info_list[i].id;

Multi line statements need curly braces.

> +	return -1;
> +}
> +
> +void rdt_cbm_update(void *arg)
> +{
> +	struct msr_param *m = (struct msr_param *)arg;
> +	struct rdt_resource *r = m->res;
> +	struct rdt_domain *d;
> +	struct list_head *l;
> +	int i, cpu = smp_processor_id();
> +
> +	list_for_each(l, &r->domains) {
> +		d = list_entry(l, struct rdt_domain, list);

We have list_for_each_entry() ....

> +		if (cpumask_test_cpu(cpu, &d->cpu_mask))
> +			goto found;
> +	}
> +	pr_info_once("cpu %d not found in any domain for resource %s\n",
> +		     cpu, r->name);

If the list is empty, then 'd' is not initialized and dereferenced. If the
list is not empty then you use the last domain in the list. Neither one is
the right thing to do....

> +
> +found:
> +	for (i = m->low; i < m->high; i++)
> +		wrmsrl(r->msr_base + i, d->cbm[i]);
> +}
> +
> +static void update_domain(int cpu, struct rdt_resource *r, int add)
> +{
> +	struct list_head *l;
> +	struct rdt_domain *d;
> +	int i, cache_id;

+	struct rdt_domain *d;
+	struct list_head *l;
+	int i, cache_id;

Can you see how this is simpler to read?

> +
> +	cache_id = get_cache_id(cpu, r->cache_level);

> +	if (cache_id == -1) {
> +		pr_info_once("Could't find cache id for cpu %d\n", cpu);
> +		return;
> +	}

Please seperate this by a empty line.

> +	list_for_each(l, &r->domains) {
> +		d = list_entry(l, struct rdt_domain, list);

list_for_each_entry() once more.

> +		if (cache_id == d->id)
> +			goto found;
> +		if (cache_id < d->id)
> +			break;
> +	}
> +	if (!add) {
> +		pr_info_once("removed unknown cpu %d\n", cpu);

_once? Why should this happen at all?

> +		return;
> +	}
> +	d = kzalloc(sizeof(*d), GFP_KERNEL);

Shouldn't this be kzalloc_node() ?

> +	if (!d)
> +		return;
> +
> +	d->id = cache_id;
> +	d->cbm = kmalloc_array(r->max_closid, sizeof(*d->cbm), GFP_KERNEL);
> +	if (!d->cbm) {
> +		pr_info("Failed to alloc CBM array for cpu %d\n", cpu);
> +		kfree(d);
> +		return;
> +	}
> +	cpumask_set_cpu(cpu, &d->cpu_mask);
> +	for (i = 0; i < r->max_closid; i++) {
> +		d->cbm[i] = r->max_cbm;
> +		wrmsrl(r->msr_base + i, d->cbm[i]);
> +	}
> +	list_add_tail(&d->list, l);
> +	r->num_domains++;
> +	return;
> +
> +found:
> +	if (add) {
> +		cpumask_set_cpu(cpu, &d->cpu_mask);

Gah. Reusing this function for add and del is just a silly optimization
which makes the code harder to read.

All you share is the find thing. So you can split that out into a helper:

static struct rdt_domain *rdt_find_domain(struct rdt_resource *r,
					  unsigned int cpu, int id)
{
	if (id < 0)
		return ERR_PTR(id);

	list_for_each_entry(d, &r->domains, list) {
		if (d->id == cache_id)
			return d;
	}
	return NULL;
}

and use it for seperate add/del functions.

static void domain_add_cpu(unsigned int cpu, struct rdt_resource *r)
{
	int id = get_cache_id(cpu, r->cache_level);
	struct rdt_domain *d;

	d = rdt_find_domain(r, cpu, id);
	if (IS_ERR(d)) {
		pr_warn("Print some useful information\n", r->cache_level, cpu, ....);
		return;
	}

	if (d) {
		cpumask_set_cpu(cpu, &d->cpu_mask);
		return;
	}

   	/* Do the allocaction stuff */
}

static void domain_remove_cpu(unsigned int cpu, struct rdt_resource *r)
{
	int id = get_cache_id(cpu, r->cache_level);
	struct rdt_domain *d;

	d = rdt_find_domain(r, cpu, id);
	if (IS_ERR_OR_NULL(d)) {
		pr_warn("Print some useful information\n", r->cache_level, cpu, ...);
		return;
	}

	cpumask_clear_cpu(cpu, &d->cpu_mask);
	....	
}

Hmm?

>  static int __init intel_rdt_late_init(void)
>  {
>  	struct rdt_resource *r;
> +	int state;
>  
>  	if (!get_rdt_resources())
>  		return -ENODEV;
>  
> +	state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> +				"AP_INTEL_RDT_ONLINE",

Please use: "x86/rdt/cat:online:" or similar. We messed that up in a few
places when adding the names, but we don't want to add more of this.

> +				intel_rdt_online_cpu, intel_rdt_offline_cpu);

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1501229 — [PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86

From"Fenghua Yu" <fenghua.yu@intel.com>
Date2016-10-15 01:20 +0200
Subject[PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86
Message-ID<ssjVE-7Za-39@gated-at.bofh.it>
In reply to#1501214
From: Fenghua Yu <fenghua.yu@intel.com>

Cache id is retrieved from APIC ID and CPUID leaf 4 on x86.

For more details see the section on "Cache ID Extraction Parameters" in
"Intel 64 Architecture Processor Topology Enumeration" at
https://software.intel.com/sites/default/files/63/1a/Kuo_CpuTopology_rc1.rh1.final.pdf

Also "Intel 64 and IA-32 Architectures Software Developer's Manual" volume 2,
table 3-18 "information Returned by CPUID Instruction" at
http://www.intel.com/sdm

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/intel_cacheinfo.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index de6626c..8dc5720 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -153,6 +153,7 @@ struct _cpuid4_info_regs {
 	union _cpuid4_leaf_eax eax;
 	union _cpuid4_leaf_ebx ebx;
 	union _cpuid4_leaf_ecx ecx;
+	unsigned int id;
 	unsigned long size;
 	struct amd_northbridge *nb;
 };
@@ -894,6 +895,8 @@ static void __cache_cpumap_setup(unsigned int cpu, int index,
 static void ci_leaf_init(struct cacheinfo *this_leaf,
 			 struct _cpuid4_info_regs *base)
 {
+	this_leaf->id = base->id;
+	this_leaf->attributes = CACHE_ID;
 	this_leaf->level = base->eax.split.level;
 	this_leaf->type = cache_type_map[base->eax.split.type];
 	this_leaf->coherency_line_size =
@@ -920,6 +923,22 @@ static int __init_cache_level(unsigned int cpu)
 	return 0;
 }
 
+/*
+ * The max shared threads number comes from CPUID.4:EAX[25-14] with input
+ * ECX as cache index. Then right shift apicid by the number's order to get
+ * cache id for this cache node.
+ */
+static void get_cache_id(int cpu, struct _cpuid4_info_regs *id4_regs)
+{
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
+	unsigned long num_threads_sharing;
+	int index_msb;
+
+	num_threads_sharing = 1 + id4_regs->eax.split.num_threads_sharing;
+	index_msb = get_count_order(num_threads_sharing);
+	id4_regs->id = c->apicid >> index_msb;
+}
+
 static int __populate_cache_leaves(unsigned int cpu)
 {
 	unsigned int idx, ret;
@@ -931,6 +950,7 @@ static int __populate_cache_leaves(unsigned int cpu)
 		ret = cpuid4_cache_lookup_regs(idx, &id4_regs);
 		if (ret)
 			return ret;
+		get_cache_id(cpu, &id4_regs);
 		ci_leaf_init(this_leaf++, &id4_regs);
 		__cache_cpumap_setup(cpu, idx, &id4_regs);
 	}
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1501870 — Re: [PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86

FromThomas Gleixner <tglx@linutronix.de>
Date2016-10-17 13:00 +0200
SubjectRe: [PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86
Message-ID<stdOa-2ot-7@gated-at.bofh.it>
In reply to#1501229
On Fri, 14 Oct 2016, Fenghua Yu wrote:

> Subject: x86, intel_cacheinfo: Enable cache id in x86

That should be:

  x86/intel_cacheinfo: Enable cache id in cache info

> Cache id is retrieved from APIC ID and CPUID leaf 4 on x86.
> 
> For more details see the section on "Cache ID Extraction Parameters" in
> "Intel 64 Architecture Processor Topology Enumeration" at
> https://software.intel.com/sites/default/files/63/1a/Kuo_CpuTopology_rc1.rh1.final.pdf

That link is going to be stale before this hits Linus tree.

> Also "Intel 64 and IA-32 Architectures Software Developer's Manual" volume 2,
> table 3-18 "information Returned by CPUID Instruction" at

   Table 3-18 FADD/FADDP/FIADD Results ...

The correct one is:

   Table 3-8 Information Returned by CPUID Instruction ...

> http://www.intel.com/sdm

So can we please just say:

  For more details please see the documentation of the CPUID instruction in
  the "Intel 64 and IA-32 Architectures Software Developer's Manual".

and be done with it?

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1501230 — [PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery

From"Fenghua Yu" <fenghua.yu@intel.com>
Date2016-10-15 01:20 +0200
Subject[PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery
Message-ID<ssjVE-7Za-37@gated-at.bofh.it>
In reply to#1501214
From: Fenghua Yu <fenghua.yu@intel.com>

Some Haswell generation CPUs support RDT, but they don't enumerate this
using CPUID.  Use rdmsr_safe() and wrmsr_safe() to probe the MSRs on
cpu model 63 (INTEL_FAM6_HASWELL_X)

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/events/intel/cqm.c             |  2 +-
 arch/x86/include/asm/intel_rdt.h        |  6 +++++
 arch/x86/include/asm/intel_rdt_common.h |  6 +++++
 arch/x86/kernel/cpu/intel_rdt.c         | 41 +++++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/include/asm/intel_rdt.h
 create mode 100644 arch/x86/include/asm/intel_rdt_common.h

diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
index 8f82b02..df86874 100644
--- a/arch/x86/events/intel/cqm.c
+++ b/arch/x86/events/intel/cqm.c
@@ -7,9 +7,9 @@
 #include <linux/perf_event.h>
 #include <linux/slab.h>
 #include <asm/cpu_device_id.h>
+#include <asm/intel_rdt_common.h>
 #include "../perf_event.h"
 
-#define MSR_IA32_PQR_ASSOC	0x0c8f
 #define MSR_IA32_QM_CTR		0x0c8e
 #define MSR_IA32_QM_EVTSEL	0x0c8d
 
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
new file mode 100644
index 0000000..3aca86d
--- /dev/null
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_X86_INTEL_RDT_H
+#define _ASM_X86_INTEL_RDT_H
+
+#define IA32_L3_CBM_BASE	0xc90
+
+#endif /* _ASM_X86_INTEL_RDT_H */
diff --git a/arch/x86/include/asm/intel_rdt_common.h b/arch/x86/include/asm/intel_rdt_common.h
new file mode 100644
index 0000000..e6e15cf
--- /dev/null
+++ b/arch/x86/include/asm/intel_rdt_common.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_X86_INTEL_RDT_COMMON_H
+#define _ASM_X86_INTEL_RDT_COMMON_H
+
+#define MSR_IA32_PQR_ASSOC	0x0c8f
+
+#endif /* _ASM_X86_INTEL_RDT_COMMON_H */
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 7d7aebe..9d55942 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -27,10 +27,51 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 
+#include <asm/intel_rdt_common.h>
+#include <asm/intel-family.h>
+#include <asm/intel_rdt.h>
+
+/*
+ * cache_alloc_hsw_probe() - Have to probe for Intel haswell server CPUs
+ * as they do not have CPUID enumeration support for Cache allocation.
+ * The check for Vendor/Family/Model is not enough to guarantee that
+ * the MSRs won't #GP fault because only the following SKUs support
+ * CAT:
+ *	Intel(R) Xeon(R)  CPU E5-2658  v3  @  2.20GHz
+ *	Intel(R) Xeon(R)  CPU E5-2648L v3  @  1.80GHz
+ *	Intel(R) Xeon(R)  CPU E5-2628L v3  @  2.00GHz
+ *	Intel(R) Xeon(R)  CPU E5-2618L v3  @  2.30GHz
+ *	Intel(R) Xeon(R)  CPU E5-2608L v3  @  2.00GHz
+ *	Intel(R) Xeon(R)  CPU E5-2658A v3  @  2.20GHz
+ *
+ * Probe by trying to write the first of the L3 cach mask registers
+ * and checking that the bits stick. Max CLOSids is always 4 and max cbm length
+ * is always 20 on hsw server parts. The minimum cache bitmask length
+ * allowed for HSW server is always 2 bits. Hardcode all of them.
+ */
+static inline bool cache_alloc_hsw_probe(void)
+{
+	u32 l, h;
+	u32 max_cbm = BIT_MASK(20) - 1;
+
+	if (wrmsr_safe(IA32_L3_CBM_BASE, max_cbm, 0))
+		return false;
+	rdmsr(IA32_L3_CBM_BASE, l, h);
+	if (l != max_cbm)
+		return false;
+
+	return true;
+}
+
 static inline bool get_rdt_resources(void)
 {
 	bool ret = false;
 
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+	    boot_cpu_data.x86 == 6 &&
+	    boot_cpu_data.x86_model == INTEL_FAM6_HASWELL_X)
+		return cache_alloc_hsw_probe();
+
 	if (!boot_cpu_has(X86_FEATURE_RDT_A))
 		return false;
 	if (boot_cpu_has(X86_FEATURE_CAT_L3))
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1501878 — Re: [PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery

FromThomas Gleixner <tglx@linutronix.de>
Date2016-10-17 13:10 +0200
SubjectRe: [PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery
Message-ID<stdXQ-2H4-25@gated-at.bofh.it>
In reply to#1501230
On Fri, 14 Oct 2016, Fenghua Yu wrote:
> +static inline bool cache_alloc_hsw_probe(void)
> +{
> +	u32 l, h;
> +	u32 max_cbm = BIT_MASK(20) - 1;

Two options here:

+	u32 l, h, max_cbm = BIT_MASK(20) - 1;

or

+	u32 max_cbm = BIT_MASK(20) - 1;
+	u32 l, h;

I personally prefer #1, but I can accept #2 as well. Both are quick to
parse while the one you chose is stopping the reading flow.

> +
> +	if (wrmsr_safe(IA32_L3_CBM_BASE, max_cbm, 0))
> +		return false;
> +	rdmsr(IA32_L3_CBM_BASE, l, h);
> +	if (l != max_cbm)
> +		return false;
> +
> +	return true;

  	return l == max_cbm;

Hmm?

> +}
> +
>  static inline bool get_rdt_resources(void)
>  {
>  	bool ret = false;
>  
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> +	    boot_cpu_data.x86 == 6 &&
> +	    boot_cpu_data.x86_model == INTEL_FAM6_HASWELL_X)
> +		return cache_alloc_hsw_probe();

Can you please stick that model check into the probe function and do:

    	if (cache_alloc_hsw_probe())
		return true;
> +
>  	if (!boot_cpu_has(X86_FEATURE_RDT_A))
>  		return false;
>  	if (boot_cpu_has(X86_FEATURE_CAT_L3))

Thanks,

	tglx

[toc] | [prev] | [standalone]


Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel


csiph-web