All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 05/10] powerpc/perf: IMC pmu cpumask and cpuhotplug support
@ 2017-06-28 18:58 Anju T Sudhakar
  2017-06-28 18:58 ` [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging Anju T Sudhakar
  2017-06-28 18:58 ` [PATCH v11 09/10] powerpc/perf: Thread IMC PMU functions Anju T Sudhakar
  0 siblings, 2 replies; 6+ messages in thread
From: Anju T Sudhakar @ 2017-06-28 18:58 UTC (permalink / raw)
  To: mpe
  Cc: linux-kernel, linuxppc-dev, ego, bsingharora, anton, sukadev,
	mikey, stewart, dja, eranian, hemant, maddy, anju, tglx

Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any            
online CPU) from each chip for nest PMUs is designated to read counters.        
                                                                                
On CPU hotplug, dying CPU is checked to see whether it is one of the            
designated cpus, if yes, next online cpu from the same chip (for nest           
units) is designated as new cpu to read counters. For this purpose, we          
introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE.                  
                                                                                
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>                        
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>                         
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> 
---
 arch/powerpc/include/asm/imc-pmu.h             |   1 +
 arch/powerpc/include/asm/opal-api.h            |  10 +-
 arch/powerpc/include/asm/opal.h                |   4 +
 arch/powerpc/perf/imc-pmu.c                    | 188 ++++++++++++++++++++++++-
 arch/powerpc/platforms/powernv/opal-imc.c      |  18 ++-
 arch/powerpc/platforms/powernv/opal-wrappers.S |   3 +
 include/linux/cpuhotplug.h                     |   1 +
 7 files changed, 219 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
index 25d0c57..650c1e8 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -24,6 +24,7 @@
  * For static allocation of some of the structures.
  */
 #define IMC_MAX_PMUS			32
+#define IMC_MAX_CHIPS			32
 
 /*
  * This macro is used for memory buffer allocation of
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index cb3e624..fdacb03 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -190,7 +190,10 @@
 #define OPAL_NPU_INIT_CONTEXT			146
 #define OPAL_NPU_DESTROY_CONTEXT		147
 #define OPAL_NPU_MAP_LPAR			148
-#define OPAL_LAST				148
+#define OPAL_IMC_COUNTERS_INIT			149
+#define OPAL_IMC_COUNTERS_START			150
+#define OPAL_IMC_COUNTERS_STOP			151
+#define OPAL_LAST				151
 
 /* Device tree flags */
 
@@ -1003,6 +1006,11 @@ enum {
 	XIVE_DUMP_EMU_STATE	= 5,
 };
 
+/* Argument to OPAL_IMC_COUNTERS_*  */
+enum {
+	OPAL_IMC_COUNTERS_NEST = 1,
+};
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __OPAL_API_H */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 588fb1c..48842d2 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -268,6 +268,10 @@ int64_t opal_xive_free_irq(uint32_t girq);
 int64_t opal_xive_sync(uint32_t type, uint32_t id);
 int64_t opal_xive_dump(uint32_t type, uint32_t id);
 
+int64_t opal_imc_counters_init(uint32_t type, uint64_t address, uint64_t cpu);
+int64_t opal_imc_counters_start(uint32_t type, uint64_t cpu_pir);
+int64_t opal_imc_counters_stop(uint32_t type, uint64_t cpu_pir);
+
 /* Internal functions */
 extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
 				   int depth, void *data);
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 326c9ea..b0bcb77 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -20,6 +20,16 @@
 
 /* Needed for sanity check */
 struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
+static cpumask_t nest_imc_cpumask;
+static int nest_imc_cpumask_initialized;
+
+static atomic_t nest_pmus;
+static atomic_t nest_events[IMC_MAX_CHIPS];
+/* Used to avoid races in counting the nest-pmu units inited */
+static DEFINE_MUTEX(imc_nest_inited_reserve);
+/* Used to avoid races in calling enable/disable nest-pmu units */
+static DEFINE_MUTEX(imc_nest_reserve);
+
 
 struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
@@ -43,9 +53,135 @@ static struct attribute_group imc_format_group = {
 	.attrs = nest_imc_format_attrs,
 };
 
+/* Get the cpumask printed to a buffer "buf" */
+static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	cpumask_t *active_mask;
+
+	active_mask = &nest_imc_cpumask;
+	return cpumap_print_to_pagebuf(true, buf, active_mask);
+}
+
+static DEVICE_ATTR(cpumask, S_IRUGO, imc_pmu_cpumask_get_attr, NULL);
+
+static struct attribute *imc_pmu_cpumask_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group imc_pmu_cpumask_attr_group = {
+	.attrs = imc_pmu_cpumask_attrs,
+};
+
+static void nest_change_cpu_context(int old_cpu, int new_cpu)
+{
+	struct imc_pmu **pn = per_nest_pmu_arr;
+	int i;
+
+	if (old_cpu < 0 || new_cpu < 0)
+		return;
+
+	for (i = 0; *pn && i < IMC_MAX_PMUS; i++, pn++)
+		perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
+}
+
+static int ppc_nest_imc_cpu_offline(unsigned int cpu)
+{
+	int nid, target = -1;
+	const struct cpumask *l_cpumask;
+
+	/*
+	 * Check in the designated list for this cpu. Dont bother
+	 * if not one of them.
+	 */
+	if (!cpumask_test_and_clear_cpu(cpu, &nest_imc_cpumask))
+		return 0;
+
+	/*
+	 * Now that this cpu is one of the designated,
+	 * find a next cpu a) which is online and b) in same chip.
+	 */
+	nid = cpu_to_node(cpu);
+	l_cpumask = cpumask_of_node(nid);
+	target = cpumask_any_but(l_cpumask, cpu);
+
+	/*
+	* Update the cpumask with the target cpu and
+	* migrate the context if needed
+	*/
+	if (target >= 0 && target < nr_cpu_ids) {
+		cpumask_set_cpu(target, &nest_imc_cpumask);
+		nest_change_cpu_context(cpu, target);
+	} else {
+		opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST,
+				       get_hard_smp_processor_id(cpu));
+	}
+	return 0;
+}
+
+static int ppc_nest_imc_cpu_online(unsigned int cpu)
+{
+	const struct cpumask *l_cpumask;
+	static struct cpumask tmp_mask;
+	int res;
+
+	/* Get the cpumask of this node */
+	l_cpumask = cpumask_of_node(cpu_to_node(cpu));
+
+	/*
+	 * If this is not the first online CPU on this node, then
+	 * just return.
+	 */
+	if (cpumask_and(&tmp_mask, l_cpumask, &nest_imc_cpumask))
+		return 0;
+
+	/*
+	 * If this is the first online cpu on this node
+	 * disable the nest counters by making an OPAL call.
+	 */
+	res = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST,
+				     get_hard_smp_processor_id(cpu));
+	if (res)
+		return res;
+
+	/* Make this CPU the designated target for counter collection */
+	cpumask_set_cpu(cpu, &nest_imc_cpumask);
+	return 0;
+}
+
+static int nest_pmu_cpumask_init(void)
+{
+	return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE,
+				 "perf/powerpc/imc:online",
+				 ppc_nest_imc_cpu_online,
+				 ppc_nest_imc_cpu_offline);
+}
+
+static void nest_imc_counters_release(struct perf_event *event)
+{
+	int rc, node_id;
+	/*
+	 * See if we need to disable the nest PMU.
+	 * If no events are currently in use, then we have to take a
+	 * mutex to ensure that we don't race with another task doing
+	 * enable or disable the nest counters.
+	 */
+	node_id = cpu_to_node(event->cpu);
+	if (atomic_dec_return(&nest_events[node_id]) == 0) {
+		mutex_lock(&imc_nest_reserve);
+		rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST,
+					    get_hard_smp_processor_id(event->cpu));
+		mutex_unlock(&imc_nest_reserve);
+		if (rc)
+			pr_err("IMC: Disable counters failed for node %d\n", node_id);
+	}
+}
+
 static int nest_imc_event_init(struct perf_event *event)
 {
-	int chip_id;
+	int chip_id, rc, node_id;
 	u32 l_config, config = event->attr.config;
 	struct imc_mem_info *pcni;
 	struct imc_pmu *pmu;
@@ -98,6 +234,25 @@ static int nest_imc_event_init(struct perf_event *event)
 	 */
 	l_config = config & IMC_EVENT_OFFSET_MASK;
 	event->hw.event_base = (u64)pcni->vbase[l_config/PAGE_SIZE] + (config & ~PAGE_MASK);
+	/*
+	* Nest pmu units are enabled only when it is used.
+	* See if this is triggered for the first time.
+	* If yes, take the mutex lock and enable the nest counters.
+	* If not, just increment the count in nest_events.
+	*/
+	node_id = cpu_to_node(event->cpu);
+	if (atomic_inc_return(&nest_events[node_id]) == 1) {
+		mutex_lock(&imc_nest_reserve);
+		rc = opal_imc_counters_start(OPAL_IMC_COUNTERS_NEST,
+					     get_hard_smp_processor_id(event->cpu));
+		mutex_unlock(&imc_nest_reserve);
+		if (rc) {
+			atomic_dec_return(&nest_events[node_id]);
+			pr_err("IMC: Unable to start the counters for node %d\n", node_id);
+			return -ENODEV;
+		}
+	}
+	event->destroy = nest_imc_counters_release;
 	return 0;
 }
 
