Re: [PATCH 05/24] drm/omap: Use devm_kzalloc() to allocate omap_drm_private

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Monday, 12 February 2018 11:44:35 EET Tomi Valkeinen wrote:
> From: Peter Ujfalusi 
> 
> It makes the cleanup paths a bit cleaner.

But it also goes in the wrong direction. The data structure is accessible 
after the .remove() handler returns and thus should outlive the probe/remove 
sequence through proper reference counting. We're of course not doing a good 
job here as we kfree() it in .remove() instead of reference-counting it 
properly, and that should be fixed, but this patch makes the fix more complex 
as we'll have to move back from devm_kzalloc().

> Signed-off-by: Peter Ujfalusi 
> Signed-off-by: Tomi Valkeinen 
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 485684c637ff..3cd9188ab30b
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -529,19 +529,17 @@ static int pdev_probe(struct platform_device *pdev)
>   return ret;
>   }
> 
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
>   omap_crtc_pre_init();
> 
>   ret = omap_connect_dssdevs();
>   if (ret)
>   goto err_crtc_uninit;
> 
> - /* Allocate and initialize the driver private structure. */
> - priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> - if (!priv) {
> - ret = -ENOMEM;
> - goto err_disconnect_dssdevs;
> - }
> -
> + /* Initialize the driver private structure. */
>   priv->dispc_ops = dispc_get_ops();
> 
>   soc = soc_device_match(omapdrm_soc_devices);
> @@ -555,7 +553,7 @@ static int pdev_probe(struct platform_device *pdev)
>   ddev = drm_dev_alloc(_drm_driver, >dev);
>   if (IS_ERR(ddev)) {
>   ret = PTR_ERR(ddev);
> - goto err_free_priv;
> + goto err_destroy_wq;
>   }
> 
>   ddev->dev_private = priv;
> @@ -610,10 +608,8 @@ static int pdev_probe(struct platform_device *pdev)
>  err_free_drm_dev:
>   omap_gem_deinit(ddev);
>   drm_dev_unref(ddev);
> -err_free_priv:
> +err_destroy_wq:
>   destroy_workqueue(priv->wq);
> - kfree(priv);
> -err_disconnect_dssdevs:
>   omap_disconnect_dssdevs();
>  err_crtc_uninit:
>   omap_crtc_pre_uninit();
> @@ -644,7 +640,6 @@ static int pdev_remove(struct platform_device *pdev)
>   drm_dev_unref(ddev);
> 
>   destroy_workqueue(priv->wq);
> - kfree(priv);
> 
>   omap_disconnect_dssdevs();
>   omap_crtc_pre_uninit();

-- 
Regards,

Laurent Pinchart

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/24] drm/omap: Use devm_kzalloc() to allocate omap_drm_private

[Tomi Valkeinen]

From: Peter Ujfalusi 

It makes the cleanup paths a bit cleaner.

Signed-off-by: Peter Ujfalusi 
Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 485684c637ff..3cd9188ab30b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -529,19 +529,17 @@ static int pdev_probe(struct platform_device *pdev)
return ret;
}
 
+   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
omap_crtc_pre_init();
 
ret = omap_connect_dssdevs();
if (ret)
goto err_crtc_uninit;
 
-   /* Allocate and initialize the driver private structure. */
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-   if (!priv) {
-   ret = -ENOMEM;
-   goto err_disconnect_dssdevs;
-   }
-
+   /* Initialize the driver private structure. */
priv->dispc_ops = dispc_get_ops();
 
soc = soc_device_match(omapdrm_soc_devices);
@@ -555,7 +553,7 @@ static int pdev_probe(struct platform_device *pdev)
ddev = drm_dev_alloc(_drm_driver, >dev);
if (IS_ERR(ddev)) {
ret = PTR_ERR(ddev);
-   goto err_free_priv;
+   goto err_destroy_wq;
}
 
ddev->dev_private = priv;
@@ -610,10 +608,8 @@ static int pdev_probe(struct platform_device *pdev)
 err_free_drm_dev:
omap_gem_deinit(ddev);
drm_dev_unref(ddev);
-err_free_priv:
+err_destroy_wq:
destroy_workqueue(priv->wq);
-   kfree(priv);
-err_disconnect_dssdevs:
omap_disconnect_dssdevs();
 err_crtc_uninit:
omap_crtc_pre_uninit();
@@ -644,7 +640,6 @@ static int pdev_remove(struct platform_device *pdev)
drm_dev_unref(ddev);
 
destroy_workqueue(priv->wq);
-   kfree(priv);
 
omap_disconnect_dssdevs();
omap_crtc_pre_uninit();
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel