* [PATCH] perf/imx_ddr: Fix leaking cpuhp_state
@ 2019-12-18 23:20 Leonard Crestez
2020-01-13 20:00 ` Leonard Crestez
2020-01-14 16:51 ` Will Deacon
0 siblings, 2 replies; 3+ messages in thread
From: Leonard Crestez @ 2019-12-18 23:20 UTC (permalink / raw)
To: Joakim Zhang, Frank Li, Will Deacon
Cc: Mark Rutland, Dong Aisheng, linux-imx, kernel, Fabio Estevam,
Shawn Guo, linux-arm-kernel
This driver allocates a dynamic cpu hotplug state but never releases it.
If reloaded in a loop it will quickly trigger a WARN message:
"No more dynamic states available for CPU hotplug"
Fix by calling cpuhp_remove_multi_state like several other perf pmu
drivers.
Fixes: 8f4c58ae59f5 ("perf/imx_ddr: Fix leaking cpuhp_state")
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/perf/fsl_imx8_ddr_perf.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
index 3be61be1ba91..aa30ca5f6b29 100644
--- a/drivers/perf/fsl_imx8_ddr_perf.c
+++ b/drivers/perf/fsl_imx8_ddr_perf.c
@@ -672,12 +672,14 @@ static int ddr_perf_probe(struct platform_device *pdev)
goto ddr_perf_err;
return 0;
ddr_perf_err:
- if (pmu->cpuhp_state)
+ if (pmu->cpuhp_state) {
cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+ cpuhp_remove_multi_state(pmu->cpuhp_state);
+ }
ida_simple_remove(&ddr_ida, pmu->id);
dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret);
return ret;
}
@@ -685,10 +687,11 @@ static int ddr_perf_probe(struct platform_device *pdev)
static int ddr_perf_remove(struct platform_device *pdev)
{
struct ddr_pmu *pmu = platform_get_drvdata(pdev);
cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+ cpuhp_remove_multi_state(pmu->cpuhp_state);
irq_set_affinity_hint(pmu->irq, NULL);
perf_pmu_unregister(&pmu->pmu);
ida_simple_remove(&ddr_ida, pmu->id);
--
2.17.1
_______________________________________________
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] 3+ messages in thread* Re: [PATCH] perf/imx_ddr: Fix leaking cpuhp_state
2019-12-18 23:20 [PATCH] perf/imx_ddr: Fix leaking cpuhp_state Leonard Crestez
@ 2020-01-13 20:00 ` Leonard Crestez
2020-01-14 16:51 ` Will Deacon
1 sibling, 0 replies; 3+ messages in thread
From: Leonard Crestez @ 2020-01-13 20:00 UTC (permalink / raw)
To: Joakim Zhang, Frank Li, Will Deacon
Cc: Mark Rutland, Aisheng Dong, dl-linux-imx, kernel@pengutronix.de,
Fabio Estevam, Shawn Guo, linux-arm-kernel@lists.infradead.org
On 19.12.2019 01:20, Leonard Crestez wrote:
> This driver allocates a dynamic cpu hotplug state but never releases it.
> If reloaded in a loop it will quickly trigger a WARN message:
>
> "No more dynamic states available for CPU hotplug"
>
> Fix by calling cpuhp_remove_multi_state like several other perf pmu
> drivers.
>
> Fixes: 8f4c58ae59f5 ("perf/imx_ddr: Fix leaking cpuhp_state")
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > ---
> drivers/perf/fsl_imx8_ddr_perf.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index 3be61be1ba91..aa30ca5f6b29 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -672,12 +672,14 @@ static int ddr_perf_probe(struct platform_device *pdev)
> goto ddr_perf_err;
>
> return 0;
>
> ddr_perf_err:
> - if (pmu->cpuhp_state)
> + if (pmu->cpuhp_state) {
> cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> + cpuhp_remove_multi_state(pmu->cpuhp_state);
> + }
>
> ida_simple_remove(&ddr_ida, pmu->id);
> dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret);
> return ret;
> }
> @@ -685,10 +687,11 @@ static int ddr_perf_probe(struct platform_device *pdev)
> static int ddr_perf_remove(struct platform_device *pdev)
> {
> struct ddr_pmu *pmu = platform_get_drvdata(pdev);
>
> cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> + cpuhp_remove_multi_state(pmu->cpuhp_state);
> irq_set_affinity_hint(pmu->irq, NULL);
>
> perf_pmu_unregister(&pmu->pmu);
>
> ida_simple_remove(&ddr_ida, pmu->id);
>
Gentle ping?
I believe this got lost over the holidays.
--
Regards,
Leonard
_______________________________________________
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] 3+ messages in thread* Re: [PATCH] perf/imx_ddr: Fix leaking cpuhp_state
2019-12-18 23:20 [PATCH] perf/imx_ddr: Fix leaking cpuhp_state Leonard Crestez
2020-01-13 20:00 ` Leonard Crestez
@ 2020-01-14 16:51 ` Will Deacon
1 sibling, 0 replies; 3+ messages in thread
From: Will Deacon @ 2020-01-14 16:51 UTC (permalink / raw)
To: Leonard Crestez
Cc: Mark Rutland, Dong Aisheng, Frank Li, Joakim Zhang, linux-imx,
kernel, Fabio Estevam, Shawn Guo, linux-arm-kernel
On Thu, Dec 19, 2019 at 01:20:19AM +0200, Leonard Crestez wrote:
> This driver allocates a dynamic cpu hotplug state but never releases it.
> If reloaded in a loop it will quickly trigger a WARN message:
>
> "No more dynamic states available for CPU hotplug"
>
> Fix by calling cpuhp_remove_multi_state like several other perf pmu
> drivers.
>
> Fixes: 8f4c58ae59f5 ("perf/imx_ddr: Fix leaking cpuhp_state")
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
> drivers/perf/fsl_imx8_ddr_perf.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index 3be61be1ba91..aa30ca5f6b29 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -672,12 +672,14 @@ static int ddr_perf_probe(struct platform_device *pdev)
> goto ddr_perf_err;
>
> return 0;
>
> ddr_perf_err:
> - if (pmu->cpuhp_state)
> + if (pmu->cpuhp_state) {
> cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> + cpuhp_remove_multi_state(pmu->cpuhp_state);
> + }
Shouldn't we also be checking the return value of the earlier call to
cpuhp_state_add_instant_nocalls() and then calling only
cpuhp_remove_multi_state() if it fails?
Thanks,
Will
_______________________________________________
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] 3+ messages in thread
end of thread, other threads:[~2020-01-14 16:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-18 23:20 [PATCH] perf/imx_ddr: Fix leaking cpuhp_state Leonard Crestez
2020-01-13 20:00 ` Leonard Crestez
2020-01-14 16:51 ` Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox