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


Groups > linux.kernel > #1675008 > unrolled thread

[PATCH 07/21] x86/intel_rdt/cqm: Add RDT monitoring initialization

Started byVikas Shivappa <vikas.shivappa@linux.intel.com>
First post2017-06-26 21:00 +0200
Last post2017-07-02 11:20 +0200
Articles 2 — 2 participants

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 07/21] x86/intel_rdt/cqm: Add RDT monitoring initialization Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 07/21] x86/intel_rdt/cqm: Add RDT monitoring  initialization Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 11:20 +0200

#1675008 — [PATCH 07/21] x86/intel_rdt/cqm: Add RDT monitoring initialization

FromVikas Shivappa <vikas.shivappa@linux.intel.com>
Date2017-06-26 21:00 +0200
Subject[PATCH 07/21] x86/intel_rdt/cqm: Add RDT monitoring initialization
Message-ID<tWHoR-5Yd-5@gated-at.bofh.it>
Add common data structures for RDT resource monitoring and perform RDT
monitoring related data structure initializations which include setting
up the RMID(Resource monitoring ID) lists and event list which the
resource supports.

[tony: some cleanup to make adding MBM easier later, remove "cqm"
from some names, make some data structure local to intel_rdt_monitor.c
static. Add copyright header]

Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com>
---
 arch/x86/kernel/cpu/Makefile            |   2 +-
 arch/x86/kernel/cpu/intel_rdt.c         |  38 +++++---
 arch/x86/kernel/cpu/intel_rdt.h         |  39 ++++++++
 arch/x86/kernel/cpu/intel_rdt_monitor.c | 161 ++++++++++++++++++++++++++++++++
 4 files changed, 227 insertions(+), 13 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/intel_rdt_monitor.c

diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index a576121..81b0060 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -32,7 +32,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 intel_rdt_rdtgroup.o intel_rdt_schemata.o
+obj-$(CONFIG_INTEL_RDT)	+= intel_rdt.o intel_rdt_rdtgroup.o intel_rdt_schemata.o intel_rdt_monitor.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 59500f9..121eb14 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -55,6 +55,12 @@
  */
 int max_name_width, max_data_width;
 
+/*
+ * Global boolean for rdt_alloc which is true if any
+ * resource allocation is enabled.
+ */
+bool rdt_alloc_enabled;
+
 static void
 mba_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r);
 static void
@@ -230,7 +236,7 @@ static bool rdt_get_mem_config(struct rdt_resource *r)
 	return true;
 }
 
-static void rdt_get_cache_config(int idx, struct rdt_resource *r)
+static void rdt_get_cache_alloc_config(int idx, struct rdt_resource *r)
 {
 	union cpuid_0x10_1_eax eax;
 	union cpuid_0x10_x_edx edx;
@@ -422,7 +428,7 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
 
 	d->id = id;
 
-	if (domain_setup_ctrlval(r, d)) {
+	if (r->alloc_capable && domain_setup_ctrlval(r, d)) {
 		kfree(d);
 		return;
 	}
@@ -513,34 +519,39 @@ static __init void rdt_init_padding(void)
 
 static __init bool get_rdt_resources(void)
 {
-	bool ret = false;
-
 	if (cache_alloc_hsw_probe())
-		return true;
+		rdt_alloc_enabled = true;
 
-	if (!boot_cpu_has(X86_FEATURE_RDT_A))
+	if ((!rdt_alloc_enabled && !boot_cpu_has(X86_FEATURE_RDT_A)) &&
+	    !boot_cpu_has(X86_FEATURE_CQM))
 		return false;
 
+	if (boot_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
+		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID);
+
+	if (rdt_mon_features)
+		rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
+
 	if (boot_cpu_has(X86_FEATURE_CAT_L3)) {
-		rdt_get_cache_config(1, &rdt_resources_all[RDT_RESOURCE_L3]);
+		rdt_get_cache_alloc_config(1, &rdt_resources_all[RDT_RESOURCE_L3]);
 		if (boot_cpu_has(X86_FEATURE_CDP_L3)) {
 			rdt_get_cdp_l3_config(RDT_RESOURCE_L3DATA);
 			rdt_get_cdp_l3_config(RDT_RESOURCE_L3CODE);
 		}
-		ret = true;
+		rdt_alloc_enabled = true;
 	}
 	if (boot_cpu_has(X86_FEATURE_CAT_L2)) {
 		/* CPUID 0x10.2 fields are same format at 0x10.1 */
-		rdt_get_cache_config(2, &rdt_resources_all[RDT_RESOURCE_L2]);
-		ret = true;
+		rdt_get_cache_alloc_config(2, &rdt_resources_all[RDT_RESOURCE_L2]);
+		rdt_alloc_enabled = true;
 	}
 
 	if (boot_cpu_has(X86_FEATURE_MBA)) {
 		if (rdt_get_mem_config(&rdt_resources_all[RDT_RESOURCE_MBA]))
-			ret = true;
+			rdt_alloc_enabled = true;
 	}
 
-	return ret;
+	return (rdt_mon_features || rdt_alloc_enabled);
 }
 
 static int __init intel_rdt_late_init(void)
@@ -568,6 +579,9 @@ static int __init intel_rdt_late_init(void)
 	for_each_alloc_capable_rdt_resource(r)
 		pr_info("Intel RDT %s allocation detected\n", r->name);
 
+	for_each_mon_capable_rdt_resource(r)
+		pr_info("Intel RDT %s monitoring detected\n", r->name);
+
 	return 0;
 }
 
diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index 29630af..285f106 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -12,6 +12,29 @@
 
 #define L3_QOS_CDP_ENABLE	0x01ULL
 
+/*
+ * Event IDs are used to program IA32_QM_EVTSEL before reading event
+ * counter from IA32_QM_CTR
+ */
+#define QOS_L3_OCCUP_EVENT_ID		0x01
+#define QOS_L3_MBM_TOTAL_EVENT_ID	0x02
+#define QOS_L3_MBM_LOCAL_EVENT_ID	0x03
+
+/**
+ * struct mon_evt - Entry in the event list of a resource
+ * @evtid:		event id
+ * @name:		name of the event
+ */
+struct mon_evt {
+	u32			evtid;
+	char			*name;
+	struct list_head	list;
+};
+
+extern unsigned int intel_cqm_threshold;
+extern bool rdt_alloc_enabled;
+extern int rdt_mon_features;
+
 /**
  * struct rdtgroup - store rdtgroup's data in resctrl file system.
  * @kn:				kernfs node
@@ -136,7 +159,9 @@ struct rdt_membw {
 /**
  * struct rdt_resource - attributes of an RDT resource
  * @alloc_enabled:	Is allocation enabled on this machine
+ * @mon_enabled:		Is monitoring enabled for this feature
  * @alloc_capable:	Is allocation available on this machine
+ * @mon_capable:		Is monitor feature available on this machine
  * @name:		Name to use in "schemata" file
  * @num_closid:		Number of CLOSIDs available
  * @cache_level:	Which cache level defines scope of this resource
@@ -150,10 +175,15 @@ struct rdt_membw {
  * @nr_info_files:	Number of info files
  * @format_str:		Per resource format string to show domain value
  * @parse_ctrlval:	Per resource function pointer to parse control values
+ * @evt_list:			List of monitoring events
+ * @num_rmid:			Number of RMIDs available
+ * @mon_scale:			cqm counter * mon_scale = occupancy in bytes
  */
 struct rdt_resource {
 	bool			alloc_enabled;
+	bool			mon_enabled;
 	bool			alloc_capable;
+	bool			mon_capable;
 	char			*name;
 	int			num_closid;
 	int			cache_level;
@@ -170,6 +200,9 @@ struct rdt_resource {
 	const char		*format_str;
 	int (*parse_ctrlval)	(char *buf, struct rdt_resource *r,
 				 struct rdt_domain *d);
+	struct list_head	evt_list;
+	int			num_rmid;
+	unsigned int		mon_scale;
 };
 
 void rdt_get_cache_infofile(struct rdt_resource *r);
@@ -201,6 +234,11 @@ enum {
 	     r++)							      \
 		if (r->alloc_capable)
 
+#define for_each_mon_capable_rdt_resource(r)				      \
+	for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
+	     r++)							      \
+		if (r->mon_capable)
+
 #define for_each_alloc_enabled_rdt_resource(r)				      \
 	for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
 	     r++)							      \
@@ -239,5 +277,6 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
 				char *buf, size_t nbytes, loff_t off);
 int rdtgroup_schemata_show(struct kernfs_open_file *of,
 			   struct seq_file *s, void *v);
+void rdt_get_mon_l3_config(struct rdt_resource *r);
 
 #endif /* _ASM_X86_INTEL_RDT_H */
diff --git a/arch/x86/kernel/cpu/intel_rdt_monitor.c b/arch/x86/kernel/cpu/intel_rdt_monitor.c
new file mode 100644
index 0000000..a418854
--- /dev/null
+++ b/arch/x86/kernel/cpu/intel_rdt_monitor.c
@@ -0,0 +1,161 @@
+/*
+ * Resource Director Technology(RDT)
+ * - Monitoring code
+ *
+ * Copyright (C) 2017 Intel Corporation
+ *
+ * Author:
+ *    Vikas Shivappa <vikas.shivappa@intel.com>
+ *
+ * This replaces the cqm.c based on perf but we reuse a lot of
+ * code and datastructures originally from Peter Zijlstra and Matt Fleming.
+ *
+ * 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 June 2016, volume 3, section 17.17.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <asm/cpu_device_id.h>
+#include "intel_rdt.h"
+
+enum rmid_recycle_state {
+	RMID_CHECK = 0,
+	RMID_DIRTY,
+};
+
+struct rmid_entry {
+	u32 rmid;
+	enum rmid_recycle_state state;
+	struct list_head list;
+};
+
+/**
+ * @rmid_free_lru    A least recently used list of free RMIDs
+ *     These RMIDs are guaranteed to have an occupancy less than the
+ *     threshold occupancy
+ */
+static struct list_head	rmid_free_lru;
+
+/**
+ * @rmid_limbo_lru       list of currently unused but (potentially)
+ *     dirty RMIDs.
+ *     This list contains RMIDs that no one is currently using but that
+ *     may have a occupancy value > intel_cqm_threshold. User can change
+ *     the threshold occupancy value.
+ */
+static struct list_head	rmid_limbo_lru;
+
+/**
+ * @rmid_entry - The entry in the limbo and free lists.
+ */
+static struct rmid_entry	*rmid_ptrs;
+
+/*
+ * Global boolean for rdt_monitor which is true if any
+ * resource monitoring is enabled.
+ */
+int rdt_mon_features;
+
+/*
+ * This is the threshold cache occupancy at which we will consider an
+ * RMID available for re-allocation.
+ */
+unsigned int intel_cqm_threshold;
+
+static inline struct rmid_entry *__rmid_entry(u32 rmid)
+{
+	struct rmid_entry *entry;
+
+	entry = &rmid_ptrs[rmid];
+	WARN_ON(entry->rmid != rmid);
+
+	return entry;
+}
+
+static int dom_data_init(struct rdt_resource *r)
+{
+	struct rmid_entry *entry = NULL;
+	int i = 0, nr_rmids;
+
+	INIT_LIST_HEAD(&rmid_free_lru);
+	INIT_LIST_HEAD(&rmid_limbo_lru);
+
+	nr_rmids = r->num_rmid;
+	rmid_ptrs = kcalloc(nr_rmids, sizeof(struct rmid_entry), GFP_KERNEL);
+	if (!rmid_ptrs)
+		return -ENOMEM;
+
+	for (; i < nr_rmids; i++) {
+		entry = &rmid_ptrs[i];
+		INIT_LIST_HEAD(&entry->list);
+
+		entry->rmid = i;
+		list_add_tail(&entry->list, &rmid_free_lru);
+	}
+
+	/*
+	 * RMID 0 is special and is always allocated. It's used for all
+	 * tasks that are not monitored.
+	 */
+	entry = __rmid_entry(0);
+	list_del(&entry->list);
+
+	return 0;
+}
+
+static struct mon_evt llc_occupancy_event = {
+	.name = "llc_occupancy",
+	.evtid = QOS_L3_OCCUP_EVENT_ID,
+};
+
+static void l3_mon_evt_init(struct rdt_resource *r)
+{
+	INIT_LIST_HEAD(&r->evt_list);
+
+	if (rdt_mon_features & (1 << QOS_L3_OCCUP_EVENT_ID))
+		list_add_tail(&llc_occupancy_event.list, &r->evt_list);
+}
+
+void rdt_get_mon_l3_config(struct rdt_resource *r)
+{
+	int ret;
+
+	r->mon_scale = boot_cpu_data.x86_cache_occ_scale;
+	r->num_rmid = boot_cpu_data.x86_cache_max_rmid + 1;
+
+	/*
+	 * A reasonable upper limit on the max threshold is the number
+	 * of lines tagged per RMID if all RMIDs have the same number of
+	 * lines tagged in the LLC.
+	 *
+	 * For a 35MB LLC and 56 RMIDs, this is ~1.8% of the LLC.
+	 */
+	intel_cqm_threshold = boot_cpu_data.x86_cache_size * 1024 / r->num_rmid;
+
+	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
+	intel_cqm_threshold /= r->mon_scale;
+
+	ret = dom_data_init(r);
+	if (ret)
+		goto out;
+
+	l3_mon_evt_init(r);
+
+	r->mon_capable = true;
+	r->mon_enabled = true;
+
+	return;
+out:
+	kfree(rmid_ptrs);
+	rdt_mon_features = 0;
+}
-- 
1.9.1

