[PATCH 0/4] powerpc/perf: IMC Cleanups

linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/4] powerpc/perf: IMC Cleanups
@ 2018-04-09  9:00 Anju T Sudhakar
  2018-04-09  9:00 ` [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Anju T Sudhakar @ 2018-04-09  9:00 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, maddy, anju

This patch series includes some cleanups and Unregistration of                  
thread-imc pmu, if the kernel does not have core-imc registered.                
                                                                                
The entire patch set has been verified using the static checker smatch.         
Command used:                                                                   
$ make ARCH=powerpc CHECK="<path>/smatch -p=kernel"  C=1 vmlinux | tee warns.txt
                                                                                
Tests Done:                                                                     
                                                                                
* Fail core-imc at init:                                                        
nest-imc - working                                                              
cpuhotplug - works as expected                                                  
thread-imc - not registered                                                     
                                                                                
* Fail thread-imc at init:                                                      
nest-imc - works                                                                
core-imc - works                                                                
cpuhotplug - works                                                              
                                                                                
* Fail nest-imc at init                                                         
                                                                                
core-imc - works                                                                
thread-imc -works                                                               
cpuhotplug - works                                                              
                                                                                
* Fail only one nest unit (say for mcs23)                                       
                                                                                
Other nest-units - works                                                        
core-imc - works                                                                
thread-imc - works                                                              
cpuhotplug - works.                                                             
                                                                                
                                                                                
* Kexec works                                                                   
                                                                                
The first three patches in this series addresses the comments by Dan Carpenter. 


Anju T Sudhakar (4):
  powerpc/perf: Rearrange memory freeing in imc init
  powerpc/perf: Replace the direct return with goto statement
  powerpc/perf: Return appropriate value for unknown domain
  powerpc/perf: Unregister thread-imc if core-imc not supported

 arch/powerpc/include/asm/imc-pmu.h        |  1 +
 arch/powerpc/perf/imc-pmu.c               | 64 +++++++++++++++++++------------
 arch/powerpc/platforms/powernv/opal-imc.c | 22 +++++++++--
 3 files changed, 60 insertions(+), 27 deletions(-)

-- 
2.7.4

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

* [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init
  2018-04-09  9:00 [PATCH 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
@ 2018-04-09  9:00 ` Anju T Sudhakar
  2018-05-03  3:33   ` Madhavan Srinivasan
  2018-04-09  9:00 ` [PATCH 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Anju T Sudhakar @ 2018-04-09  9:00 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, maddy, anju

When any of the IMC (In-Memory Collection counter) devices fail
to initialize, imc_common_mem_free() frees set of memory. In doing so,
pmu_ptr pointer is also freed. But pmu_ptr pointer is used in subsequent
function (imc_common_cpuhp_mem_free()) which is wrong. Patch here reorders
the code to avoid such access.

Also free the memory which is dynamically allocated during imc initialization,
wherever required.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
test matrix and static checker run details are updated in the cover letter
patch is based on 
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git (branch: merge)

 arch/powerpc/perf/imc-pmu.c               | 32 ++++++++++++++++---------------
 arch/powerpc/platforms/powernv/opal-imc.c | 13 ++++++++++---
 2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index d7532e7..258b0f4 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1153,7 +1153,7 @@ static void cleanup_all_core_imc_memory(void)
 	/* mem_info will never be NULL */
 	for (i = 0; i < nr_cores; i++) {
 		if (ptr[i].vbase)
-			free_pages((u64)ptr->vbase, get_order(size));
+			free_pages((u64)ptr[i].vbase, get_order(size));
 	}
 
 	kfree(ptr);
@@ -1191,7 +1191,6 @@ static void imc_common_mem_free(struct imc_pmu *pmu_ptr)
 	if (pmu_ptr->attr_groups[IMC_EVENT_ATTR])
 		kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]->attrs);
 	kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]);
-	kfree(pmu_ptr);
 }
 
 /*
@@ -1208,6 +1207,7 @@ static void imc_common_cpuhp_mem_free(struct imc_pmu *pmu_ptr)
 			cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE);
 			kfree(nest_imc_refc);
 			kfree(per_nest_pmu_arr);
+			per_nest_pmu_arr = NULL;
 		}
 
 		if (nest_pmus > 0)
@@ -1319,10 +1319,8 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 	int ret;
 
 	ret = imc_mem_init(pmu_ptr, parent, pmu_idx);
-	if (ret) {
-		imc_common_mem_free(pmu_ptr);
-		return ret;
-	}
+	if (ret)
+		goto err_free_mem;
 
 	switch (pmu_ptr->domain) {
 	case IMC_DOMAIN_NEST:
@@ -1337,7 +1335,9 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 			ret = init_nest_pmu_ref();
 			if (ret) {
 				mutex_unlock(&nest_init_lock);
-				goto err_free;
+				kfree(per_nest_pmu_arr);
+				per_nest_pmu_arr = NULL;
+				goto err_free_mem;
 			}
 			/* Register for cpu hotplug notification. */
 			ret = nest_pmu_cpumask_init();
@@ -1345,7 +1345,8 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 				mutex_unlock(&nest_init_lock);
 				kfree(nest_imc_refc);
 				kfree(per_nest_pmu_arr);
-				goto err_free;
+				per_nest_pmu_arr = NULL;
+				goto err_free_mem;
 			}
 		}
 		nest_pmus++;
@@ -1355,7 +1356,7 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 		ret = core_imc_pmu_cpumask_init();
 		if (ret) {
 			cleanup_all_core_imc_memory();
-			return ret;
+			goto err_free_mem;
 		}
 
 		break;
@@ -1363,7 +1364,7 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 		ret = thread_imc_cpu_init();
 		if (ret) {
 			cleanup_all_thread_imc_memory();
-			return ret;
+			goto err_free_mem;
 		}
 
 		break;
@@ -1373,23 +1374,24 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 
 	ret = update_events_in_group(parent, pmu_ptr);
 	if (ret)
-		goto err_free;
+		goto err_free_cpuhp_mem;
 
 	ret = update_pmu_ops(pmu_ptr);
 	if (ret)
-		goto err_free;
+		goto err_free_cpuhp_mem;
 
 	ret = perf_pmu_register(&pmu_ptr->pmu, pmu_ptr->pmu.name, -1);
 	if (ret)
-		goto err_free;
+		goto err_free_cpuhp_mem;
 
 	pr_info("%s performance monitor hardware support registered\n",
 							pmu_ptr->pmu.name);
 
 	return 0;
 
-err_free:
-	imc_common_mem_free(pmu_ptr);
+err_free_cpuhp_mem:
 	imc_common_cpuhp_mem_free(pmu_ptr);
