linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [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, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

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;
as well as URLs for NNTP newsgroup(s).