* [PATCH 1/2] drivers/perf: hisi: Remove redundant initialized of pmu->name
2023-04-03 8:14 [PATCH 0/2]Fix NULL pointer and doing cleanup Junhao He
@ 2023-04-03 8:14 ` Junhao He
2023-04-03 11:02 ` Jonathan Cameron
2023-04-03 8:14 ` [PATCH 2/2] drivers/perf: hisi: add NULL check for name Junhao He
2023-04-17 15:03 ` [PATCH 0/2]Fix NULL pointer and doing cleanup Will Deacon
2 siblings, 1 reply; 6+ messages in thread
From: Junhao He @ 2023-04-03 8:14 UTC (permalink / raw)
To: jonathan.cameron, will, linux-kernel, mark.rutland
Cc: linux-arm-kernel, catalin.marinas, kernel-team, linuxarm,
yangyicong, f.fangjian, shenyang39, prime.zeng, hejunhao3
"pmu->name" is initialized by perf_pmu_register() function, so remove
the redundant initialized in hisi_pmu_init().
Signed-off-by: Junhao He <hejunhao3@huawei.com>
---
drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c | 2 +-
drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 2 +-
drivers/perf/hisilicon/hisi_uncore_hha_pmu.c | 2 +-
drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 2 +-
drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 2 +-
drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 +---
drivers/perf/hisilicon/hisi_uncore_pmu.h | 3 +--
drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 2 +-
8 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
index 4c67d57217a7..40f1bc9f9b91 100644
--- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
@@ -316,7 +316,7 @@ static int hisi_cpa_pmu_probe(struct platform_device *pdev)
if (!name)
return -ENOMEM;
- hisi_pmu_init(cpa_pmu, name, THIS_MODULE);
+ hisi_pmu_init(cpa_pmu, THIS_MODULE);
/* Power Management should be disabled before using CPA PMU. */
hisi_cpa_pmu_disable_pm(cpa_pmu);
diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index 8c3ffcbfd4c0..8a3d74ddcd6d 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -516,7 +516,7 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
"hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
ddrc_pmu->index_id);
- hisi_pmu_init(ddrc_pmu, name, THIS_MODULE);
+ hisi_pmu_init(ddrc_pmu, THIS_MODULE);
ret = perf_pmu_register(&ddrc_pmu->pmu, name, -1);
if (ret) {
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index 806698b9eabf..5701a84edb0e 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -519,7 +519,7 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
hha_pmu->sccl_id, hha_pmu->index_id);
- hisi_pmu_init(hha_pmu, name, THIS_MODULE);
+ hisi_pmu_init(hha_pmu, THIS_MODULE);
ret = perf_pmu_register(&hha_pmu->pmu, name, -1);
if (ret) {
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index 5b2c35f1658a..68596b566344 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -557,7 +557,7 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
*/
name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
l3c_pmu->sccl_id, l3c_pmu->ccl_id);
- hisi_pmu_init(l3c_pmu, name, THIS_MODULE);
+ hisi_pmu_init(l3c_pmu, THIS_MODULE);
ret = perf_pmu_register(&l3c_pmu->pmu, name, -1);
if (ret) {
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index afe3419f3f6d..71b6687d6696 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -412,7 +412,7 @@ static int hisi_pa_pmu_probe(struct platform_device *pdev)
return ret;
}
- hisi_pmu_init(pa_pmu, name, THIS_MODULE);
+ hisi_pmu_init(pa_pmu, THIS_MODULE);
ret = perf_pmu_register(&pa_pmu->pmu, name, -1);
if (ret) {
dev_err(pa_pmu->dev, "PMU register failed, ret = %d\n", ret);
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index f1b0f5e1a28f..2823f381930d 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -531,12 +531,10 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
}
EXPORT_SYMBOL_GPL(hisi_uncore_pmu_offline_cpu);
-void hisi_pmu_init(struct hisi_pmu *hisi_pmu, const char *name,
- struct module *module)
+void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module)
{
struct pmu *pmu = &hisi_pmu->pmu;
- pmu->name = name;
pmu->module = module;
pmu->task_ctx_nr = perf_invalid_context;
pmu->event_init = hisi_uncore_pmu_event_init;
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index f8e3cc6903d7..07890a8e96ca 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -121,6 +121,5 @@ ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
int hisi_uncore_pmu_init_irq(struct hisi_pmu *hisi_pmu,
struct platform_device *pdev);
-void hisi_pmu_init(struct hisi_pmu *hisi_pmu, const char *name,
- struct module *module);
+void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module);
#endif /* __HISI_UNCORE_PMU_H__ */
diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
index 1e354433776a..6fe534a665ed 100644
--- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
@@ -445,7 +445,7 @@ static int hisi_sllc_pmu_probe(struct platform_device *pdev)
return ret;
}
- hisi_pmu_init(sllc_pmu, name, THIS_MODULE);
+ hisi_pmu_init(sllc_pmu, THIS_MODULE);
ret = perf_pmu_register(&sllc_pmu->pmu, name, -1);
if (ret) {
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] drivers/perf: hisi: Remove redundant initialized of pmu->name
2023-04-03 8:14 ` [PATCH 1/2] drivers/perf: hisi: Remove redundant initialized of pmu->name Junhao He
@ 2023-04-03 11:02 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-04-03 11:02 UTC (permalink / raw)
To: Junhao He
Cc: will, linux-kernel, mark.rutland, linux-arm-kernel,
catalin.marinas, kernel-team, linuxarm, yangyicong, f.fangjian,
shenyang39, prime.zeng
On Mon, 3 Apr 2023 16:14:22 +0800
Junhao He <hejunhao3@huawei.com> wrote:
> "pmu->name" is initialized by perf_pmu_register() function, so remove
> the redundant initialized in hisi_pmu_init().
>
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_hha_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 +---
> drivers/perf/hisilicon/hisi_uncore_pmu.h | 3 +--
> drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 2 +-
> 8 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
> index 4c67d57217a7..40f1bc9f9b91 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
> @@ -316,7 +316,7 @@ static int hisi_cpa_pmu_probe(struct platform_device *pdev)
> if (!name)
> return -ENOMEM;
>
> - hisi_pmu_init(cpa_pmu, name, THIS_MODULE);
> + hisi_pmu_init(cpa_pmu, THIS_MODULE);
>
> /* Power Management should be disabled before using CPA PMU. */
> hisi_cpa_pmu_disable_pm(cpa_pmu);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> index 8c3ffcbfd4c0..8a3d74ddcd6d 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> @@ -516,7 +516,7 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
> "hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
> ddrc_pmu->index_id);
>
> - hisi_pmu_init(ddrc_pmu, name, THIS_MODULE);
> + hisi_pmu_init(ddrc_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&ddrc_pmu->pmu, name, -1);
> if (ret) {
> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> index 806698b9eabf..5701a84edb0e 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> @@ -519,7 +519,7 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
>
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
> hha_pmu->sccl_id, hha_pmu->index_id);
> - hisi_pmu_init(hha_pmu, name, THIS_MODULE);
> + hisi_pmu_init(hha_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&hha_pmu->pmu, name, -1);
> if (ret) {
> diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> index 5b2c35f1658a..68596b566344 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> @@ -557,7 +557,7 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
> */
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
> l3c_pmu->sccl_id, l3c_pmu->ccl_id);
> - hisi_pmu_init(l3c_pmu, name, THIS_MODULE);
> + hisi_pmu_init(l3c_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&l3c_pmu->pmu, name, -1);
> if (ret) {
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
> index afe3419f3f6d..71b6687d6696 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
> @@ -412,7 +412,7 @@ static int hisi_pa_pmu_probe(struct platform_device *pdev)
> return ret;
> }
>
> - hisi_pmu_init(pa_pmu, name, THIS_MODULE);
> + hisi_pmu_init(pa_pmu, THIS_MODULE);
> ret = perf_pmu_register(&pa_pmu->pmu, name, -1);
> if (ret) {
> dev_err(pa_pmu->dev, "PMU register failed, ret = %d\n", ret);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> index f1b0f5e1a28f..2823f381930d 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> @@ -531,12 +531,10 @@ int hisi_uncore_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> }
> EXPORT_SYMBOL_GPL(hisi_uncore_pmu_offline_cpu);
>
> -void hisi_pmu_init(struct hisi_pmu *hisi_pmu, const char *name,
> - struct module *module)
> +void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module)
> {
> struct pmu *pmu = &hisi_pmu->pmu;
>
> - pmu->name = name;
> pmu->module = module;
> pmu->task_ctx_nr = perf_invalid_context;
> pmu->event_init = hisi_uncore_pmu_event_init;
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
> index f8e3cc6903d7..07890a8e96ca 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
> @@ -121,6 +121,5 @@ ssize_t hisi_uncore_pmu_identifier_attr_show(struct device *dev,
> int hisi_uncore_pmu_init_irq(struct hisi_pmu *hisi_pmu,
> struct platform_device *pdev);
>
> -void hisi_pmu_init(struct hisi_pmu *hisi_pmu, const char *name,
> - struct module *module);
> +void hisi_pmu_init(struct hisi_pmu *hisi_pmu, struct module *module);
> #endif /* __HISI_UNCORE_PMU_H__ */
> diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
> index 1e354433776a..6fe534a665ed 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
> @@ -445,7 +445,7 @@ static int hisi_sllc_pmu_probe(struct platform_device *pdev)
> return ret;
> }
>
> - hisi_pmu_init(sllc_pmu, name, THIS_MODULE);
> + hisi_pmu_init(sllc_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&sllc_pmu->pmu, name, -1);
> if (ret) {
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] drivers/perf: hisi: add NULL check for name
2023-04-03 8:14 [PATCH 0/2]Fix NULL pointer and doing cleanup Junhao He
2023-04-03 8:14 ` [PATCH 1/2] drivers/perf: hisi: Remove redundant initialized of pmu->name Junhao He
@ 2023-04-03 8:14 ` Junhao He
2023-04-03 11:08 ` Jonathan Cameron
2023-04-17 15:03 ` [PATCH 0/2]Fix NULL pointer and doing cleanup Will Deacon
2 siblings, 1 reply; 6+ messages in thread
From: Junhao He @ 2023-04-03 8:14 UTC (permalink / raw)
To: jonathan.cameron, will, linux-kernel, mark.rutland
Cc: linux-arm-kernel, catalin.marinas, kernel-team, linuxarm,
yangyicong, f.fangjian, shenyang39, prime.zeng, hejunhao3
When allocations fails that can be NULL now.
If the name provided is NULL, then the initialization process of the PMU
type and dev will be skipped in function perf_pmu_register().
Consequently, the PMU will not be able to register into the kernel.
Moreover, in the case of unregister the PMU, the function device_del()
will need to handle NULL pointers, which potentially can cause issues.
So move this allocation above the cpuhp_state_add_instance() and directly
return if it does fail.
Signed-off-by: Junhao He <hejunhao3@huawei.com>
---
drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 17 ++++++++++-------
drivers/perf/hisilicon/hisi_uncore_hha_pmu.c | 7 +++++--
drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 11 +++++------
3 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
index 8a3d74ddcd6d..ffb039d05d07 100644
--- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
@@ -499,13 +499,6 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
- &ddrc_pmu->node);
- if (ret) {
- dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
- return ret;
- }
-
if (ddrc_pmu->identifier >= HISI_PMU_V2)
name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
"hisi_sccl%u_ddrc%u_%u",
@@ -516,6 +509,16 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
"hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
ddrc_pmu->index_id);
+ if (!name)
+ return -ENOMEM;
+
+ ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
+ &ddrc_pmu->node);
+ if (ret) {
+ dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
+ return ret;
+ }
+
hisi_pmu_init(ddrc_pmu, THIS_MODULE);
ret = perf_pmu_register(&ddrc_pmu->pmu, name, -1);
diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
index 5701a84edb0e..15caf99e1eef 100644
--- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
@@ -510,6 +510,11 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
if (ret)
return ret;
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
+ hha_pmu->sccl_id, hha_pmu->index_id);
+ if (!name)
+ return -ENOMEM;
+
ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_HHA_ONLINE,
&hha_pmu->node);
if (ret) {
@@ -517,8 +522,6 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
return ret;
}
- name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
- hha_pmu->sccl_id, hha_pmu->index_id);
hisi_pmu_init(hha_pmu, THIS_MODULE);
ret = perf_pmu_register(&hha_pmu->pmu, name, -1);
diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
index 68596b566344..794dbcd19b7a 100644
--- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
@@ -544,6 +544,11 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
if (ret)
return ret;
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
+ l3c_pmu->sccl_id, l3c_pmu->ccl_id);
+ if (!name)
+ return -ENOMEM;
+
ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_L3_ONLINE,
&l3c_pmu->node);
if (ret) {
@@ -551,12 +556,6 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
return ret;
}
- /*
- * CCL_ID is used to identify the L3C in the same SCCL which was
- * used _UID by mistake.
- */
- name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
- l3c_pmu->sccl_id, l3c_pmu->ccl_id);
hisi_pmu_init(l3c_pmu, THIS_MODULE);
ret = perf_pmu_register(&l3c_pmu->pmu, name, -1);
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] drivers/perf: hisi: add NULL check for name
2023-04-03 8:14 ` [PATCH 2/2] drivers/perf: hisi: add NULL check for name Junhao He
@ 2023-04-03 11:08 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-04-03 11:08 UTC (permalink / raw)
To: Junhao He
Cc: will, linux-kernel, mark.rutland, linux-arm-kernel,
catalin.marinas, kernel-team, linuxarm, yangyicong, f.fangjian,
shenyang39, prime.zeng
On Mon, 3 Apr 2023 16:14:23 +0800
Junhao He <hejunhao3@huawei.com> wrote:
> When allocations fails that can be NULL now.
>
> If the name provided is NULL, then the initialization process of the PMU
> type and dev will be skipped in function perf_pmu_register().
> Consequently, the PMU will not be able to register into the kernel.
> Moreover, in the case of unregister the PMU, the function device_del()
> will need to handle NULL pointers, which potentially can cause issues.
If you could be more specific in what the problem is wrt to a NULL pointer here
that would be great. Given there is code to skip the device registration
without it being an error, I'd expect the release flow to be safe in that condition.
Seems like a sensible change even without that. So with some more info
feel free to add
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> So move this allocation above the cpuhp_state_add_instance() and directly
> return if it does fail.
>
> Signed-off-by: Junhao He <hejunhao3@huawei.com>
> ---
> drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 17 ++++++++++-------
> drivers/perf/hisilicon/hisi_uncore_hha_pmu.c | 7 +++++--
> drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 11 +++++------
> 3 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> index 8a3d74ddcd6d..ffb039d05d07 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c
> @@ -499,13 +499,6 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
> - &ddrc_pmu->node);
> - if (ret) {
> - dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
> - return ret;
> - }
> -
> if (ddrc_pmu->identifier >= HISI_PMU_V2)
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> "hisi_sccl%u_ddrc%u_%u",
> @@ -516,6 +509,16 @@ static int hisi_ddrc_pmu_probe(struct platform_device *pdev)
> "hisi_sccl%u_ddrc%u", ddrc_pmu->sccl_id,
> ddrc_pmu->index_id);
>
> + if (!name)
> + return -ENOMEM;
> +
> + ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
> + &ddrc_pmu->node);
> + if (ret) {
> + dev_err(&pdev->dev, "Error %d registering hotplug;\n", ret);
> + return ret;
> + }
> +
> hisi_pmu_init(ddrc_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&ddrc_pmu->pmu, name, -1);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> index 5701a84edb0e..15caf99e1eef 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_hha_pmu.c
> @@ -510,6 +510,11 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
> + hha_pmu->sccl_id, hha_pmu->index_id);
> + if (!name)
> + return -ENOMEM;
> +
> ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_HHA_ONLINE,
> &hha_pmu->node);
> if (ret) {
> @@ -517,8 +522,6 @@ static int hisi_hha_pmu_probe(struct platform_device *pdev)
> return ret;
> }
>
> - name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_hha%u",
> - hha_pmu->sccl_id, hha_pmu->index_id);
> hisi_pmu_init(hha_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&hha_pmu->pmu, name, -1);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> index 68596b566344..794dbcd19b7a 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c
> @@ -544,6 +544,11 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
> + l3c_pmu->sccl_id, l3c_pmu->ccl_id);
> + if (!name)
> + return -ENOMEM;
> +
> ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_L3_ONLINE,
> &l3c_pmu->node);
> if (ret) {
> @@ -551,12 +556,6 @@ static int hisi_l3c_pmu_probe(struct platform_device *pdev)
> return ret;
> }
>
> - /*
> - * CCL_ID is used to identify the L3C in the same SCCL which was
> - * used _UID by mistake.
> - */
> - name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_l3c%u",
> - l3c_pmu->sccl_id, l3c_pmu->ccl_id);
> hisi_pmu_init(l3c_pmu, THIS_MODULE);
>
> ret = perf_pmu_register(&l3c_pmu->pmu, name, -1);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2]Fix NULL pointer and doing cleanup
2023-04-03 8:14 [PATCH 0/2]Fix NULL pointer and doing cleanup Junhao He
2023-04-03 8:14 ` [PATCH 1/2] drivers/perf: hisi: Remove redundant initialized of pmu->name Junhao He
2023-04-03 8:14 ` [PATCH 2/2] drivers/perf: hisi: add NULL check for name Junhao He
@ 2023-04-17 15:03 ` Will Deacon
2 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2023-04-17 15:03 UTC (permalink / raw)
To: Junhao He, mark.rutland, linux-kernel, jonathan.cameron
Cc: catalin.marinas, kernel-team, Will Deacon, yangyicong, shenyang39,
f.fangjian, linuxarm, linux-arm-kernel, prime.zeng
On Mon, 3 Apr 2023 16:14:21 +0800, Junhao He wrote:
> Fix NULL pointer and cleanup redundant initialized.
>
> Junhao He (2):
> drivers/perf: hisi: Remove redundant initialized of pmu->name
> drivers/perf: hisi: add NULL check for name
>
> drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 19 +++++++++++--------
> drivers/perf/hisilicon/hisi_uncore_hha_pmu.c | 9 ++++++---
> drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 13 ++++++-------
> drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 2 +-
> drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 +---
> drivers/perf/hisilicon/hisi_uncore_pmu.h | 3 +--
> drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 2 +-
> 8 files changed, 28 insertions(+), 26 deletions(-)
>
> [...]
Applied to will (for-next/perf), thanks!
[1/2] drivers/perf: hisi: Remove redundant initialized of pmu->name
https://git.kernel.org/will/c/25d8c25025a4
[2/2] drivers/perf: hisi: add NULL check for name
https://git.kernel.org/will/c/257aedb72e73
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread