* [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
* [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
* [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
* [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
* 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
* 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
* 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
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).