[toc] | [next] | [standalone]


#1679438 — Re: [PATCH 07/21] x86/intel_rdt/cqm: Add RDT monitoring initialization

FromThomas Gleixner <tglx@linutronix.de>
Date2017-07-02 11:20 +0200
SubjectRe: [PATCH 07/21] x86/intel_rdt/cqm: Add RDT monitoring initialization
Message-ID<tYJcR-7V2-5@gated-at.bofh.it>
In reply to#1675008
On Mon, 26 Jun 2017, Vikas Shivappa wrote:
> +/*
> + * Global boolean for rdt_alloc which is true if any
> + * resource allocation is enabled.
> + */
> +bool rdt_alloc_enabled;

That should be rdt_alloc_capable. It's not enabled at probe time. Probing
merily detects the capability. That mirrors the capable/enabled bits in the
rdt resource struct.

>  static void
>  mba_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r);
>  static void
> @@ -230,7 +236,7 @@ static bool rdt_get_mem_config(struct rdt_resource *r)
>  	return true;
>  }
>  
> -static void rdt_get_cache_config(int idx, struct rdt_resource *r)
> +static void rdt_get_cache_alloc_config(int idx, struct rdt_resource *r)
>  {
>  	union cpuid_0x10_1_eax eax;
>  	union cpuid_0x10_x_edx edx;
> @@ -422,7 +428,7 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
>  
>  	d->id = id;
>  
> -	if (domain_setup_ctrlval(r, d)) {
> +	if (r->alloc_capable && domain_setup_ctrlval(r, d)) {

This should be done in the name space cleanup patch or in a separate one.

>  		kfree(d);
>  		return;
>  	}
> @@ -513,34 +519,39 @@ static __init void rdt_init_padding(void)
>  
>  static __init bool get_rdt_resources(void)
>  {
> -	bool ret = false;
> -
>  	if (cache_alloc_hsw_probe())
> -		return true;
> +		rdt_alloc_enabled = true;
>  
> -	if (!boot_cpu_has(X86_FEATURE_RDT_A))
> +	if ((!rdt_alloc_enabled && !boot_cpu_has(X86_FEATURE_RDT_A)) &&
> +	    !boot_cpu_has(X86_FEATURE_CQM))
>  		return false;
>  
> +	if (boot_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
> +		rdt_mon_features |= (1 << QOS_L3_OCCUP_EVENT_ID);

Instead of artificially cramming the CQM bits into this function, it
would be cleaner to leave that function alone, rename it to

      get_rdt_alloc_resources()

and have a new function

      get_rdt_mon_resources()

and handle the aggregation at the call site.

	rdt_alloc_capable = get_rdt_alloc_resources();
	rdt_mon_capable = get_rdt_mon_resources();

	if (!rdt_alloc_capable && !rdt_mon_capable)
		return -ENODEV;

I'd make both variables boolean and have rdt_mon_features as a separate
one, which carries the actual available feature bits. This is neither
hotpath nor are we in a situation where we need to spare the last 4byte of
memory. Clean separation of code and functionality is more important.

> +/*
> + * Event IDs are used to program IA32_QM_EVTSEL before reading event
> + * counter from IA32_QM_CTR
> + */
> +#define QOS_L3_OCCUP_EVENT_ID		0x01
> +#define QOS_L3_MBM_TOTAL_EVENT_ID	0x02
> +#define QOS_L3_MBM_LOCAL_EVENT_ID	0x03
> +
> +/**
> + * struct mon_evt - Entry in the event list of a resource
> + * @evtid:		event id
> + * @name:		name of the event
> + */
> +struct mon_evt {
> +	u32			evtid;
> +	char			*name;
> +	struct list_head	list;
> +};
> +
> +extern unsigned int intel_cqm_threshold;
> +extern bool rdt_alloc_enabled;
> +extern int rdt_mon_features;

Please do not use 'int' for variables which contain bit flags. unsigned int
is the proper choice here.

> +struct rmid_entry {
> +	u32 rmid;
> +	enum rmid_recycle_state state;
> +	struct list_head list;

Please make it tabular as you did with mon_evt and other structs.

> +};
> +
> +/**
> + * @rmid_free_lru    A least recently used list of free RMIDs
> + *     These RMIDs are guaranteed to have an occupancy less than the
> + *     threshold occupancy
> + */
> +static struct list_head	rmid_free_lru;
> +
> +/**
> + * @rmid_limbo_lru       list of currently unused but (potentially)
> + *     dirty RMIDs.
> + *     This list contains RMIDs that no one is currently using but that
> + *     may have a occupancy value > intel_cqm_threshold. User can change
> + *     the threshold occupancy value.
> + */
> +static struct list_head	rmid_limbo_lru;
> +
> +/**
> + * @rmid_entry - The entry in the limbo and free lists.
> + */
> +static struct rmid_entry	*rmid_ptrs;
> +
> +/*
> + * Global boolean for rdt_monitor which is true if any

Boolean !?!?!

> + * resource monitoring is enabled.
> + */
> +int rdt_mon_features;
> +
> +/*
> + * This is the threshold cache occupancy at which we will consider an
> + * RMID available for re-allocation.
> + */
> +unsigned int intel_cqm_threshold;
> +
> +static inline struct rmid_entry *__rmid_entry(u32 rmid)
> +{
> +	struct rmid_entry *entry;
> +
> +	entry = &rmid_ptrs[rmid];
> +	WARN_ON(entry->rmid != rmid);
> +
> +	return entry;
> +}
> +
> +static int dom_data_init(struct rdt_resource *r)
> +{
> +	struct rmid_entry *entry = NULL;
> +	int i = 0, nr_rmids;
> +
> +	INIT_LIST_HEAD(&rmid_free_lru);
> +	INIT_LIST_HEAD(&rmid_limbo_lru);

You can spare that by declaring the list head with

static LIST_HEAD(rmid_xxx_lru);

> +
> +	nr_rmids = r->num_rmid;
> +	rmid_ptrs = kcalloc(nr_rmids, sizeof(struct rmid_entry), GFP_KERNEL);
> +	if (!rmid_ptrs)
> +		return -ENOMEM;
> +
> +	for (; i < nr_rmids; i++) {

Please initialize i in the for() construct. It's really bad to read,
because the missing initialization statement makes one look for a special
initialization magic just to figure out that it's simply i = 0.

> +		entry = &rmid_ptrs[i];
> +		INIT_LIST_HEAD(&entry->list);
> +
> +		entry->rmid = i;
> +		list_add_tail(&entry->list, &rmid_free_lru);
> +	}
> +
> +	/*
> +	 * RMID 0 is special and is always allocated. It's used for all
> +	 * tasks that are not monitored.
> +	 */
> +	entry = __rmid_entry(0);
> +	list_del(&entry->list);
> +
> +	return 0;
> +}
> +
> +static struct mon_evt llc_occupancy_event = {
> +	.name = "llc_occupancy",
> +	.evtid = QOS_L3_OCCUP_EVENT_ID,

Tabluar...

> +};
> +
> +static void l3_mon_evt_init(struct rdt_resource *r)
> +{
> +	INIT_LIST_HEAD(&r->evt_list);
> +
> +	if (rdt_mon_features & (1 << QOS_L3_OCCUP_EVENT_ID))
> +		list_add_tail(&llc_occupancy_event.list, &r->evt_list);

What's that list for? Why don't you have that event as a member of the L3
rdt resource and control it via r->mon_capable/enabled?

> +}
> +
> +void rdt_get_mon_l3_config(struct rdt_resource *r)
> +{
> +	int ret;
> +
> +	r->mon_scale = boot_cpu_data.x86_cache_occ_scale;
> +	r->num_rmid = boot_cpu_data.x86_cache_max_rmid + 1;
> +
> +	/*
> +	 * A reasonable upper limit on the max threshold is the number
> +	 * of lines tagged per RMID if all RMIDs have the same number of
> +	 * lines tagged in the LLC.
> +	 *
> +	 * For a 35MB LLC and 56 RMIDs, this is ~1.8% of the LLC.
> +	 */
> +	intel_cqm_threshold = boot_cpu_data.x86_cache_size * 1024 / r->num_rmid;
> +
> +	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
> +	intel_cqm_threshold /= r->mon_scale;
> +
> +	ret = dom_data_init(r);
> +	if (ret)
> +		goto out;
> +
> +	l3_mon_evt_init(r);
> +
> +	r->mon_capable = true;
> +	r->mon_enabled = true;
> +
> +	return;
> +out:
> +	kfree(rmid_ptrs);
> +	rdt_mon_features = 0;

This is silly. if dom_data_init() fails, then it failed because it was
unable to allocate rmid_ptrs. .....

Also clearing rdt_mod_features here is conceptually wrong. Make that
function return int, i.e. the failure value, and clear rdt_mon_capable at
the call site in case of error.

Thanks,

	tglx


	 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web