@@ -175,6 +330,7 @@ static int update_pmu_ops(struct imc_pmu *pmu)
 	pmu->pmu.start = imc_event_start;
 	pmu->pmu.stop = imc_event_stop;
 	pmu->pmu.read = imc_perf_event_update;
+	pmu->attr_groups[IMC_CPUMASK_ATTR] = &imc_pmu_cpumask_attr_group;
 	pmu->attr_groups[IMC_FORMAT_ATTR] = &imc_format_group;
 	pmu->pmu.attr_groups = pmu->attr_groups;
 
@@ -244,12 +400,31 @@ static int update_events_in_group(struct imc_events *events,
  * @events:	events memory for this pmu.
  * @idx:	number of event entries created.
  * @pmu_ptr:	memory allocated for this pmu.
+ *
+ * init_imc_pmu() setup the cpu mask information for these pmus and setup
+ * the state machine hotplug notifiers as well.
  */
 int init_imc_pmu(struct imc_events *events, int idx,
 		 struct imc_pmu *pmu_ptr)
 {
 	int ret = -ENODEV;
-
+	/* Add cpumask and register for hotplug notification */
+	if (atomic_inc_return(&nest_pmus) == 1) {
+		/*
+		 * Nest imc pmu need only one cpu per chip, we initialize the
+		 * cpumask for the first nest imc pmu and use the same for the rest.
+		 * To handle the cpuhotplug callback unregister, we track
+		 * the number of nest pmus registers in "nest_pmus".
+		 * "nest_imc_cpumask_initialized" is set to zero during cpuhotplug
+		 * callback unregister.
+		 */
+		ret = nest_pmu_cpumask_init();
+		if (ret)
+			goto err_free;
+		mutex_lock(&imc_nest_inited_reserve);
+		nest_imc_cpumask_initialized = 1;
+		mutex_unlock(&imc_nest_inited_reserve);
+	}
 	ret = update_events_in_group(events, idx, pmu_ptr);
 	if (ret)
 		goto err_free;
@@ -274,6 +449,13 @@ int init_imc_pmu(struct imc_events *events, int idx,
 			kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]->attrs);
 		kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]);
 	}
-
+	if (pmu_ptr->domain == IMC_DOMAIN_NEST) {
+		if (atomic_dec_return(&nest_pmus) == 0) {
+			mutex_lock(&imc_nest_inited_reserve);
+			cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE);
+			nest_imc_cpumask_initialized = 0;
+			mutex_unlock(&imc_nest_inited_reserve);
+		}
+	}
 	return ret;
 }
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index a68d66d..a970ed3 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -467,6 +467,19 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
 	return ret;
 }
 