+err_free_mem:
+	imc_common_mem_free(pmu_ptr);
 	return ret;
 }
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 2a14fda..490bb72 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -115,8 +115,10 @@ static int imc_get_mem_addr_nest(struct device_node *node,
 		return -ENOMEM;
 
 	chipid_arr = kcalloc(nr_chips, sizeof(*chipid_arr), GFP_KERNEL);
-	if (!chipid_arr)
+	if (!chipid_arr) {
+		kfree(base_addr_arr);
 		return -ENOMEM;
+	}
 
 	if (of_property_read_u32_array(node, "chip-id", chipid_arr, nr_chips))
 		goto error;
@@ -143,7 +145,6 @@ static int imc_get_mem_addr_nest(struct device_node *node,
 	return 0;
 
 error:
-	kfree(pmu_ptr->mem_info);
 	kfree(base_addr_arr);
 	kfree(chipid_arr);
 	return -1;
@@ -183,8 +184,14 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
 
 	/* Function to register IMC pmu */
 	ret = init_imc_pmu(parent, pmu_ptr, pmu_index);
-	if (ret)
+	if (ret) {
 		pr_err("IMC PMU %s Register failed\n", pmu_ptr->pmu.name);
+		kfree(pmu_ptr->pmu.name);
+		if (pmu_ptr->domain == IMC_DOMAIN_NEST)
+			kfree(pmu_ptr->mem_info);
+		kfree(pmu_ptr);
+		return ret;
+	}
 
 	return 0;
 
-- 
2.7.4

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

* Re: [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init
  2018-04-09  9:00 ` [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
@ 2018-05-03  3:33   ` Madhavan Srinivasan
  0 siblings, 0 replies; 9+ messages in thread
From: Madhavan Srinivasan @ 2018-05-03  3:33 UTC (permalink / raw)
  To: linuxppc-dev



On Monday 09 April 2018 02:30 PM, Anju T Sudhakar wrote:
> When any of the IMC (In-Memory Collection counter) devices fail
> to initialize, imc_common_mem_free() frees set of memory. In doing so,
> pmu_ptr pointer is also freed. But pmu_ptr pointer is used in subsequent
> function (imc_common_cpuhp_mem_free()) which is wrong. Patch here reorders
> the code to avoid such access.
>
> Also free the memory which is dynamically allocated during imc initialization,
> wherever required.

Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>

> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
> test matrix and static checker run details are updated in the cover letter
> patch is based on
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git (branch: merge)
>
>   arch/powerpc/perf/imc-pmu.c               | 32 ++++++++++++++++---------------
>   arch/powerpc/platforms/powernv/opal-imc.c | 13 ++++++++++---
>   2 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index d7532e7..258b0f4 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1153,7 +1153,7 @@ static void cleanup_all_core_imc_memory(void)
>   	/* mem_info will never be NULL */
>   	for (i = 0; i < nr_cores; i++) {
>   		if (ptr[i].vbase)
> -			free_pages((u64)ptr->vbase, get_order(size));
> +			free_pages((u64)ptr[i].vbase, get_order(size));
>   	}
>
>   	kfree(ptr);
> @@ -1191,7 +1191,6 @@ static void imc_common_mem_free(struct imc_pmu *pmu_ptr)
>   	if (pmu_ptr->attr_groups[IMC_EVENT_ATTR])
>   		kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]->attrs);
>   	kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]);
> -	kfree(pmu_ptr);
>   }
>
>   /*
> @@ -1208,6 +1207,7 @@ static void imc_common_cpuhp_mem_free(struct imc_pmu *pmu_ptr)
>   			cpuhp_remove_state(CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE);
>   			kfree(nest_imc_refc);
>   			kfree(per_nest_pmu_arr);
> +			per_nest_pmu_arr = NULL;
>   		}
>
>   		if (nest_pmus > 0)
> @@ -1319,10 +1319,8 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>   	int ret;
>
>   	ret = imc_mem_init(pmu_ptr, parent, pmu_idx);
> -	if (ret) {
> -		imc_common_mem_free(pmu_ptr);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_free_mem;
>
>   	switch (pmu_ptr->domain) {
>   	case IMC_DOMAIN_NEST:
> @@ -1337,7 +1335,9 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>   			ret = init_nest_pmu_ref();
>   			if (ret) {
>   				mutex_unlock(&nest_init_lock);
> -				goto err_free;
> +				kfree(per_nest_pmu_arr);
> +				per_nest_pmu_arr = NULL;
> +				goto err_free_mem;
>   			}
>   			/* Register for cpu hotplug notification. */
>   			ret = nest_pmu_cpumask_init();
> @@ -1345,7 +1345,8 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>   				mutex_unlock(&nest_init_lock);
>   				kfree(nest_imc_refc);
>   				kfree(per_nest_pmu_arr);
> -				goto err_free;
> +				per_nest_pmu_arr = NULL;
> +				goto err_free_mem;
>   			}
>   		}
>   		nest_pmus++;
> @@ -1355,7 +1356,7 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>   		ret = core_imc_pmu_cpumask_init();
>   		if (ret) {
>   			cleanup_all_core_imc_memory();
> -			return ret;
> +			goto err_free_mem;
>   		}
>
>   		break;
> @@ -1363,7 +1364,7 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>   		ret = thread_imc_cpu_init();
>   		if (ret) {
>   			cleanup_all_thread_imc_memory();
> -			return ret;
> +			goto err_free_mem;
>   		}
>
>   		break;
> @@ -1373,23 +1374,24 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>
>   	ret = update_events_in_group(parent, pmu_ptr);
>   	if (ret)
> -		goto err_free;
> +		goto err_free_cpuhp_mem;
>
>   	ret = update_pmu_ops(pmu_ptr);
>   	if (ret)
> -		goto err_free;
> +		goto err_free_cpuhp_mem;
>
>   	ret = perf_pmu_register(&pmu_ptr->pmu, pmu_ptr->pmu.name, -1);
>   	if (ret)
> -		goto err_free;
> +		goto err_free_cpuhp_mem;
>
>   	pr_info("%s performance monitor hardware support registered\n",
>   							pmu_ptr->pmu.name);
>
>   	return 0;
>
> -err_free:
> -	imc_common_mem_free(pmu_ptr);
> +err_free_cpuhp_mem:
>   	imc_common_cpuhp_mem_free(pmu_ptr);
> +err_free_mem:
> +	imc_common_mem_free(pmu_ptr);
>   	return ret;
>   }
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> index 2a14fda..490bb72 100644
> --- a/arch/powerpc/platforms/powernv/opal-imc.c
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -115,8 +115,10 @@ static int imc_get_mem_addr_nest(struct device_node *node,
>   		return -ENOMEM;
>
>   	chipid_arr = kcalloc(nr_chips, sizeof(*chipid_arr), GFP_KERNEL);
> -	if (!chipid_arr)
> +	if (!chipid_arr) {
> +		kfree(base_addr_arr);
>   		return -ENOMEM;
> +	}
>
>   	if (of_property_read_u32_array(node, "chip-id", chipid_arr, nr_chips))
>   		goto error;
> @@ -143,7 +145,6 @@ static int imc_get_mem_addr_nest(struct device_node *node,
>   	return 0;
>
>   error:
> -	kfree(pmu_ptr->mem_info);
>   	kfree(base_addr_arr);
>   	kfree(chipid_arr);
>   	return -1;
> @@ -183,8 +184,14 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
>
>   	/* Function to register IMC pmu */
>   	ret = init_imc_pmu(parent, pmu_ptr, pmu_index);
> -	if (ret)
> +	if (ret) {
>   		pr_err("IMC PMU %s Register failed\n", pmu_ptr->pmu.name);
> +		kfree(pmu_ptr->pmu.name);
> +		if (pmu_ptr->domain == IMC_DOMAIN_NEST)
> +			kfree(pmu_ptr->mem_info);
> +		kfree(pmu_ptr);
> +		return ret;
> +	}
>
>   	return 0;
>

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

* [PATCH 2/4] powerpc/perf: Replace the direct return with goto statement
  2018-04-09  9:00 [PATCH 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
  2018-04-09  9:00 ` [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
@ 2018-04-09  9:00 ` Anju T Sudhakar
  2018-05-03  3:33   ` Madhavan Srinivasan
  2018-04-09  9:00 ` [PATCH 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
  2018-04-09  9:00 ` [PATCH 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
  3 siblings, 1 reply; 9+ messages in thread
From: Anju T Sudhakar @ 2018-04-09  9:00 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, maddy, anju

Replace the direct return statement in imc_mem_init() with goto,
to adhere to the kernel coding style.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/perf/imc-pmu.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 258b0f4..1b285cd 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1236,7 +1236,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 								int pmu_index)
 {
 	const char *s;
-	int nr_cores, cpu, res;
+	int nr_cores, cpu, res = -ENOMEM;
 
 	if (of_property_read_string(parent, "name", &s))
 		return -ENODEV;
@@ -1246,7 +1246,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 		/* Update the pmu name */
 		pmu_ptr->pmu.name = kasprintf(GFP_KERNEL, "%s%s_imc", "nest_", s);
 		if (!pmu_ptr->pmu.name)
-			return -ENOMEM;
+			goto err;
 
 		/* Needed for hotplug/migration */
 		if (!per_nest_pmu_arr) {
@@ -1254,7 +1254,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 						sizeof(struct imc_pmu *),
 						GFP_KERNEL);
 			if (!per_nest_pmu_arr)
-				return -ENOMEM;
+				goto err;
 		}
 		per_nest_pmu_arr[pmu_index] = pmu_ptr;
 		break;
@@ -1262,21 +1262,21 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 		/* Update the pmu name */
 		pmu_ptr->pmu.name = kasprintf(GFP_KERNEL, "%s%s", s, "_imc");
 		if (!pmu_ptr->pmu.name)
-			return -ENOMEM;
+			goto err;
 
 		nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
 		pmu_ptr->mem_info = kcalloc(nr_cores, sizeof(struct imc_mem_info),
 								GFP_KERNEL);
 
 		if (!pmu_ptr->mem_info)
-			return -ENOMEM;
+			goto err;
 
 		core_imc_refc = kcalloc(nr_cores, sizeof(struct imc_pmu_ref),
 								GFP_KERNEL);
 
 		if (!core_imc_refc) {
 			kfree(pmu_ptr->mem_info);
-			return -ENOMEM;
+			goto err;
 		}
 
 		core_imc_pmu = pmu_ptr;
@@ -1285,14 +1285,14 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 		/* Update the pmu name */
 		pmu_ptr->pmu.name = kasprintf(GFP_KERNEL, "%s%s", s, "_imc");
 		if (!pmu_ptr->pmu.name)
-			return -ENOMEM;
+			goto err;
 
 		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) {
 				cleanup_all_thread_imc_memory();
-				return res;
+				goto err;
 			}
 		}
 
@@ -1302,6 +1302,8 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 	}
 
 	return 0;
+err:
+	return res;
 }
 
 /*
-- 
2.7.4

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

* Re: [PATCH 2/4] powerpc/perf: Replace the direct return with goto statement
  2018-04-09  9:00 ` [PATCH 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
@ 2018-05-03  3:33   ` Madhavan Srinivasan
  0 siblings, 0 replies; 9+ messages in thread
From: Madhavan Srinivasan @ 2018-05-03  3:33 UTC (permalink / raw)
  To: linuxppc-dev



On Monday 09 April 2018 02:30 PM, Anju T Sudhakar wrote:
> Replace the direct return statement in imc_mem_init() with goto,
> to adhere to the kernel coding style.

Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>

> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>   arch/powerpc/perf/imc-pmu.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index 258b0f4..1b285cd 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1236,7 +1236,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   								int pmu_index)
>   {
>   	const char *s;
> -	int nr_cores, cpu, res;
> +	int nr_cores, cpu, res = -ENOMEM;
>
>   	if (of_property_read_string(parent, "name", &s))
>   		return -ENODEV;
> @@ -1246,7 +1246,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   		/* Update the pmu name */
>   		pmu_ptr->pmu.name = kasprintf(GFP_KERNEL, "%s%s_imc", "nest_", s);
>   		if (!pmu_ptr->pmu.name)
> -			return -ENOMEM;
> +			goto err;
>
>   		/* Needed for hotplug/migration */
>   		if (!per_nest_pmu_arr) {
> @@ -1254,7 +1254,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   						sizeof(struct imc_pmu *),
>   						GFP_KERNEL);
>   			if (!per_nest_pmu_arr)
> -				return -ENOMEM;
> +				goto err;
>   		}
>   		per_nest_pmu_arr[pmu_index] = pmu_ptr;
>   		break;
> @@ -1262,21 +1262,21 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   		/* Update the pmu name */
>   		pmu_ptr->pmu.name = kasprintf(GFP_KERNEL, "%s%s", s, "_imc");
>   		if (!pmu_ptr->pmu.name)
> -			return -ENOMEM;
> +			goto err;
>
>   		nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
>   		pmu_ptr->mem_info = kcalloc(nr_cores, sizeof(struct imc_mem_info),
>   								GFP_KERNEL);
>
>   		if (!pmu_ptr->mem_info)
> -			return -ENOMEM;
> +			goto err;
>
>   		core_imc_refc = kcalloc(nr_cores, sizeof(struct imc_pmu_ref),
>   								GFP_KERNEL);
>
>   		if (!core_imc_refc) {
>   			kfree(pmu_ptr->mem_info);
> -			return -ENOMEM;
> +			goto err;
>   		}
>
>   		core_imc_pmu = pmu_ptr;
> @@ -1285,14 +1285,14 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   		/* Update the pmu name */
>   		pmu_ptr->pmu.name = kasprintf(GFP_KERNEL, "%s%s", s, "_imc");
>   		if (!pmu_ptr->pmu.name)
> -			return -ENOMEM;
> +			goto err;
>
>   		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) {
>   				cleanup_all_thread_imc_memory();
> -				return res;
> +				goto err;
>   			}
>   		}
>
> @@ -1302,6 +1302,8 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   	}
>
>   	return 0;
> +err:
> +	return res;
>   }
>
>   /*

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

* [PATCH 3/4] powerpc/perf: Return appropriate value for unknown domain
  2018-04-09  9:00 [PATCH 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
  2018-04-09  9:00 ` [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
  2018-04-09  9:00 ` [PATCH 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
@ 2018-04-09  9:00 ` Anju T Sudhakar
  2018-05-03  3:32   ` Madhavan Srinivasan
  2018-04-09  9:00 ` [PATCH 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
  3 siblings, 1 reply; 9+ messages in thread
From: Anju T Sudhakar @ 2018-04-09  9:00 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, maddy, anju

Return proper error code for unknown domain during IMC initialization.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/perf/imc-pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 1b285cd..4b4ca83 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1371,7 +1371,7 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
 
 		break;
 	default:
-		return  -1;	/* Unknown domain */
+		return  -EINVAL;	/* Unknown domain */
 	}
 
 	ret = update_events_in_group(parent, pmu_ptr);
-- 
2.7.4

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

* Re: [PATCH 3/4] powerpc/perf: Return appropriate value for unknown domain
  2018-04-09  9:00 ` [PATCH 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
@ 2018-05-03  3:32   ` Madhavan Srinivasan
  0 siblings, 0 replies; 9+ messages in thread
From: Madhavan Srinivasan @ 2018-05-03  3:32 UTC (permalink / raw)
  To: linuxppc-dev



On Monday 09 April 2018 02:30 PM, Anju T Sudhakar wrote:
> Return proper error code for unknown domain during IMC initialization.

Looks good to me.

Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>

> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>   arch/powerpc/perf/imc-pmu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index 1b285cd..4b4ca83 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1371,7 +1371,7 @@ int init_imc_pmu(struct device_node *parent, struct imc_pmu *pmu_ptr, int pmu_id
>
>   		break;
>   	default:
> -		return  -1;	/* Unknown domain */
> +		return  -EINVAL;	/* Unknown domain */
>   	}
>
>   	ret = update_events_in_group(parent, pmu_ptr);

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

* [PATCH 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported
  2018-04-09  9:00 [PATCH 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
                   ` (2 preceding siblings ...)
  2018-04-09  9:00 ` [PATCH 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
@ 2018-04-09  9:00 ` Anju T Sudhakar
  2018-05-03  3:31   ` Madhavan Srinivasan
  3 siblings, 1 reply; 9+ messages in thread
From: Anju T Sudhakar @ 2018-04-09  9:00 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, maddy, anju

Enable thread-imc in the kernel, only if core-imc is registered.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/imc-pmu.h        |  1 +
 arch/powerpc/perf/imc-pmu.c               | 12 ++++++++++++
 arch/powerpc/platforms/powernv/opal-imc.c |  9 +++++++++
 3 files changed, 22 insertions(+)

diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
index d76cb11..69f516e 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -128,4 +128,5 @@ extern int init_imc_pmu(struct device_node *parent,
 				struct imc_pmu *pmu_ptr, int pmu_id);
 extern void thread_imc_disable(void);
 extern int get_max_nest_dev(void);
+extern void unregister_thread_imc(void);
 #endif /* __ASM_POWERPC_IMC_PMU_H */
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 4b4ca83..fa88785 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -40,6 +40,7 @@ static struct imc_pmu *core_imc_pmu;
 /* Thread IMC data structures and variables */
 
 static DEFINE_PER_CPU(u64 *, thread_imc_mem);
+static struct imc_pmu *thread_imc_pmu;
 static int thread_imc_mem_size;
 
 struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
@@ -1228,6 +1229,16 @@ static void imc_common_cpuhp_mem_free(struct imc_pmu *pmu_ptr)
 	}
 }
 
+/*
+ * Function to unregister thread-imc if core-imc
+ * is not registered.
+ */
+void unregister_thread_imc(void)
+{
+	imc_common_cpuhp_mem_free(thread_imc_pmu);
+	imc_common_mem_free(thread_imc_pmu);
+	perf_pmu_unregister(&thread_imc_pmu->pmu);
+}
 
 /*
  * imc_mem_init : Function to support memory allocation for core imc.
@@ -1296,6 +1307,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 			}
 		}
 
+		thread_imc_pmu = pmu_ptr;
 		break;
 	default:
 		return -EINVAL;
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 490bb72..58a0794 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -255,6 +255,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 {
 	struct device_node *imc_dev = pdev->dev.of_node;
 	int pmu_count = 0, domain;
+	bool core_imc_reg = false, thread_imc_reg = false;
 	u32 type;
 
 	/*
@@ -292,6 +293,10 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 		if (!imc_pmu_create(imc_dev, pmu_count, domain)) {
 			if (domain == IMC_DOMAIN_NEST)
 				pmu_count++;
+			if (domain == IMC_DOMAIN_CORE)
+				core_imc_reg = true;
+			if (domain == IMC_DOMAIN_THREAD)
+				thread_imc_reg = true;
 		}
 	}
 
@@ -299,6 +304,10 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 	if (pmu_count == 0)
 		debugfs_remove_recursive(imc_debugfs_parent);
 
+	/* If core imc is not registered, unregister thread-imc */
+	if (!core_imc_reg && thread_imc_reg)
+		unregister_thread_imc();
+
 	return 0;
 }
 
-- 
2.7.4

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

* Re: [PATCH 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported
  2018-04-09  9:00 ` [PATCH 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
@ 2018-05-03  3:31   ` Madhavan Srinivasan
  0 siblings, 0 replies; 9+ messages in thread
From: Madhavan Srinivasan @ 2018-05-03  3:31 UTC (permalink / raw)
  To: linuxppc-dev



On Monday 09 April 2018 02:30 PM, Anju T Sudhakar wrote:
> Enable thread-imc in the kernel, only if core-imc is registered.

Can you add more info here? Why we need this and so on.

> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>   arch/powerpc/include/asm/imc-pmu.h        |  1 +
>   arch/powerpc/perf/imc-pmu.c               | 12 ++++++++++++
>   arch/powerpc/platforms/powernv/opal-imc.c |  9 +++++++++
>   3 files changed, 22 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
> index d76cb11..69f516e 100644
> --- a/arch/powerpc/include/asm/imc-pmu.h
> +++ b/arch/powerpc/include/asm/imc-pmu.h
> @@ -128,4 +128,5 @@ extern int init_imc_pmu(struct device_node *parent,
>   				struct imc_pmu *pmu_ptr, int pmu_id);
>   extern void thread_imc_disable(void);
>   extern int get_max_nest_dev(void);
> +extern void unregister_thread_imc(void);
>   #endif /* __ASM_POWERPC_IMC_PMU_H */
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index 4b4ca83..fa88785 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -40,6 +40,7 @@ static struct imc_pmu *core_imc_pmu;
>   /* Thread IMC data structures and variables */
>
>   static DEFINE_PER_CPU(u64 *, thread_imc_mem);
> +static struct imc_pmu *thread_imc_pmu;
>   static int thread_imc_mem_size;
>
>   struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
> @@ -1228,6 +1229,16 @@ static void imc_common_cpuhp_mem_free(struct imc_pmu *pmu_ptr)
>   	}
>   }
>
> +/*
> + * Function to unregister thread-imc if core-imc
> + * is not registered.
> + */
> +void unregister_thread_imc(void)
> +{
> +	imc_common_cpuhp_mem_free(thread_imc_pmu);
> +	imc_common_mem_free(thread_imc_pmu);
> +	perf_pmu_unregister(&thread_imc_pmu->pmu);
> +}
>
>   /*
>    * imc_mem_init : Function to support memory allocation for core imc.
> @@ -1296,6 +1307,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   			}
>   		}
>
> +		thread_imc_pmu = pmu_ptr;
>   		break;
>   	default:
>   		return -EINVAL;
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> index 490bb72..58a0794 100644
> --- a/arch/powerpc/platforms/powernv/opal-imc.c
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -255,6 +255,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>   {
>   	struct device_node *imc_dev = pdev->dev.of_node;
>   	int pmu_count = 0, domain;
> +	bool core_imc_reg = false, thread_imc_reg = false;
>   	u32 type;
>
>   	/*
> @@ -292,6 +293,10 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>   		if (!imc_pmu_create(imc_dev, pmu_count, domain)) {
>   			if (domain == IMC_DOMAIN_NEST)
>   				pmu_count++;
> +			if (domain == IMC_DOMAIN_CORE)
> +				core_imc_reg = true;
> +			if (domain == IMC_DOMAIN_THREAD)
> +				thread_imc_reg = true;
>   		}
>   	}
>
> @@ -299,6 +304,10 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>   	if (pmu_count == 0)
>   		debugfs_remove_recursive(imc_debugfs_parent);
>
> +	/* If core imc is not registered, unregister thread-imc */
> +	if (!core_imc_reg && thread_imc_reg)
> +		unregister_thread_imc();
> +
>   	return 0;
>   }
>

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

end of thread, other threads:[~2018-05-03  3:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-09  9:00 [PATCH 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
2018-04-09  9:00 ` [PATCH 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
2018-05-03  3:33   ` Madhavan Srinivasan
2018-04-09  9:00 ` [PATCH 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
2018-05-03  3:33   ` Madhavan Srinivasan
2018-04-09  9:00 ` [PATCH 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
2018-05-03  3:32   ` Madhavan Srinivasan
2018-04-09  9:00 ` [PATCH 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
2018-05-03  3:31   ` Madhavan Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).