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

Groups > linux.kernel > #1497597 > unrolled thread

[PATCH v3 11/18] x86/intel_rdt: Add basic resctrl filesystem support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 11/18] x86/intel_rdt: Add basic resctrl filesystem support "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-08 01:50 +0200
    Re: [PATCH v3 11/18] x86/intel_rdt: Add basic resctrl filesystem support Nilay Vaish <nilayvaish@gmail.com> - 2016-10-10 00:40 +0200
      Re: [PATCH v3 11/18] x86/intel_rdt: Add basic resctrl filesystem  support "Luck, Tony" <tony.luck@intel.com> - 2016-10-11 01:50 +0200

#1497597 — [PATCH v3 11/18] x86/intel_rdt: Add basic resctrl filesystem support

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

Use kernfs as basis for our user interface filesystem. This patch
supports mount/umount, and one mount parameter "cdp" to enable code/data
prioritization (though all we do at this point is ensure that the system
can support CDP).  The file system is not populated yet in this patch.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/intel_rdt.h         |  20 ++-
 arch/x86/kernel/cpu/Makefile             |   2 +-
 arch/x86/kernel/cpu/intel_rdt.c          |   8 ++
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 236 +++++++++++++++++++++++++++++++
 include/uapi/linux/magic.h               |   1 +
 5 files changed, 265 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c

diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index bad8dc7..f63815c 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -2,6 +2,23 @@
 #define _ASM_X86_INTEL_RDT_H
 
 /**
+ * struct rdtgroup - store rdtgroup's data in resctrl file system.
+ * @kn:				kernfs node
+ * @rdtgroup_list:		linked list for all rdtgroups
+ * @closid:			closid for this rdtgroup
+ */
+struct rdtgroup {
+	struct kernfs_node	*kn;
+	struct list_head	rdtgroup_list;
+	int			closid;
+};
+
+/* List of all resource groups */
+extern struct list_head rdt_all_groups;
+
+int __init rdtgroup_init(void);
+
+/**
  * struct rdt_resource - attributes of an RDT resource
  * @enabled:			Is this feature enabled on this machine
  * @name:			Name to use in "schemata" file
@@ -39,6 +56,7 @@ struct rdt_resource {
 	for (r = rdt_resources_all; r->name; r++) \
 		if (r->enabled)
 
+#define IA32_L3_QOS_CFG		0xc81
 #define IA32_L3_CBM_BASE	0xc90
 
 /**
@@ -72,7 +90,7 @@ extern struct mutex rdtgroup_mutex;
 
 int __init rdtgroup_init(void);
 
-extern struct rdtgroup *rdtgroup_default;
+extern struct rdtgroup rdtgroup_default;
 extern struct rdt_resource rdt_resources_all[];
 
 enum {
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 5a791c8..963c54a 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -34,7 +34,7 @@ obj-$(CONFIG_CPU_SUP_CENTAUR)		+= centaur.o
 obj-$(CONFIG_CPU_SUP_TRANSMETA_32)	+= transmeta.o
 obj-$(CONFIG_CPU_SUP_UMC_32)		+= umc.o
 
-obj-$(CONFIG_INTEL_RDT)	+= intel_rdt.o
+obj-$(CONFIG_INTEL_RDT)	+= intel_rdt.o intel_rdt_rdtgroup.o
 
 obj-$(CONFIG_X86_MCE)			+= mcheck/
 obj-$(CONFIG_MTRR)			+= mtrr/
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 76b7476..f6caabd 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -265,6 +265,14 @@ static int __init intel_rdt_late_init(void)
 	for_each_rdt_resource(r)
 		rdt_max_closid = max(rdt_max_closid, r->max_closid);
 
+	/* limitation of our allocator, but h/w is more limited */
+	if (rdt_max_closid > 32) {
+		pr_warn("Only using 32/%d CLOSIDs\n", rdt_max_closid);
+		rdt_max_closid = 32;
+	}
+
+	rdtgroup_init();
+
 	for_each_rdt_resource(r)
 		pr_info("Intel %s allocation %s detected\n", r->name,
 			r->cdp_capable ? " (with CDP)" : "");
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
new file mode 100644
index 0000000..c99d3a0
--- /dev/null
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -0,0 +1,236 @@
+/*
+ * User interface for Resource Alloction in Resource Director Technology(RDT)
+ *
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Author: Fenghua Yu <fenghua.yu@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * More information about RDT be found in the Intel (R) x86 Architecture
+ * Software Developer Manual.
+ */
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/fs.h>
+#include <linux/sysfs.h>
+#include <linux/kernfs.h>
+#include <linux/slab.h>
+
+#include <uapi/linux/magic.h>
+
+#include <asm/intel_rdt.h>
+
+DEFINE_STATIC_KEY_FALSE(rdt_enable_key);
+struct kernfs_root *rdt_root;
+struct rdtgroup rdtgroup_default;
+LIST_HEAD(rdt_all_groups);
+
+static void l3_qos_cfg_update(void *arg)
+{
+	struct rdt_resource *r = arg;
+
+	wrmsrl(IA32_L3_QOS_CFG, r->cdp_enabled);
+}
+
+static void set_l3_qos_cfg(struct rdt_resource *r)
+{
+	struct list_head *l;
+	struct rdt_domain *d;
+	struct cpumask cpu_mask;
+
+	cpumask_clear(&cpu_mask);
+	list_for_each(l, &r->domains) {
+		d = list_entry(l, struct rdt_domain, list);
+		cpumask_set_cpu(cpumask_any(&d->cpu_mask), &cpu_mask);
+	}
+	smp_call_function_many(&cpu_mask, l3_qos_cfg_update, r, 1);
+}
+
+static int parse_rdtgroupfs_options(char *data, struct rdt_resource *r)
+{
+	char *token, *o = data;
+
+	while ((token = strsep(&o, ",")) != NULL) {
+		if (!*token)
+			return -EINVAL;
+
+		if (!strcmp(token, "cdp"))
+			if (r->enabled && r->cdp_capable)
+				r->cdp_enabled = true;
+	}
+
+	return 0;
+}
+
+static struct dentry *rdt_mount(struct file_system_type *fs_type,
+				int flags, const char *unused_dev_name,
+				void *data)
+{
+	struct dentry *dentry;
+	int ret;
+	bool new_sb;
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
+
+	mutex_lock(&rdtgroup_mutex);
+	/*
+	 * resctrl file system can only be mounted once.
+	 */
+	if (static_branch_unlikely(&rdt_enable_key)) {
+		dentry = ERR_PTR(-EBUSY);
+		goto out;
+	}
+
+	r->cdp_enabled = false;
+	ret = parse_rdtgroupfs_options(data, r);
+	if (ret) {
+		dentry = ERR_PTR(ret);
+		goto out;
+	}
+	if (r->cdp_enabled)
+		r->num_closid = r->max_closid / 2;
+	else
+		r->num_closid = r->max_closid;
+
+	/* Recompute rdt_max_closid because CDP may have changed things. */
+	rdt_max_closid = 0;
+	for_each_rdt_resource(r)
+		rdt_max_closid = max(rdt_max_closid, r->num_closid);
+	if (rdt_max_closid > 32)
+		rdt_max_closid = 32;
+
+	dentry = kernfs_mount(fs_type, flags, rdt_root,
+			      RDTGROUP_SUPER_MAGIC, &new_sb);
+	if (IS_ERR(dentry))
+		goto out;
+	if (!new_sb) {
+		dentry = ERR_PTR(-EINVAL);
+		goto out;
+	}
+	r = &rdt_resources_all[RDT_RESOURCE_L3];
+	if (r->cdp_capable)
+		set_l3_qos_cfg(r);
+	static_branch_enable(&rdt_enable_key);
+
+out:
+	mutex_unlock(&rdtgroup_mutex);
+
+	return dentry;
+}
+
+static void reset_all_cbms(struct rdt_resource *r)
+{
+	struct list_head *l;
+	struct rdt_domain *d;
+	struct msr_param msr_param;
+	struct cpumask cpu_mask;
+	int i;
+
+	cpumask_clear(&cpu_mask);
+	msr_param.res = r;
+	msr_param.low = 0;
+	msr_param.high = r->max_closid;
+
+	list_for_each(l, &r->domains) {
+		d = list_entry(l, struct rdt_domain, list);
+		cpumask_set_cpu(cpumask_any(&d->cpu_mask), &cpu_mask);
+
+		for (i = 0; i < r->max_closid; i++)
+			d->cbm[i] = r->max_cbm;
+	}
+	smp_call_function_many(&cpu_mask, rdt_cbm_update, &msr_param, 1);
+}
+
+static void rdt_kill_sb(struct super_block *sb)
+{
+	struct rdt_resource *r;
+
+	mutex_lock(&rdtgroup_mutex);
+
+	/*Put everything back to default values. */
+	for_each_rdt_resource(r)
+		reset_all_cbms(r);
+	r = &rdt_resources_all[RDT_RESOURCE_L3];
+	if (r->cdp_capable) {
+		r->cdp_enabled = 0;
+		set_l3_qos_cfg(r);
+	}
+
+	static_branch_disable(&rdt_enable_key);
+	kernfs_kill_sb(sb);
+	mutex_unlock(&rdtgroup_mutex);
+}
+
+static struct file_system_type rdt_fs_type = {
+	.name    = "resctrl",
+	.mount   = rdt_mount,
+	.kill_sb = rdt_kill_sb,
+};
+
+static struct kernfs_syscall_ops rdtgroup_kf_syscall_ops = {
+};
+
+static int __init rdtgroup_setup_root(void)
+{
+	int ret;
+
+	rdt_root = kernfs_create_root(&rdtgroup_kf_syscall_ops,
+				      KERNFS_ROOT_CREATE_DEACTIVATED,
+				      &rdtgroup_default);
+	if (IS_ERR(rdt_root))
+		return PTR_ERR(rdt_root);
+
+	mutex_lock(&rdtgroup_mutex);
+
+	rdtgroup_default.closid = 0;
+	list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
+
+	rdtgroup_default.kn = rdt_root->kn;
+	kernfs_activate(rdtgroup_default.kn);
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return ret;
+}
+
+/*
+ * rdtgroup_init - rdtgroup initialization
+ *
+ * Setup resctrl file system including set up root, create mount point,
+ * register rdtgroup filesystem, and initialize files under root directory.
+ *
+ * Return: 0 on success or -errno
+ */
+int __init rdtgroup_init(void)
+{
+	int ret = 0;
+
+	ret = rdtgroup_setup_root();
+	if (ret)
+		return ret;
+
+	ret = sysfs_create_mount_point(fs_kobj, "resctrl");
+	if (ret)
+		goto cleanup_root;
+
+	ret = register_filesystem(&rdt_fs_type);
+	if (ret)
+		goto cleanup_mountpoint;
+
+	return 0;
+
+cleanup_mountpoint:
+	sysfs_remove_mount_point(fs_kobj, "resctrl");
+cleanup_root:
+	kernfs_destroy_root(rdt_root);
+
+	return ret;
+}
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index e398bea..27ef03d 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -57,6 +57,7 @@
 #define CGROUP_SUPER_MAGIC	0x27e0eb
 #define CGROUP2_SUPER_MAGIC	0x63677270
 
+#define RDTGROUP_SUPER_MAGIC	0x7655821
 
 #define STACK_END_MAGIC		0x57AC6E9D
 
-- 
2.5.0

[toc] | [next] | [standalone]

#1498028

On 7 October 2016 at 21:45, Fenghua Yu <fenghua.yu@intel.com> wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
>
> diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
> index bad8dc7..f63815c 100644
> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -2,6 +2,23 @@
>  #define _ASM_X86_INTEL_RDT_H
>
>  /**
> + * struct rdtgroup - store rdtgroup's data in resctrl file system.
> + * @kn:                                kernfs node
> + * @rdtgroup_list:             linked list for all rdtgroups
> + * @closid:                    closid for this rdtgroup
> + */
> +struct rdtgroup {
> +       struct kernfs_node      *kn;
> +       struct list_head        rdtgroup_list;
> +       int                     closid;
> +};
> +
> +/* List of all resource groups */
> +extern struct list_head rdt_all_groups;
> +
> +int __init rdtgroup_init(void);
> +
> +/**
>   * struct rdt_resource - attributes of an RDT resource
>   * @enabled:                   Is this feature enabled on this machine
>   * @name:                      Name to use in "schemata" file
> @@ -39,6 +56,7 @@ struct rdt_resource {
>         for (r = rdt_resources_all; r->name; r++) \
>                 if (r->enabled)
>
> +#define IA32_L3_QOS_CFG                0xc81
>  #define IA32_L3_CBM_BASE       0xc90
>
>  /**
> @@ -72,7 +90,7 @@ extern struct mutex rdtgroup_mutex;
>
>  int __init rdtgroup_init(void);

This statement appears about twenty lines above as well.


> diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> new file mode 100644
> index 0000000..c99d3a0
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@ -0,0 +1,236 @@
> +/*
> + * User interface for Resource Alloction in Resource Director Technology(RDT)
> + *
> + * Copyright (C) 2016 Intel Corporation
> + *
> + * Author: Fenghua Yu <fenghua.yu@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * More information about RDT be found in the Intel (R) x86 Architecture
> + * Software Developer Manual.
> + */
> +
> +#define pr_fmt(fmt)    KBUILD_MODNAME ": " fmt
> +
> +#include <linux/fs.h>
> +#include <linux/sysfs.h>
> +#include <linux/kernfs.h>
> +#include <linux/slab.h>
> +
> +#include <uapi/linux/magic.h>
> +
> +#include <asm/intel_rdt.h>
> +
> +DEFINE_STATIC_KEY_FALSE(rdt_enable_key);
> +struct kernfs_root *rdt_root;
> +struct rdtgroup rdtgroup_default;
> +LIST_HEAD(rdt_all_groups);
> +
> +static void l3_qos_cfg_update(void *arg)
> +{
> +       struct rdt_resource *r = arg;
> +
> +       wrmsrl(IA32_L3_QOS_CFG, r->cdp_enabled);
> +}
> +
> +static void set_l3_qos_cfg(struct rdt_resource *r)
> +{
> +       struct list_head *l;
> +       struct rdt_domain *d;
> +       struct cpumask cpu_mask;
> +
> +       cpumask_clear(&cpu_mask);
> +       list_for_each(l, &r->domains) {
> +               d = list_entry(l, struct rdt_domain, list);
> +               cpumask_set_cpu(cpumask_any(&d->cpu_mask), &cpu_mask);
> +       }
> +       smp_call_function_many(&cpu_mask, l3_qos_cfg_update, r, 1);
> +}
> +
> +static int parse_rdtgroupfs_options(char *data, struct rdt_resource *r)
> +{
> +       char *token, *o = data;
> +
> +       while ((token = strsep(&o, ",")) != NULL) {
> +               if (!*token)
> +                       return -EINVAL;
> +
> +               if (!strcmp(token, "cdp"))
> +                       if (r->enabled && r->cdp_capable)
> +                               r->cdp_enabled = true;
> +       }
> +
> +       return 0;
> +}
> +
> +static struct dentry *rdt_mount(struct file_system_type *fs_type,
> +                               int flags, const char *unused_dev_name,
> +                               void *data)
> +{
> +       struct dentry *dentry;
> +       int ret;
> +       bool new_sb;
> +       struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
> +
> +       mutex_lock(&rdtgroup_mutex);
> +       /*
> +        * resctrl file system can only be mounted once.
> +        */
> +       if (static_branch_unlikely(&rdt_enable_key)) {
> +               dentry = ERR_PTR(-EBUSY);
> +               goto out;
> +       }
> +
> +       r->cdp_enabled = false;
> +       ret = parse_rdtgroupfs_options(data, r);
> +       if (ret) {
> +               dentry = ERR_PTR(ret);
> +               goto out;
> +       }
> +       if (r->cdp_enabled)
> +               r->num_closid = r->max_closid / 2;
> +       else
> +               r->num_closid = r->max_closid;
> +
> +       /* Recompute rdt_max_closid because CDP may have changed things. */
> +       rdt_max_closid = 0;
> +       for_each_rdt_resource(r)
> +               rdt_max_closid = max(rdt_max_closid, r->num_closid);
> +       if (rdt_max_closid > 32)
> +               rdt_max_closid = 32;
> +
> +       dentry = kernfs_mount(fs_type, flags, rdt_root,
> +                             RDTGROUP_SUPER_MAGIC, &new_sb);
> +       if (IS_ERR(dentry))
> +               goto out;
> +       if (!new_sb) {
> +               dentry = ERR_PTR(-EINVAL);
> +               goto out;
> +       }
> +       r = &rdt_resources_all[RDT_RESOURCE_L3];
> +       if (r->cdp_capable)
> +               set_l3_qos_cfg(r);
> +       static_branch_enable(&rdt_enable_key);
> +
> +out:
> +       mutex_unlock(&rdtgroup_mutex);
> +
> +       return dentry;
> +}

I am not too happy with the function above.  I would have expected
that the function above executes the common code and also makes calls
to per resource functions.  The way it has been written as now, it
seems to be mixing up things.

> +
> +static void reset_all_cbms(struct rdt_resource *r)
> +{
> +       struct list_head *l;
> +       struct rdt_domain *d;
> +       struct msr_param msr_param;
> +       struct cpumask cpu_mask;
> +       int i;
> +
> +       cpumask_clear(&cpu_mask);
> +       msr_param.res = r;
> +       msr_param.low = 0;
> +       msr_param.high = r->max_closid;
> +
> +       list_for_each(l, &r->domains) {
> +               d = list_entry(l, struct rdt_domain, list);
> +               cpumask_set_cpu(cpumask_any(&d->cpu_mask), &cpu_mask);
> +
> +               for (i = 0; i < r->max_closid; i++)
> +                       d->cbm[i] = r->max_cbm;
> +       }
> +       smp_call_function_many(&cpu_mask, rdt_cbm_update, &msr_param, 1);
> +}
> +
> +static void rdt_kill_sb(struct super_block *sb)
> +{
> +       struct rdt_resource *r;
> +
> +       mutex_lock(&rdtgroup_mutex);
> +
> +       /*Put everything back to default values. */
> +       for_each_rdt_resource(r)
> +               reset_all_cbms(r);
> +       r = &rdt_resources_all[RDT_RESOURCE_L3];
> +       if (r->cdp_capable) {
> +               r->cdp_enabled = 0;
> +               set_l3_qos_cfg(r);
> +       }
> +
> +       static_branch_disable(&rdt_enable_key);
> +       kernfs_kill_sb(sb);
> +       mutex_unlock(&rdtgroup_mutex);
> +}
> +
> +static struct file_system_type rdt_fs_type = {
> +       .name    = "resctrl",
> +       .mount   = rdt_mount,
> +       .kill_sb = rdt_kill_sb,
> +};
> +
> +static struct kernfs_syscall_ops rdtgroup_kf_syscall_ops = {
> +};
> +
> +static int __init rdtgroup_setup_root(void)
> +{
> +       int ret;
> +
> +       rdt_root = kernfs_create_root(&rdtgroup_kf_syscall_ops,
> +                                     KERNFS_ROOT_CREATE_DEACTIVATED,
> +                                     &rdtgroup_default);
> +       if (IS_ERR(rdt_root))
> +               return PTR_ERR(rdt_root);
> +
> +       mutex_lock(&rdtgroup_mutex);
> +
> +       rdtgroup_default.closid = 0;
> +       list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
> +
> +       rdtgroup_default.kn = rdt_root->kn;
> +       kernfs_activate(rdtgroup_default.kn);
> +
> +       mutex_unlock(&rdtgroup_mutex);
> +
> +       return ret;

You don't set ret anywhere.


--
Nilay

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

#1498564 — Re: [PATCH v3 11/18] x86/intel_rdt: Add basic resctrl filesystem support

On Sun, Oct 09, 2016 at 05:31:25PM -0500, Nilay Vaish wrote:
> > +static struct dentry *rdt_mount(struct file_system_type *fs_type,
> > +                               int flags, const char *unused_dev_name,
> > +                               void *data)
> > +{
> > +       struct dentry *dentry;
> > +       int ret;
> > +       bool new_sb;
> > +       struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
> > +
> > +       mutex_lock(&rdtgroup_mutex);
> > +       /*
> > +        * resctrl file system can only be mounted once.
> > +        */
> > +       if (static_branch_unlikely(&rdt_enable_key)) {
> > +               dentry = ERR_PTR(-EBUSY);
> > +               goto out;
> > +       }
> > +
> > +       r->cdp_enabled = false;
> > +       ret = parse_rdtgroupfs_options(data, r);
> > +       if (ret) {
> > +               dentry = ERR_PTR(ret);
> > +               goto out;
> > +       }
> > +       if (r->cdp_enabled)
> > +               r->num_closid = r->max_closid / 2;
> > +       else
> > +               r->num_closid = r->max_closid;
> > +
> > +       /* Recompute rdt_max_closid because CDP may have changed things. */
> > +       rdt_max_closid = 0;
> > +       for_each_rdt_resource(r)
> > +               rdt_max_closid = max(rdt_max_closid, r->num_closid);
> > +       if (rdt_max_closid > 32)
> > +               rdt_max_closid = 32;
> > +
> > +       dentry = kernfs_mount(fs_type, flags, rdt_root,
> > +                             RDTGROUP_SUPER_MAGIC, &new_sb);
> > +       if (IS_ERR(dentry))
> > +               goto out;
> > +       if (!new_sb) {
> > +               dentry = ERR_PTR(-EINVAL);
> > +               goto out;
> > +       }
> > +       r = &rdt_resources_all[RDT_RESOURCE_L3];
> > +       if (r->cdp_capable)
> > +               set_l3_qos_cfg(r);
> > +       static_branch_enable(&rdt_enable_key);
> > +
> > +out:
> > +       mutex_unlock(&rdtgroup_mutex);
> > +
> > +       return dentry;
> > +}
> 
> I am not too happy with the function above.  I would have expected
> that the function above executes the common code and also makes calls
> to per resource functions.  The way it has been written as now, it
> seems to be mixing up things.

It is a bit muddled.  Below patch is on top of the series, but can
be threaded back into appropriate parts.

1) Moves the r->cdp_enabled = false into parse_rdtgroupfs_options()

