* [PATCH v2 0/4] powerpc/perf: IMC Cleanups
@ 2018-05-22 9:12 Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Anju T Sudhakar @ 2018-05-22 9:12 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.
Patch series is based on:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git (merge branch)
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] 6+ messages in thread
* [PATCH v2 1/4] powerpc/perf: Rearrange memory freeing in imc init
2018-05-22 9:12 [PATCH v2 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
@ 2018-05-22 9:12 ` Anju T Sudhakar
2018-06-04 14:11 ` [v2,1/4] " Michael Ellerman
2018-05-22 9:12 ` [PATCH v2 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Anju T Sudhakar @ 2018-05-22 9:12 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>
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
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 75fb23c..c1665ff 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] 6+ messages in thread
* [PATCH v2 2/4] powerpc/perf: Replace the direct return with goto statement
2018-05-22 9:12 [PATCH v2 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
@ 2018-05-22 9:12 ` Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
3 siblings, 0 replies; 6+ messages in thread
From: Anju T Sudhakar @ 2018-05-22 9:12 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>
Reviewed-by: Madhavan Srinivasan <maddy@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 c1665ff..61000d1 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_possible_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] 6+ messages in thread
* [PATCH v2 3/4] powerpc/perf: Return appropriate value for unknown domain
2018-05-22 9:12 [PATCH v2 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
@ 2018-05-22 9:12 ` Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
3 siblings, 0 replies; 6+ messages in thread
From: Anju T Sudhakar @ 2018-05-22 9:12 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>
Reviewed-by: Madhavan Srinivasan <maddy@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 61000d1..f563d30 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] 6+ messages in thread
* [PATCH v2 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported
2018-05-22 9:12 [PATCH v2 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
` (2 preceding siblings ...)
2018-05-22 9:12 ` [PATCH v2 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
@ 2018-05-22 9:12 ` Anju T Sudhakar
3 siblings, 0 replies; 6+ messages in thread
From: Anju T Sudhakar @ 2018-05-22 9:12 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, maddy, anju
Since thread-imc internally use the core-imc hardware infrastructure
and is depended on it, having thread-imc in the kernel in the
absence of core-imc is trivial. Patch disables thread-imc, if
core-imc is not registered.
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Reviewed-by: Madhavan Srinivasan <maddy@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 f563d30..d1977b6 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] 6+ messages in thread
* Re: [v2,1/4] powerpc/perf: Rearrange memory freeing in imc init
2018-05-22 9:12 ` [PATCH v2 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
@ 2018-06-04 14:11 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2018-06-04 14:11 UTC (permalink / raw)
To: Anju T Sudhakar; +Cc: maddy, linuxppc-dev, anju
On Tue, 2018-05-22 at 09:12:34 UTC, 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.
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/cb094fa5af7c9623084aa4c3cf529b
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-04 14:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-22 9:12 [PATCH v2 0/4] powerpc/perf: IMC Cleanups Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 1/4] powerpc/perf: Rearrange memory freeing in imc init Anju T Sudhakar
2018-06-04 14:11 ` [v2,1/4] " Michael Ellerman
2018-05-22 9:12 ` [PATCH v2 2/4] powerpc/perf: Replace the direct return with goto statement Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 3/4] powerpc/perf: Return appropriate value for unknown domain Anju T Sudhakar
2018-05-22 9:12 ` [PATCH v2 4/4] powerpc/perf: Unregister thread-imc if core-imc not supported Anju T Sudhakar
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).