| Message ID | 3cff630697d76d24d4ab93dfcddc978d84b8f2d8.1576711200.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | perf/imx_ddr: Fix leaking cpuhp_state | expand |
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
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
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);
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(-)