+static void disable_nest_counters(void)
+{
+	int nid, cpu;
+	struct cpumask *l_cpumask;
+
+	for_each_online_node(nid) {
+		l_cpumask = cpumask_of_node(nid);
+		cpu = cpumask_first(l_cpumask);
+		opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST,
+				       get_hard_smp_processor_id(cpu));
+	}
+}
+
 static int opal_imc_counters_probe(struct platform_device *pdev)
 {
 	struct device_node *imc_dev = NULL;
@@ -479,9 +492,10 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 	/*
 	 * Check whether this is kdump kernel. If yes, just return.
 	 */
-	if (is_kdump_kernel())
+	if (is_kdump_kernel()) {
+		disable_nest_counters();
 		return -ENODEV;
-
+	}
 	imc_dev = pdev->dev.of_node;
 	if (!imc_dev)
 		return -ENODEV;
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index f620572..1828b24f 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -310,3 +310,6 @@ OPAL_CALL(opal_xive_dump,			OPAL_XIVE_DUMP);
 OPAL_CALL(opal_npu_init_context,		OPAL_NPU_INIT_CONTEXT);
 OPAL_CALL(opal_npu_destroy_context,		OPAL_NPU_DESTROY_CONTEXT);
 OPAL_CALL(opal_npu_map_lpar,			OPAL_NPU_MAP_LPAR);
+OPAL_CALL(opal_imc_counters_init,              OPAL_IMC_COUNTERS_INIT);
+OPAL_CALL(opal_imc_counters_start,             OPAL_IMC_COUNTERS_START);
+OPAL_CALL(opal_imc_counters_stop,              OPAL_IMC_COUNTERS_STOP);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 0f2a803..dca7f2b 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -139,6 +139,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_L2X0_ONLINE,
 	CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
 	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
+	CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE,
 	CPUHP_AP_WORKQUEUE_ONLINE,
 	CPUHP_AP_RCUTREE_ONLINE,
 	CPUHP_AP_ONLINE_DYN,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging
  2017-06-28 18:58 [PATCH v11 05/10] powerpc/perf: IMC pmu cpumask and cpuhotplug support Anju T Sudhakar
@ 2017-06-28 18:58 ` Anju T Sudhakar
  2017-06-28 19:41   ` Thomas Gleixner
  2017-06-28 18:58 ` [PATCH v11 09/10] powerpc/perf: Thread IMC PMU functions Anju T Sudhakar
  1 sibling, 1 reply; 6+ messages in thread
From: Anju T Sudhakar @ 2017-06-28 18:58 UTC (permalink / raw)
  To: mpe
  Cc: linux-kernel, linuxppc-dev, ego, bsingharora, anton, sukadev,
	mikey, stewart, dja, eranian, hemant, maddy, anju, tglx

From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>                            
                                                                                
Code to add PMU function to initialize a core IMC event. It also                
adds cpumask initialization function for core IMC PMU. For                      
initialization, memory is allocated per core where the data                     
for core IMC counters will be accumulated. The base address for this            
page is sent to OPAL via an OPAL call which initializes various SCOMs           
related to Core IMC initialization. Upon any errors, the pages are              
free'ed and core IMC counters are disabled using the same OPAL call.            
                                                                                
For CPU hotplugging, a cpumask is initialized which contains an online          
CPU from each core. If a cpu goes offline, we check whether that cpu            
belongs to the core imc cpumask, if yes, then, we migrate the PMU               
context to any other online cpu (if available) in that core. If a cpu           
comes back online, then this cpu will be added to the core imc cpumask          
only if there was no other cpu from that core in the previous cpumask.          
                                                                                
To register the hotplug functions for core_imc, a new state                     
CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE is added to the list of existing          
states.                                                                         
                                                                                
Patch also adds OPAL device shutdown callback. Needed to disable the            
IMC core engine to handle kexec.                                                
                                                                                
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>                         
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>                        
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>    
---
 arch/powerpc/include/asm/imc-pmu.h        |   1 +
 arch/powerpc/include/asm/opal-api.h       |   1 +
 arch/powerpc/perf/imc-pmu.c               | 307 ++++++++++++++++++++++++++++--
 arch/powerpc/platforms/powernv/opal-imc.c |  24 ++-
 include/linux/cpuhotplug.h                |   1 +
 5 files changed, 314 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
index e9da151..74cbb47 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -25,6 +25,7 @@
  */
 #define IMC_MAX_PMUS			32
 #define IMC_MAX_CHIPS			32
+#define IMC_MAX_CORES			32
 
 /*
  * This macro is used for memory buffer allocation of
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index fdacb03..0d83427 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -1009,6 +1009,7 @@ enum {
 /* Argument to OPAL_IMC_COUNTERS_*  */
 enum {
 	OPAL_IMC_COUNTERS_NEST = 1,
+	OPAL_IMC_COUNTERS_CORE = 2,
 };
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index f4856eb..38da866 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1,5 +1,5 @@
 /*
- * Nest Performance Monitor counter support.
+ * IMC Performance Monitor counter support.
  *
  * Copyright (C) 2017 Madhavan Srinivasan, IBM Corporation.
  *           (C) 2017 Anju T Sudhakar, IBM Corporation.
@@ -21,15 +21,18 @@
 /* Needed for sanity check */
 struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
 static cpumask_t nest_imc_cpumask;
+static cpumask_t core_imc_cpumask;
 static int nest_imc_cpumask_initialized;
 
 static atomic_t nest_pmus;
 static atomic_t nest_events[IMC_MAX_CHIPS];
+static atomic_t core_events[IMC_MAX_CORES];
 /* Used to avoid races in counting the nest-pmu units inited */
 static DEFINE_MUTEX(imc_nest_inited_reserve);
 /* Used to avoid races in calling enable/disable nest-pmu units */
 static DEFINE_MUTEX(imc_nest_reserve);
-
+/* Used to avoid races in calling enable/disable nest-pmu units */
+static DEFINE_MUTEX(imc_core_reserve);
 
 struct imc_pmu *core_imc_pmu;
 
@@ -55,14 +58,32 @@ static struct attribute_group imc_format_group = {
 	.attrs = nest_imc_format_attrs,
 };
 
+static struct attribute *core_imc_format_attrs[] = {
+	&format_attr_event.attr,
+	&format_attr_offset.attr,
+	&format_attr_rvalue.attr,
+	NULL,
+};
+
+static struct attribute_group core_imc_format_group = {
+	.name = "format",
+	.attrs = core_imc_format_attrs,
+};
+
 /* Get the cpumask printed to a buffer "buf" */
 static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
+	struct pmu *pmu = dev_get_drvdata(dev);
 	cpumask_t *active_mask;
 
-	active_mask = &nest_imc_cpumask;
+	if (!strncmp(pmu->name, "nest_", strlen("nest_")))
+		active_mask = &nest_imc_cpumask;
+	else if (!strncmp(pmu->name, "core_", strlen("core_")))
+		active_mask = &core_imc_cpumask;
+	else
+		return 0;
 	return cpumap_print_to_pagebuf(true, buf, active_mask);
 }
 
@@ -181,6 +202,164 @@ static void nest_imc_counters_release(struct perf_event *event)
 	}
 }
 
+static void cleanup_all_core_imc_memory(struct imc_pmu *pmu_ptr)
+{
+	struct imc_mem_info *ptr;
+
+	for (ptr = pmu_ptr->mem_info; ptr; ptr++) {
+		if (ptr->vbase[0])
+			free_pages((u64)ptr->vbase[0], 0);
+	}
+
+	kfree(pmu_ptr->mem_info);
+}
+
+static void core_imc_counters_release(struct perf_event *event)
+{
+	int rc, core_id;
+	/*
+	 * See if we need to disable the IMC PMU.
+	 * If no events are currently in use, then we have to take a
+	 * mutex to ensure that we don't race with another task doing
+	 * enable or disable the core counters.
+	 */
+	core_id = event->cpu / threads_per_core;
+	if (atomic_dec_return(&core_events[core_id]) == 0) {
+		mutex_lock(&imc_core_reserve);
+		rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_CORE,
+					    get_hard_smp_processor_id(event->cpu));
+		mutex_unlock(&imc_core_reserve);
+		if (rc)
+			pr_err("IMC: Disable counters failed for core %d\n", core_id);
+	}
+}
+
+/*
+ * core_imc_mem_init : Initializes memory for the current core.
+ *
+ * Uses alloc_pages_node() and uses the returned address as an argument to
+ * an opal call to configure the pdbar. The address sent as an argument is
+ * converted to physical address before the opal call is made. This is the
+ * base address at which the core imc counters are populated.
+ */
+static int core_imc_mem_init(int cpu, int size)
+{
+	int phys_id, rc = 0, core_id = (cpu / threads_per_core);
+	struct imc_mem_info *mem_info;
+
+	/*
+	 * alloc_pages_node() will allocate memory for core in the
+	 * local node only.
+	 */
+	phys_id = topology_physical_package_id(cpu);
+	mem_info = &core_imc_pmu->mem_info[core_id];
+	mem_info->id = core_id;
+	mem_info->vbase[0] = page_address(alloc_pages_node(phys_id,
+					  GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
+					  get_order(size)));
+	if (!mem_info->vbase[0])
+		return -ENOMEM;
+
+	rc = opal_imc_counters_init(OPAL_IMC_COUNTERS_CORE,
+				(u64)virt_to_phys((void *)mem_info->vbase[0]),
+				get_hard_smp_processor_id(cpu));
+	if (rc) {
+		free_pages((u64)mem_info->vbase[0], get_order(size));
+		mem_info->vbase[0] = NULL;
+	}
+
+	return rc;
+}
+
+bool is_core_imc_mem_inited(int cpu)
+{
+	struct imc_mem_info *mem_info;
+	int core_id = (cpu / threads_per_core);
+
+	mem_info = &core_imc_pmu->mem_info[core_id];
+	if ((mem_info->id == core_id) && (mem_info->vbase[0] != NULL))
+		return true;
+
+	return false;
+}
+
+/*
+ * imc_mem_init : Function to support memory allocation for core imc.
+ */
+static int imc_mem_init(struct imc_pmu *pmu_ptr)
+{
+	int nr_cores;
+
+	if (pmu_ptr->imc_counter_mmaped)
+		return 0;
+	nr_cores = num_present_cpus() / threads_per_core;
+	pmu_ptr->mem_info = kzalloc((sizeof(struct imc_mem_info) * nr_cores), GFP_KERNEL);
+	if (!pmu_ptr->mem_info)
+		return -ENOMEM;
+	return 0;
+}
+
+static int ppc_core_imc_cpu_online(unsigned int cpu)
+{
+	const struct cpumask *l_cpumask;
+	static struct cpumask tmp_mask;
+	int ret = 0;
+
+	/* Get the cpumask for this core */
+	l_cpumask = cpu_sibling_mask(cpu);
+
+	/* If a cpu for this core is already set, then, don't do anything */
+	if (cpumask_and(&tmp_mask, l_cpumask, &core_imc_cpumask))
+		return 0;
+
+	if (!is_core_imc_mem_inited(cpu)) {
+		ret = core_imc_mem_init(cpu, core_imc_pmu->counter_mem_size);
+		if (ret) {
+			pr_info("core_imc memory allocation for cpu %d failed\n", cpu);
+			return ret;
+		}
+	} else {
+		opal_imc_counters_stop(OPAL_IMC_COUNTERS_CORE,
+				       get_hard_smp_processor_id(cpu));
+	}
+
+	/* set the cpu in the mask, and change the context */
+	cpumask_set_cpu(cpu, &core_imc_cpumask);
+	return 0;
+}
+
+static int ppc_core_imc_cpu_offline(unsigned int cpu)
+{
+	unsigned int ncpu;
+
+	/*
+	 * clear this cpu out of the mask, if not present in the mask,
+	 * don't bother doing anything.
+	 */
+	if (!cpumask_test_and_clear_cpu(cpu, &core_imc_cpumask))
+		return 0;
+
+	/* Find any online cpu in that core except the current "cpu" */
+	ncpu = cpumask_any_but(cpu_sibling_mask(cpu), cpu);
+
+	if (ncpu >= 0 && ncpu < nr_cpu_ids) {
+		cpumask_set_cpu(ncpu, &core_imc_cpumask);
+		perf_pmu_migrate_context(&core_imc_pmu->pmu, cpu, ncpu);
+	} else {
+		opal_imc_counters_stop(OPAL_IMC_COUNTERS_CORE,
+				       get_hard_smp_processor_id(cpu));
+	}
+	return 0;
+}
+
+static int core_imc_pmu_cpumask_init(void)
+{
+	return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE,
+				 "perf/powerpc/imc_core:online",
+				 ppc_core_imc_cpu_online,
+				 ppc_core_imc_cpu_offline);
+}
+
 static int nest_imc_event_init(struct perf_event *event)
 {
 	int chip_id, rc, node_id;
@@ -258,6 +437,70 @@ static int nest_imc_event_init(struct perf_event *event)
 	return 0;
 }
 
+static int core_imc_event_init(struct perf_event *event)
+{
+	int core_id, rc;
+	u64 config = event->attr.config;
+	struct imc_mem_info *pcmi;
+	struct imc_pmu *pmu;
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* Sampling not supported */
+	if (event->hw.sample_period)
+		return -EINVAL;
+
+	/* unsupported modes and filters */
+	if (event->attr.exclude_user   ||
+	    event->attr.exclude_kernel ||
+	    event->attr.exclude_hv     ||
+	    event->attr.exclude_idle   ||
+	    event->attr.exclude_host   ||
+	    event->attr.exclude_guest)
+		return -EINVAL;
+
+	if (event->cpu < 0)
+		return -EINVAL;
+
+	event->hw.idx = -1;
+	pmu = imc_event_to_pmu(event);
+
+	/* Sanity check for config (event offset and rvalue) */
+	if (((config & IMC_EVENT_OFFSET_MASK) > pmu->counter_mem_size) ||
+	    ((config & IMC_EVENT_RVALUE_MASK) != 0))
+		return -EINVAL;
+
+	if (!is_core_imc_mem_inited(event->cpu))
+		return -ENODEV;
+
+	core_id = event->cpu / threads_per_core;
+	pcmi = &pmu->mem_info[core_id];
+	if ((pcmi->id != core_id) || (!pcmi->vbase[0]))
+		return -ENODEV;
+
+	event->hw.event_base = (u64)pcmi->vbase[0] + (config & IMC_EVENT_OFFSET_MASK);
+	/*
+	 * Core pmu units are enabled only when it is used.
+	 * See if this is triggered for the first time.
+	 * If yes, take the mutex lock and enable the core counters.
+	 * If not, just increment the count in core_events.
+	 */
+	if (atomic_inc_return(&core_events[core_id]) == 1) {
+		mutex_lock(&imc_core_reserve);
+		rc = opal_imc_counters_start(OPAL_IMC_COUNTERS_CORE,
+					     get_hard_smp_processor_id(event->cpu));
+		mutex_unlock(&imc_core_reserve);
+		if (rc) {
+			atomic_dec_return(&core_events[core_id]);
+			pr_err("IMC: Unable to start the counters for core %d\n", core_id);
+			return -ENODEV;
+		}
+	}
+	event->destroy = core_imc_counters_release;
+	return 0;
+}
+
 static void imc_read_counter(struct perf_event *event)
 {
 	u64 *addr, data;
@@ -326,14 +569,19 @@ static int update_pmu_ops(struct imc_pmu *pmu)
 		return -EINVAL;
 
 	pmu->pmu.task_ctx_nr = perf_invalid_context;
-	pmu->pmu.event_init = nest_imc_event_init;
+	if (pmu->domain == IMC_DOMAIN_NEST) {
+		pmu->pmu.event_init = nest_imc_event_init;
+		pmu->attr_groups[IMC_FORMAT_ATTR] = &imc_format_group;
+	} else if (pmu->domain == IMC_DOMAIN_CORE) {
+		pmu->pmu.event_init = core_imc_event_init;
+		pmu->attr_groups[IMC_FORMAT_ATTR] = &core_imc_format_group;
+	}
 	pmu->pmu.add = imc_event_add;
 	pmu->pmu.del = imc_event_stop;
 	pmu->pmu.start = imc_event_start;
 	pmu->pmu.stop = imc_event_stop;
 	pmu->pmu.read = imc_perf_event_update;
 	pmu->attr_groups[IMC_CPUMASK_ATTR] = &imc_pmu_cpumask_attr_group;
-	pmu->attr_groups[IMC_FORMAT_ATTR] = &imc_format_group;
 	pmu->pmu.attr_groups = pmu->attr_groups;
 
 	return 0;
@@ -410,22 +658,38 @@ int init_imc_pmu(struct imc_events *events, int idx,
 		 struct imc_pmu *pmu_ptr)
 {
 	int ret = -ENODEV;
+
+	ret = imc_mem_init(pmu_ptr);
+	if (ret)
+		goto err_free;
 	/* Add cpumask and register for hotplug notification */
-	if (atomic_inc_return(&nest_pmus) == 1) {
-		/*
-		 * Nest imc pmu need only one cpu per chip, we initialize the
-		 * cpumask for the first nest imc pmu and use the same for the rest.
-		 * To handle the cpuhotplug callback unregister, we track
-		 * the number of nest pmus registers in "nest_pmus".
-		 * "nest_imc_cpumask_initialized" is set to zero during cpuhotplug
-		 * callback unregister.
-		 */
-		ret = nest_pmu_cpumask_init();
+	switch (pmu_ptr->domain) {
+	case IMC_DOMAIN_NEST:
+		if (atomic_inc_return(&nest_pmus) == 1) {
+			/*
+			 * Nest imc pmu need only one cpu per chip, we initialize
+			 * the cpumask for the first nest imc pmu and use the
+			 * same for the rest.
+			 * To handle the cpuhotplug callback unregister, we track
+			 * the number of nest pmus registers in "nest_pmus".
+			 * "nest_imc_cpumask_initialized" is set to zero during
+			 * cpuhotplug callback unregister.
+			 */
+			ret = nest_pmu_cpumask_init();
+			if (ret)
+				goto err_free;
+			mutex_lock(&imc_nest_inited_reserve);
+			nest_imc_cpumask_initialized = 1;
+			mutex_unlock(&imc_nest_inited_reserve);
+		}
+		break;
+	case IMC_DOMAIN_CORE:
+		ret = core_imc_pmu_cpumask_init();
 		if (ret)
-			goto err_free;
-		mutex_lock(&imc_nest_inited_reserve);
-		nest_imc_cpumask_initialized = 1;
-		mutex_unlock(&imc_nest_inited_reserve);
+			return ret;
+		break;
+	default:
+		return -1;	/* Unknown domain */
 	}
 	ret = update_events_in_group(events, idx, pmu_ptr);
 	if (ret)
@@ -459,5 +723,10 @@ int init_imc_pmu(struct imc_events *events, int idx,
 			mutex_unlock(&imc_nest_inited_reserve);
 		}
 	}
+	/* For core_imc, we have allocated memory, we need to free it */
+	if (pmu_ptr->domain == IMC_DOMAIN_CORE) {
+		cleanup_all_core_imc_memory(pmu_ptr);
+		cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE);
+	}
 	return ret;
 }
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 98e7b0f5..62add4f 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -33,6 +33,7 @@
 #include <asm/uaccess.h>
 #include <asm/cputable.h>
 #include <asm/imc-pmu.h>
+#include <asm/cputhreads.h>
 
 static int imc_event_prop_update(char *name, struct imc_events *events)
 {
@@ -489,7 +490,7 @@ static void disable_nest_counters(void)
 static int opal_imc_counters_probe(struct platform_device *pdev)
 {
 	struct device_node *imc_dev = NULL;
-	int pmu_count = 0, domain;
+	int pmu_count = 0, domain, cpu;
 	u32 type;
 
 	if (!pdev || !pdev->dev.of_node)
@@ -500,6 +501,9 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 	 */
 	if (is_kdump_kernel()) {
 		disable_nest_counters();
+		for_each_possible_cpu(cpu)
+			opal_imc_counters_stop(OPAL_IMC_COUNTERS_CORE,
+					       get_hard_smp_processor_id(cpu));
 		return -ENODEV;
 	}
 	imc_dev = pdev->dev.of_node;
@@ -520,6 +524,23 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static void opal_imc_stop(void *type)
+{
+	opal_imc_counters_stop((unsigned long) type,
+			       get_hard_smp_processor_id(smp_processor_id()));
+}
+
+static void opal_imc_counters_shutdown(struct platform_device *pdev)
+{
+	smp_call_func_t fn;
+	static cpumask_t cores_map;
+
+	fn = opal_imc_stop;
+	cores_map = cpu_online_cores_map();
+	/* Disable the IMC Core functions */
+	on_each_cpu_mask(&cores_map, fn, "OPAL_IMC_COUNTERS_CORE", 1);
+}
+
 static const struct of_device_id opal_imc_match[] = {
 	{ .compatible = IMC_DTB_COMPAT },
 	{},
@@ -531,6 +552,7 @@ static struct platform_driver opal_imc_driver = {
 		.of_match_table = opal_imc_match,
 	},
 	.probe = opal_imc_counters_probe,
+	.shutdown = opal_imc_counters_shutdown,
 };
 
 MODULE_DEVICE_TABLE(of, opal_imc_match);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index dca7f2b..e145fff 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -140,6 +140,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
 	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
 	CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE,
+	CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE,
 	CPUHP_AP_WORKQUEUE_ONLINE,
 	CPUHP_AP_RCUTREE_ONLINE,
 	CPUHP_AP_ONLINE_DYN,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v11 09/10] powerpc/perf: Thread IMC PMU functions
  2017-06-28 18:58 [PATCH v11 05/10] powerpc/perf: IMC pmu cpumask and cpuhotplug support Anju T Sudhakar
  2017-06-28 18:58 ` [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging Anju T Sudhakar
@ 2017-06-28 18:58 ` Anju T Sudhakar
  1 sibling, 0 replies; 6+ messages in thread
From: Anju T Sudhakar @ 2017-06-28 18:58 UTC (permalink / raw)
  To: mpe
  Cc: linux-kernel, linuxppc-dev, ego, bsingharora, anton, sukadev,
	mikey, stewart, dja, eranian, hemant, maddy, anju, tglx

Code to add PMU functions required for event initialization,                    
read, update, add, del etc. for thread IMC PMU. Thread IMC PMUs are used        
for per-task monitoring.                                                        
                                                                                
For each CPU, a page of memory is allocated and is kept static i.e.,            
these pages will exist till the machine shuts down. The base address of         
this page is assigned to the ldbar of that cpu. As soon as we do that,          
the thread IMC counters start running for that cpu and the data of these        
counters are assigned to the page allocated. But we use this for                
per-task monitoring. Whenever we start monitoring a task, the event is          
added is onto the task. At that point, we read the initial value of the         
event. Whenever, we stop monitoring the task, the final value is taken          
and the difference is the event data.                                           
                                                                                
Now, a task can move to a different cpu. Suppose a task X is moving from        
cpu A to cpu B. When the task is scheduled out of A, we get an                  
event_del for A, and hence, the event data is updated. And, we stop             
updating the X's event data. As soon as X moves on to B, event_add is           
called for B, and we again update the event_data. And this is how it            
keeps on updating the event data even when the task is scheduled on to          
different cpus.                                                                 
                                                                                
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>                        
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>                         
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>                   
---
 arch/powerpc/include/asm/imc-pmu.h        |   4 +
 arch/powerpc/perf/imc-pmu.c               | 228 +++++++++++++++++++++++++++++-
 arch/powerpc/platforms/powernv/opal-imc.c |   2 +
 3 files changed, 228 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
index a18ecb2..b154396 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -45,6 +45,9 @@
 #define IMC_DTB_COMPAT			"ibm,opal-in-memory-counters"
 #define IMC_DTB_UNIT_COMPAT		"ibm,imc-counters"
 
+#define THREAD_IMC_LDBAR_MASK           0x0003ffffffffe000ULL
+#define THREAD_IMC_ENABLE               0x8000000000000000ULL
+
 /*
  * Structure to hold memory address information for imc units.
  */
@@ -113,4 +116,5 @@ enum {
 extern struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
 extern struct imc_pmu *core_imc_pmu;
 extern int init_imc_pmu(struct imc_events *events, int idx, struct imc_pmu *pmu_ptr);
+void thread_imc_disable(void);
 #endif /* PPC_POWERNV_IMC_PMU_DEF_H */
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 38da866..3deefb1 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -18,6 +18,9 @@
 #include <asm/smp.h>
 #include <linux/string.h>
 
+/* Maintains base address for all the cpus */
+static DEFINE_PER_CPU(u64 *, thread_imc_mem);
+
 /* Needed for sanity check */
 struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
 static cpumask_t nest_imc_cpumask;
@@ -35,6 +38,7 @@ static DEFINE_MUTEX(imc_nest_reserve);
 static DEFINE_MUTEX(imc_core_reserve);
 
 struct imc_pmu *core_imc_pmu;
+static int thread_imc_mem_size;
 
 struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
@@ -284,18 +288,61 @@ bool is_core_imc_mem_inited(int cpu)
 }
 
 /*
- * imc_mem_init : Function to support memory allocation for core imc.
+ * Allocates a page of memory for each of the online cpus, and, writes the
+ * physical base address of that page to the LDBAR for that cpu. This starts
+ * the thread IMC counters.
+ */
+static int thread_imc_mem_alloc(int cpu_id, int size)
+{
+	u64 ldbar_value, *local_mem;
+	int phys_id = topology_physical_package_id(cpu_id);
+
+	if (per_cpu(thread_imc_mem, cpu_id) != NULL)
+		return 0;
+
+	local_mem =  page_address(alloc_pages_node(phys_id,
+				 GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
+				 get_order(size)));
+
+	if (!local_mem)
+		return -ENOMEM;
+
+	per_cpu(thread_imc_mem, cpu_id) = local_mem;
+
+	ldbar_value = ((u64)local_mem & (u64)THREAD_IMC_LDBAR_MASK) |
+						(u64)THREAD_IMC_ENABLE;
+
+	mtspr(SPRN_LDBAR, ldbar_value);
+	return 0;
+}
+
+/*
+ * imc_mem_init : Function to support memory allocation for core and thread imc.
  */
 static int imc_mem_init(struct imc_pmu *pmu_ptr)
 {
-	int nr_cores;
+	int nr_cores, cpu, res;
 
 	if (pmu_ptr->imc_counter_mmaped)
 		return 0;
-	nr_cores = num_present_cpus() / threads_per_core;
-	pmu_ptr->mem_info = kzalloc((sizeof(struct imc_mem_info) * nr_cores), GFP_KERNEL);
-	if (!pmu_ptr->mem_info)
-		return -ENOMEM;
+	switch (pmu_ptr->domain) {
+	case IMC_DOMAIN_CORE:
+		nr_cores = num_present_cpus() / threads_per_core;
+		pmu_ptr->mem_info = kzalloc((sizeof(struct imc_mem_info) * nr_cores), GFP_KERNEL);
+		if (!pmu_ptr->mem_info)
+			return -ENOMEM;
+		break;
+	case IMC_DOMAIN_THREAD:
+		thread_imc_mem_size = pmu_ptr->counter_mem_size;
+		for_each_online_cpu(cpu) {
+			res = thread_imc_mem_alloc(cpu, pmu_ptr->counter_mem_size);
+			if (res)
+				return res;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
 	return 0;
 }
 
@@ -501,6 +548,84 @@ static int core_imc_event_init(struct perf_event *event)
 	return 0;
 }
 
+static int thread_imc_event_init(struct perf_event *event)
+{
+	int rc, core_id;
+	u32 config = event->attr.config;
+	struct task_struct *target;
+	struct imc_pmu *pmu;
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* Sampling not supported */
+	if (event->hw.sample_period)
+		return -EINVAL;
+
+	event->hw.idx = -1;
+	pmu = imc_event_to_pmu(event);
+	core_id = event->cpu / threads_per_core;
+
+	/* Sanity check for config (event offset and rvalue) */
+	if (((config & IMC_EVENT_OFFSET_MASK) > pmu->counter_mem_size) ||
+	    ((config & IMC_EVENT_RVALUE_MASK) != 0))
+		return -EINVAL;
+
+	target = event->hw.target;
+
+	if (!target)
+		return -EINVAL;
+
+	if (!is_core_imc_mem_inited(event->cpu))
+		return -ENODEV;
+
+	event->pmu->task_ctx_nr = perf_sw_context;
+	core_id = event->cpu / threads_per_core;
+	/*
+	 * Core pmu units are enabled only when it is used.
+	 * See if this is triggered for the first time.
+	 * If yes, take the mutex lock and enable the core counters.
+	 * If not, just increment the count in core_events.
+	 */
+	if (atomic_inc_return(&core_events[core_id]) == 1) {
+		mutex_lock(&imc_core_reserve);
+		rc = opal_imc_counters_start(OPAL_IMC_COUNTERS_CORE,
+					     get_hard_smp_processor_id(event->cpu));
+		mutex_unlock(&imc_core_reserve);
+		if (rc) {
+			atomic_dec_return(&core_events[core_id]);
+			pr_err("IMC: Unable to start the counters for core %d\n", core_id);
+			return -ENODEV;
+		}
+	}
+	event->destroy = core_imc_counters_release;
+	return 0;
+}
+
+static void thread_imc_read_counter(struct perf_event *event)
+{
+	u64 *addr, data;
+
+	addr = per_cpu(thread_imc_mem, smp_processor_id()) +
+	       (event->attr.config & IMC_EVENT_OFFSET_MASK);
+	data = __be64_to_cpu(READ_ONCE(*addr));
+	local64_set(&event->hw.prev_count, data);
+}
+
+static void thread_imc_perf_event_update(struct perf_event *event)
+{
+	u64 counter_prev, counter_new, final_count, *addr;
+
+	addr = per_cpu(thread_imc_mem, smp_processor_id()) +
+	       (event->attr.config & IMC_EVENT_OFFSET_MASK);
+	counter_prev = local64_read(&event->hw.prev_count);
+	counter_new = __be64_to_cpu(READ_ONCE(*addr));
+	final_count = counter_new - counter_prev;
+
+	local64_set(&event->hw.prev_count, counter_new);
+	local64_add(final_count, &event->count);
+}
+
 static void imc_read_counter(struct perf_event *event)
 {
 	u64 *addr, data;
@@ -562,6 +687,53 @@ static int imc_event_add(struct perf_event *event, int flags)
 	return 0;
 }
 
+static void thread_imc_event_start(struct perf_event *event, int flags)
+{
+	thread_imc_read_counter(event);
+}
+
+static void thread_imc_event_stop(struct perf_event *event, int flags)
+{
+	thread_imc_perf_event_update(event);
+}
+
+static void thread_imc_event_del(struct perf_event *event, int flags)
+{
+	thread_imc_perf_event_update(event);
+}
+
+static int thread_imc_event_add(struct perf_event *event, int flags)
+{
+	thread_imc_event_start(event, flags);
+
+	return 0;
+}
+
+static void thread_imc_pmu_start_txn(struct pmu *pmu,
+				     unsigned int txn_flags)
+{
+	if (txn_flags & ~PERF_PMU_TXN_ADD)
+		return;
+	perf_pmu_disable(pmu);
+}
+
+static void thread_imc_pmu_cancel_txn(struct pmu *pmu)
+{
+	perf_pmu_enable(pmu);
+}
+
+static int thread_imc_pmu_commit_txn(struct pmu *pmu)
+{
+	perf_pmu_enable(pmu);
+	return 0;
+}
+
+static void thread_imc_pmu_sched_task(struct perf_event_context *ctx,
+				  bool sched_in)
+{
+	return;
+}
+
 /* update_pmu_ops : Populate the appropriate operations for "pmu" */
 static int update_pmu_ops(struct imc_pmu *pmu)
 {
@@ -583,7 +755,27 @@ static int update_pmu_ops(struct imc_pmu *pmu)
 	pmu->pmu.read = imc_perf_event_update;
 	pmu->attr_groups[IMC_CPUMASK_ATTR] = &imc_pmu_cpumask_attr_group;
 	pmu->pmu.attr_groups = pmu->attr_groups;
+	if (pmu->domain == IMC_DOMAIN_THREAD) {
+		pmu->pmu.event_init = thread_imc_event_init;
+		pmu->pmu.start = thread_imc_event_start;
+		pmu->pmu.add = thread_imc_event_add;
+		pmu->pmu.del = thread_imc_event_del;
+		pmu->pmu.stop = thread_imc_event_stop;
+		pmu->pmu.read = thread_imc_perf_event_update;
+		pmu->pmu.start_txn = thread_imc_pmu_start_txn;
+		pmu->pmu.cancel_txn = thread_imc_pmu_cancel_txn;
+		pmu->pmu.commit_txn = thread_imc_pmu_commit_txn;
+		pmu->pmu.sched_task = thread_imc_pmu_sched_task;
+		pmu->attr_groups[IMC_FORMAT_ATTR] = &core_imc_format_group;
 
+		/*
+		 * Since thread_imc does not have any CPUMASK attr,
+		 * this may drop the "events" attr all together.
+		 * So swap the IMC_EVENT_ATTR slot with IMC_CPUMASK_ATTR.
+		 */
+		pmu->attr_groups[IMC_CPUMASK_ATTR] = pmu->attr_groups[IMC_EVENT_ATTR];
+		pmu->attr_groups[IMC_EVENT_ATTR] = NULL;
+	}
 	return 0;
 }
 
@@ -644,6 +836,27 @@ static int update_events_in_group(struct imc_events *events,
 	return 0;
 }
 
+static void thread_imc_ldbar_disable(void *dummy)
+{
+	/* LDBAR spr is a per-thread */
+	mtspr(SPRN_LDBAR, 0);
+}
+
+void thread_imc_disable(void)
+{
+	on_each_cpu(thread_imc_ldbar_disable, NULL, 1);
+}
+
+static void cleanup_all_thread_imc_memory(void)
+{
+	int i;
+
+	for_each_online_cpu(i) {
+		if (per_cpu(thread_imc_mem, i))
+			free_pages((u64)per_cpu(thread_imc_mem, i), 0);
+	}
+}
+
 /*
  * init_imc_pmu : Setup and register the IMC pmu device.
  *
@@ -728,5 +941,8 @@ int init_imc_pmu(struct imc_events *events, int idx,
 		cleanup_all_core_imc_memory(pmu_ptr);
 		cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE);
 	}
+	/* For thread_imc, we have allocated memory, we need to free it */
+	if (pmu_ptr->domain == IMC_DOMAIN_THREAD)
+		cleanup_all_thread_imc_memory();
 	return ret;
 }
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 6d24dfb..bca8147 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -541,6 +541,8 @@ static void opal_imc_counters_shutdown(struct platform_device *pdev)
 	cores_map = cpu_online_cores_map();
 	/* Disable the IMC Core functions */
 	on_each_cpu_mask(&cores_map, fn, "OPAL_IMC_COUNTERS_CORE", 1);
+	/* Disable the IMC Thread functions */
+	thread_imc_disable();
 }
 
 static const struct of_device_id opal_imc_match[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging
  2017-06-28 18:58 ` [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging Anju T Sudhakar
@ 2017-06-28 19:41   ` Thomas Gleixner
  2017-06-29  9:14     ` Madhavan Srinivasan
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2017-06-28 19:41 UTC (permalink / raw)
  To: Anju T Sudhakar
  Cc: mpe, linux-kernel, linuxppc-dev, ego, bsingharora, anton,
	sukadev, mikey, stewart, dja, eranian, hemant, maddy

On Thu, 29 Jun 2017, Anju T Sudhakar wrote:
> +static void cleanup_all_core_imc_memory(struct imc_pmu *pmu_ptr)
> +{
> +	struct imc_mem_info *ptr;
> +
> +	for (ptr = pmu_ptr->mem_info; ptr; ptr++) {
> +		if (ptr->vbase[0])
> +			free_pages((u64)ptr->vbase[0], 0);
> +	}

This loop is broken beyond repair. If pmu_ptr->mem_info is not NULL, then
ptr will happily increment to the point where it wraps around to
NULL. Oh well.

> +	kfree(pmu_ptr->mem_info);

> +bool is_core_imc_mem_inited(int cpu)

This function is global because?

> +{
> +	struct imc_mem_info *mem_info;
> +	int core_id = (cpu / threads_per_core);
> +
> +	mem_info = &core_imc_pmu->mem_info[core_id];
> +	if ((mem_info->id == core_id) && (mem_info->vbase[0] != NULL))
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * imc_mem_init : Function to support memory allocation for core imc.
> + */
> +static int imc_mem_init(struct imc_pmu *pmu_ptr)
> +{

The function placement is horrible. This function belongs to the pmu init
stuff and wants to be placed there and not five pages away in the middle of
unrelated functions.

> +	int nr_cores;
> +
> +	if (pmu_ptr->imc_counter_mmaped)
> +		return 0;
> +	nr_cores = num_present_cpus() / threads_per_core;
> +	pmu_ptr->mem_info = kzalloc((sizeof(struct imc_mem_info) * nr_cores), GFP_KERNEL);
> +	if (!pmu_ptr->mem_info)
> +		return -ENOMEM;
> +	return 0;
> +}
> +static int core_imc_event_init(struct perf_event *event)
> +{
> +	int core_id, rc;
> +	u64 config = event->attr.config;
> +	struct imc_mem_info *pcmi;
> +	struct imc_pmu *pmu;
> +
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	/* Sampling not supported */
> +	if (event->hw.sample_period)
> +		return -EINVAL;
> +
> +	/* unsupported modes and filters */
> +	if (event->attr.exclude_user   ||
> +	    event->attr.exclude_kernel ||
> +	    event->attr.exclude_hv     ||
> +	    event->attr.exclude_idle   ||
> +	    event->attr.exclude_host   ||
> +	    event->attr.exclude_guest)
> +		return -EINVAL;
> +
> +	if (event->cpu < 0)
> +		return -EINVAL;
> +
> +	event->hw.idx = -1;
> +	pmu = imc_event_to_pmu(event);
> +
> +	/* Sanity check for config (event offset and rvalue) */
> +	if (((config & IMC_EVENT_OFFSET_MASK) > pmu->counter_mem_size) ||
> +	    ((config & IMC_EVENT_RVALUE_MASK) != 0))
> +		return -EINVAL;
> +
> +	if (!is_core_imc_mem_inited(event->cpu))
> +		return -ENODEV;
> +
> +	core_id = event->cpu / threads_per_core;
> +	pcmi = &pmu->mem_info[core_id];
> +	if ((pcmi->id != core_id) || (!pcmi->vbase[0]))
> +		return -ENODEV;
> +
> +	event->hw.event_base = (u64)pcmi->vbase[0] + (config & IMC_EVENT_OFFSET_MASK);
> +	/*
> +	 * Core pmu units are enabled only when it is used.
> +	 * See if this is triggered for the first time.
> +	 * If yes, take the mutex lock and enable the core counters.
> +	 * If not, just increment the count in core_events.
> +	 */
> +	if (atomic_inc_return(&core_events[core_id]) == 1) {
> +		mutex_lock(&imc_core_reserve);
> +		rc = opal_imc_counters_start(OPAL_IMC_COUNTERS_CORE,
> +					     get_hard_smp_processor_id(event->cpu));
> +		mutex_unlock(&imc_core_reserve);

That machinery here is racy as hell in several aspects.

CPU0 	       	       	       	       CPU1

atomic_inc_ret(core_events[0]) -> 1

preemption()
			       	       atomic_inc_ret(core_events[0]) -> 2
				       return 0;
				       
				       Uses the event, without counters
				       being started until the preempted
				       task comes on the CPU again.

Here is another one:

CPU0 	       	       	       	       CPU1

atomic_dec_ret(core_events[0]) -> 0
				       atomic_inc_ret(core_events[1] -> 1
				       mutex_lock();
mutex_lock()			       start counter();
				       mutex_unlock()

stop_counter();
mutex_unlock();
				       Yay, another stale event!

Brilliant stuff that, or maybe not so much.

> +		if (rc) {
> +			atomic_dec_return(&core_events[core_id]);

What's the point of using atomic_dec_return here if you ignore the return
value? Not that it matters much as the whole logic is crap anyway.

> +			pr_err("IMC: Unable to start the counters for core %d\n", core_id);
> +			return -ENODEV;
> +		}
> +	}
> @@ -410,22 +658,38 @@ int init_imc_pmu(struct imc_events *events, int idx,
>  		 struct imc_pmu *pmu_ptr)
>  {
>  	int ret = -ENODEV;
> +

Sure ret needs to stay initialized, just in case imc_mem_init() does not
return anything magically, right?

> +	ret = imc_mem_init(pmu_ptr);
> +	if (ret)
> +		goto err_free;
>  	/* Add cpumask and register for hotplug notification */
> -	if (atomic_inc_return(&nest_pmus) == 1) {
> -		/*
> -		 * Nest imc pmu need only one cpu per chip, we initialize the
> -		 * cpumask for the first nest imc pmu and use the same for the rest.
> -		 * To handle the cpuhotplug callback unregister, we track
> -		 * the number of nest pmus registers in "nest_pmus".
> -		 * "nest_imc_cpumask_initialized" is set to zero during cpuhotplug
> -		 * callback unregister.
> -		 */
> -		ret = nest_pmu_cpumask_init();
> +	switch (pmu_ptr->domain) {
> +	case IMC_DOMAIN_NEST:
> +		if (atomic_inc_return(&nest_pmus) == 1) {
> +			/*
> +			 * Nest imc pmu need only one cpu per chip, we initialize
> +			 * the cpumask for the first nest imc pmu and use the
> +			 * same for the rest.
> +			 * To handle the cpuhotplug callback unregister, we track
> +			 * the number of nest pmus registers in "nest_pmus".
> +			 * "nest_imc_cpumask_initialized" is set to zero during
> +			 * cpuhotplug callback unregister.
> +			 */
> +			ret = nest_pmu_cpumask_init();
> +			if (ret)
> +				goto err_free;
> +			mutex_lock(&imc_nest_inited_reserve);
> +			nest_imc_cpumask_initialized = 1;
> +			mutex_unlock(&imc_nest_inited_reserve);
> +		}
> +		break;
> +	case IMC_DOMAIN_CORE:
> +		ret = core_imc_pmu_cpumask_init();
>  		if (ret)
> -			goto err_free;
> -		mutex_lock(&imc_nest_inited_reserve);
> -		nest_imc_cpumask_initialized = 1;
> -		mutex_unlock(&imc_nest_inited_reserve);
> +			return ret;

Oh, so now you replaced the goto with ret. What is actually taking care of
the cleanup if that fails?

> +		break;
> +	default:
> +		return -1;	/* Unknown domain */
>  	}
>  	ret = update_events_in_group(events, idx, pmu_ptr);
>  	if (ret)
> @@ -459,5 +723,10 @@ int init_imc_pmu(struct imc_events *events, int idx,
>  			mutex_unlock(&imc_nest_inited_reserve);
>  		}
>  	}
> +	/* For core_imc, we have allocated memory, we need to free it */
> +	if (pmu_ptr->domain == IMC_DOMAIN_CORE) {
> +		cleanup_all_core_imc_memory(pmu_ptr);
> +		cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE);

Cute. You cleanuo the memory stuff and then you let the hotplug core invoke
the offline callbacks which then deal with freed memory. 

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging
  2017-06-28 19:41   ` Thomas Gleixner
@ 2017-06-29  9:14     ` Madhavan Srinivasan
  2017-06-29 10:39       ` Thomas Gleixner
  0 siblings, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2017-06-29  9:14 UTC (permalink / raw)
  To: Thomas Gleixner, Anju T Sudhakar
  Cc: mpe, linux-kernel, linuxppc-dev, ego, bsingharora, anton,
	sukadev, mikey, stewart, dja, eranian, hemant



On Thursday 29 June 2017 01:11 AM, Thomas Gleixner wrote:
> On Thu, 29 Jun 2017, Anju T Sudhakar wrote:
>> +static void cleanup_all_core_imc_memory(struct imc_pmu *pmu_ptr)
>> +{
>> +	struct imc_mem_info *ptr;
>> +
>> +	for (ptr = pmu_ptr->mem_info; ptr; ptr++) {
>> +		if (ptr->vbase[0])
>> +			free_pages((u64)ptr->vbase[0], 0);
>> +	}
> This loop is broken beyond repair. If pmu_ptr->mem_info is not NULL, then
> ptr will happily increment to the point where it wraps around to
> NULL. Oh well.
>
>> +	kfree(pmu_ptr->mem_info);
>> +bool is_core_imc_mem_inited(int cpu)
> This function is global because?
>
>> +{
>> +	struct imc_mem_info *mem_info;
>> +	int core_id = (cpu / threads_per_core);
>> +
>> +	mem_info = &core_imc_pmu->mem_info[core_id];
>> +	if ((mem_info->id == core_id) && (mem_info->vbase[0] != NULL))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/*
>> + * imc_mem_init : Function to support memory allocation for core imc.
>> + */
>> +static int imc_mem_init(struct imc_pmu *pmu_ptr)
>> +{
> The function placement is horrible. This function belongs to the pmu init
> stuff and wants to be placed there and not five pages away in the middle of
> unrelated functions.
>
>> +	int nr_cores;
>> +
>> +	if (pmu_ptr->imc_counter_mmaped)
>> +		return 0;
>> +	nr_cores = num_present_cpus() / threads_per_core;
>> +	pmu_ptr->mem_info = kzalloc((sizeof(struct imc_mem_info) * nr_cores), GFP_KERNEL);
>> +	if (!pmu_ptr->mem_info)
>> +		return -ENOMEM;
>> +	return 0;
>> +}
>> +static int core_imc_event_init(struct perf_event *event)
>> +{
>> +	int core_id, rc;
>> +	u64 config = event->attr.config;
>> +	struct imc_mem_info *pcmi;
>> +	struct imc_pmu *pmu;
>> +
>> +	if (event->attr.type != event->pmu->type)
>> +		return -ENOENT;
>> +
>> +	/* Sampling not supported */
>> +	if (event->hw.sample_period)
>> +		return -EINVAL;
>> +
>> +	/* unsupported modes and filters */
>> +	if (event->attr.exclude_user   ||
>> +	    event->attr.exclude_kernel ||
>> +	    event->attr.exclude_hv     ||
>> +	    event->attr.exclude_idle   ||
>> +	    event->attr.exclude_host   ||
>> +	    event->attr.exclude_guest)
>> +		return -EINVAL;
>> +
>> +	if (event->cpu < 0)
>> +		return -EINVAL;
>> +
>> +	event->hw.idx = -1;
>> +	pmu = imc_event_to_pmu(event);
>> +
>> +	/* Sanity check for config (event offset and rvalue) */
>> +	if (((config & IMC_EVENT_OFFSET_MASK) > pmu->counter_mem_size) ||
>> +	    ((config & IMC_EVENT_RVALUE_MASK) != 0))
>> +		return -EINVAL;
>> +
>> +	if (!is_core_imc_mem_inited(event->cpu))
>> +		return -ENODEV;
>> +
>> +	core_id = event->cpu / threads_per_core;
>> +	pcmi = &pmu->mem_info[core_id];
>> +	if ((pcmi->id != core_id) || (!pcmi->vbase[0]))
>> +		return -ENODEV;
>> +
>> +	event->hw.event_base = (u64)pcmi->vbase[0] + (config & IMC_EVENT_OFFSET_MASK);
>> +	/*
>> +	 * Core pmu units are enabled only when it is used.
>> +	 * See if this is triggered for the first time.
>> +	 * If yes, take the mutex lock and enable the core counters.
>> +	 * If not, just increment the count in core_events.
>> +	 */
>> +	if (atomic_inc_return(&core_events[core_id]) == 1) {
>> +		mutex_lock(&imc_core_reserve);
>> +		rc = opal_imc_counters_start(OPAL_IMC_COUNTERS_CORE,
>> +					     get_hard_smp_processor_id(event->cpu));
>> +		mutex_unlock(&imc_core_reserve);

Idea is to handle multiple event session for a given core and
yes, my bad, current implementation is racy/broken.
But an alternate approach is to have a per-core mutex and
per-core ref count to handle this.

event_init path:
per-core mutex lock
   if ( per-core[refcount] == 0) {
        rc = opal call to start the engine;
        if (rc on failure) {
        per-core mutex unlock;
        log error info;
        return error;
        }
   }
   increment the per-core[refcount];
per-core mutex unlock;


event_destroy path:
per-core mutex lock
   decrement the per-core[refcount];
   if ( per-core[refcount] == 0) {
      rc = opal call to stop the engine;
      if (rc on failure) {
        per-core mutex unlock;
        log the failure;
        return error;
      }
   } else if ( per-core[refcount] < 0) {
      WARN()
      per-core[refcount] = 0;
   }
per-core mutext unlock;

Kindly let me know your comment for this.

Maddy

> That machinery here is racy as hell in several aspects.
>
> CPU0 	       	       	       	       CPU1
>
> atomic_inc_ret(core_events[0]) -> 1
>
> preemption()
> 			       	       atomic_inc_ret(core_events[0]) -> 2
> 				       return 0;
> 				
> 				       Uses the event, without counters
> 				       being started until the preempted
> 				       task comes on the CPU again.
>
> Here is another one:
>
> CPU0 	       	       	       	       CPU1
>
> atomic_dec_ret(core_events[0]) -> 0
> 				       atomic_inc_ret(core_events[1] -> 1
> 				       mutex_lock();
> mutex_lock()			       start counter();
> 				       mutex_unlock()
>
> stop_counter();
> mutex_unlock();
> 				       Yay, another stale event!
>
> Brilliant stuff that, or maybe not so much.
>
>> +		if (rc) {
>> +			atomic_dec_return(&core_events[core_id]);
> What's the point of using atomic_dec_return here if you ignore the return
> value? Not that it matters much as the whole logic is crap anyway.
>
>> +			pr_err("IMC: Unable to start the counters for core %d\n", core_id);
>> +			return -ENODEV;
>> +		}
>> +	}
>> @@ -410,22 +658,38 @@ int init_imc_pmu(struct imc_events *events, int idx,
>>   		 struct imc_pmu *pmu_ptr)
>>   {
>>   	int ret = -ENODEV;
>> +
> Sure ret needs to stay initialized, just in case imc_mem_init() does not
> return anything magically, right?
>
>> +	ret = imc_mem_init(pmu_ptr);
>> +	if (ret)
>> +		goto err_free;
>>   	/* Add cpumask and register for hotplug notification */
>> -	if (atomic_inc_return(&nest_pmus) == 1) {
>> -		/*
>> -		 * Nest imc pmu need only one cpu per chip, we initialize the
>> -		 * cpumask for the first nest imc pmu and use the same for the rest.
>> -		 * To handle the cpuhotplug callback unregister, we track
>> -		 * the number of nest pmus registers in "nest_pmus".
>> -		 * "nest_imc_cpumask_initialized" is set to zero during cpuhotplug
>> -		 * callback unregister.
>> -		 */
>> -		ret = nest_pmu_cpumask_init();
>> +	switch (pmu_ptr->domain) {
>> +	case IMC_DOMAIN_NEST:
>> +		if (atomic_inc_return(&nest_pmus) == 1) {
>> +			/*
>> +			 * Nest imc pmu need only one cpu per chip, we initialize
>> +			 * the cpumask for the first nest imc pmu and use the
>> +			 * same for the rest.
>> +			 * To handle the cpuhotplug callback unregister, we track
>> +			 * the number of nest pmus registers in "nest_pmus".
>> +			 * "nest_imc_cpumask_initialized" is set to zero during
>> +			 * cpuhotplug callback unregister.
>> +			 */
>> +			ret = nest_pmu_cpumask_init();
>> +			if (ret)
>> +				goto err_free;
>> +			mutex_lock(&imc_nest_inited_reserve);
>> +			nest_imc_cpumask_initialized = 1;
>> +			mutex_unlock(&imc_nest_inited_reserve);
>> +		}
>> +		break;
>> +	case IMC_DOMAIN_CORE:
>> +		ret = core_imc_pmu_cpumask_init();
>>   		if (ret)
>> -			goto err_free;
>> -		mutex_lock(&imc_nest_inited_reserve);
>> -		nest_imc_cpumask_initialized = 1;
>> -		mutex_unlock(&imc_nest_inited_reserve);
>> +			return ret;
> Oh, so now you replaced the goto with ret. What is actually taking care of
> the cleanup if that fails?
>
>> +		break;
>> +	default:
>> +		return -1;	/* Unknown domain */
>>   	}
>>   	ret = update_events_in_group(events, idx, pmu_ptr);
>>   	if (ret)
>> @@ -459,5 +723,10 @@ int init_imc_pmu(struct imc_events *events, int idx,
>>   			mutex_unlock(&imc_nest_inited_reserve);
>>   		}
>>   	}
>> +	/* For core_imc, we have allocated memory, we need to free it */
>> +	if (pmu_ptr->domain == IMC_DOMAIN_CORE) {
>> +		cleanup_all_core_imc_memory(pmu_ptr);
>> +		cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE);
> Cute. You cleanuo the memory stuff and then you let the hotplug core invoke
> the offline callbacks which then deal with freed memory.
>
> Thanks,
>
> 	tglx
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging
  2017-06-29  9:14     ` Madhavan Srinivasan
@ 2017-06-29 10:39       ` Thomas Gleixner
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Gleixner @ 2017-06-29 10:39 UTC (permalink / raw)
  To: Madhavan Srinivasan
  Cc: Anju T Sudhakar, mpe, linux-kernel, linuxppc-dev, ego,
	bsingharora, anton, sukadev, mikey, stewart, dja, eranian,
	hemant

On Thu, 29 Jun 2017, Madhavan Srinivasan wrote:
> On Thursday 29 June 2017 01:11 AM, Thomas Gleixner wrote:
> Idea is to handle multiple event session for a given core and
> yes, my bad, current implementation is racy/broken.
> But an alternate approach is to have a per-core mutex and
> per-core ref count to handle this.
> 
> event_init path:
> per-core mutex lock
>   if ( per-core[refcount] == 0) {
>        rc = opal call to start the engine;
>        if (rc on failure) {
>        per-core mutex unlock;
>        log error info;
>        return error;
>        }
>   }
>   increment the per-core[refcount];
> per-core mutex unlock;
> 
> 
> event_destroy path:
> per-core mutex lock
>   decrement the per-core[refcount];
>   if ( per-core[refcount] == 0) {
>      rc = opal call to stop the engine;
>      if (rc on failure) {
>        per-core mutex unlock;
>        log the failure;
>        return error;
>      }
>   } else if ( per-core[refcount] < 0) {
>      WARN()
>      per-core[refcount] = 0;
>   }
> per-core mutext unlock;

Yes, that works and looks about right.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-29 10:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-28 18:58 [PATCH v11 05/10] powerpc/perf: IMC pmu cpumask and cpuhotplug support Anju T Sudhakar
2017-06-28 18:58 ` [PATCH v11 07/10] powerpc/perf: PMU functions for Core IMC and hotplugging Anju T Sudhakar
2017-06-28 19:41   ` Thomas Gleixner
2017-06-29  9:14     ` Madhavan Srinivasan
2017-06-29 10:39       ` Thomas Gleixner
2017-06-28 18:58 ` [PATCH v11 09/10] powerpc/perf: Thread IMC PMU functions Anju T Sudhakar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.