2) Move the rdt_max_closid re-computation out of mount() and into closid_init()
   [not in above as it is introduced later].

3) Bonus ... gets rid of global "rdt_max_closid" as really nothing
   outside of closid_init() needs to know what it is.

Better?

-Tony

commit 15a8ef34e2f849cdcb3e7faa72226cf2ecd82780
Author: Tony Luck <tony.luck@intel.com>
Date:   Mon Oct 10 16:18:31 2016 -0700

    Respond to Nilay's comment that the "mount" code is mixed up.

diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index aefa3a655408..1104b214b972 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -136,9 +136,6 @@ enum {
 	RDT_RESOURCE_L3,
 };
 
-/* Maximum CLOSID allowed across all enabled resoources */
-extern int rdt_max_closid;
-
 /* CPUID.(EAX=10H, ECX=ResID=1).EAX */
 union cpuid_0x10_1_eax {
 	struct {
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index e3c397306f1a..5c504abc9239 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -35,8 +35,6 @@
 /* Mutex to protect rdtgroup access. */
 DEFINE_MUTEX(rdtgroup_mutex);
 
-int rdt_max_closid;
-
 DEFINE_PER_CPU_READ_MOSTLY(int, cpu_closid);
 
 #define domain_init(name) LIST_HEAD_INIT(rdt_resources_all[name].domains)
@@ -267,15 +265,6 @@ static int __init intel_rdt_late_init(void)
 	if (ret < 0)
 		return ret;
 
-	for_each_rdt_resource(r)
-		rdt_max_closid = max(rdt_max_closid, r->max_closid);
-
-	/* limitation of our allocator, but h/w is more limited */
-	if (rdt_max_closid > 32) {
-		pr_warn("Only using 32/%d CLOSIDs\n", rdt_max_closid);
-		rdt_max_closid = 32;
-	}
-
 	rdtgroup_init();
 
 	for_each_rdt_resource(r)
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 82f53ad935ca..5978a3742e5e 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -47,6 +47,23 @@ static int closid_free_map;
 
 static void closid_init(void)
 {
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
+	int rdt_max_closid;
+
+	/* Enabling L3 CDP halves the number of CLOSIDs */
+	if (r->cdp_enabled)
+		r->num_closid = r->max_closid / 2;
+	else
+		r->num_closid = r->max_closid;
+
+	/* Compute rdt_max_closid across all resources */
+	rdt_max_closid = 0;
+	for_each_rdt_resource(r)
+		rdt_max_closid = max(rdt_max_closid, r->num_closid);
+	if (rdt_max_closid > 32) {
+		pr_warn_once("Only using 32/%d CLOSIDs\n", rdt_max_closid);
+		rdt_max_closid = 32;
+	}
 	closid_free_map = BIT_MASK(rdt_max_closid) - 1;
 
 	/* CLOSID 0 is always reserved for the default group */
@@ -546,10 +563,12 @@ static void set_l3_qos_cfg(struct rdt_resource *r)
 	smp_call_function_many(&cpu_mask, l3_qos_cfg_update, r, 1);
 }
 
-static int parse_rdtgroupfs_options(char *data, struct rdt_resource *r)
+static int parse_rdtgroupfs_options(char *data)
 {
 	char *token, *o = data;
+	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
 
+	r->cdp_enabled = false;
 	while ((token = strsep(&o, ",")) != NULL) {
 		if (!*token)
 			return -EINVAL;
@@ -617,7 +636,6 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 	struct dentry *dentry;
 	int ret;
 	bool new_sb;
-	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
 
 	mutex_lock(&rdtgroup_mutex);
 	/*
@@ -628,23 +646,11 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 		goto out;
 	}
 
-	r->cdp_enabled = false;
-	ret = parse_rdtgroupfs_options(data, r);
+	ret = parse_rdtgroupfs_options(data);
 	if (ret) {
 		dentry = ERR_PTR(ret);
 		goto out;
 	}
-	if (r->cdp_enabled)
-		r->num_closid = r->max_closid / 2;
-	else
-		r->num_closid = r->max_closid;
-
-	/* Recompute rdt_max_closid because CDP may have changed things. */
-	rdt_max_closid = 0;
-	for_each_rdt_resource(r)
-		rdt_max_closid = max(rdt_max_closid, r->num_closid);
-	if (rdt_max_closid > 32)
-		rdt_max_closid = 32;
 	closid_init();
 
 	dentry = kernfs_mount(fs_type, flags, rdt_root,
@@ -655,9 +661,8 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 		dentry = ERR_PTR(-EINVAL);
 		goto out;
 	}
-	r = &rdt_resources_all[RDT_RESOURCE_L3];
-	if (r->cdp_capable)
-		set_l3_qos_cfg(r);
+	if (rdt_resources_all[RDT_RESOURCE_L3].cdp_capable)
+		set_l3_qos_cfg(&rdt_resources_all[RDT_RESOURCE_L3]);
 	static_branch_enable(&rdt_enable_key);
 
 out:

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web