[PATCH 1/3] drm/exynos: free DP if probe fails to find a panel or bridge

[PATCH 1/3] drm/exynos: free DP if probe fails to find a panel or bridge

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

DP was leaked everytime function returns EPROBE_DEFER, free it before
returning.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 85762cf..6fd4a46 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
 	if (panel_node) {
 		dp->panel = of_drm_find_panel(panel_node);
 		of_node_put(panel_node);
-		if (!dp->panel)
-			return -EPROBE_DEFER;
+		if (!dp->panel) {
+			ret = -EPROBE_DEFER;
+			goto free_dp;
+		}
 	}
 
 	endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
@@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
 		if (bridge_node) {
 			dp->bridge = of_drm_find_bridge(bridge_node);
 			of_node_put(bridge_node);
-			if (!dp->bridge)
-				return -EPROBE_DEFER;
-		} else
-			return -EPROBE_DEFER;
+			if (!dp->bridge) {
+				ret = -EPROBE_DEFER;
+				goto free_dp;
+			}
+		} else {
+			ret = -EPROBE_DEFER;
+			goto free_dp;
+		}
 	}
 
 	exynos_dp_display.ctx = dp;
@@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
 		exynos_drm_component_del(&pdev->dev,
 						EXYNOS_DEVICE_TYPE_CONNECTOR);
 
+free_dp:
+	devm_kfree(dev, dp);
+
 	return ret;
 }
 
-- 
1.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/exynos: avoid race condition when adding a drm component

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

exynos_drm_component_add() correctly checks if a component is present on
drm_component_list however it release the lock right after the check
and before we add the new component to the list. That just creates room
to add the same component more than once to the list.

The lock should be held for the whole journey while adding a new
component.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index cb3ed9b..230573d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -402,10 +402,8 @@ int exynos_drm_component_add(struct device *dev,
 			 * added already just return.
 			 */
 			if (cdev->dev_type_flag == (EXYNOS_DEVICE_TYPE_CRTC |
-						EXYNOS_DEVICE_TYPE_CONNECTOR)) {
-				mutex_unlock(&drm_component_lock);
-				return 0;
-			}
+						EXYNOS_DEVICE_TYPE_CONNECTOR))
+				goto unlock;
 
 			if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
 				cdev->crtc_dev = dev;
@@ -417,14 +415,11 @@ int exynos_drm_component_add(struct device *dev,
 				cdev->dev_type_flag |= dev_type;
 			}
 
-			mutex_unlock(&drm_component_lock);
-			return 0;
+			goto unlock;
 		}
 	}
 
-	mutex_unlock(&drm_component_lock);
-
-	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
+	cdev = kzalloc(sizeof(*cdev), GFP_ATOMIC);
 	if (!cdev)
 		return -ENOMEM;
 
@@ -436,10 +431,9 @@ int exynos_drm_component_add(struct device *dev,
 	cdev->out_type = out_type;
 	cdev->dev_type_flag = dev_type;
 
-	mutex_lock(&drm_component_lock);
 	list_add_tail(&cdev->list, &drm_component_list);
+unlock:
 	mutex_unlock(&drm_component_lock);
-
 	return 0;
 }
 
-- 
1.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/exynos: avoid leak if exynos_dpi_probe() fails

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The component must be deleted if the probe fails.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 0673a39..173d135 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -1202,8 +1202,10 @@ static int fimd_probe(struct platform_device *pdev)
 	fimd_manager.ctx = ctx;
 
 	ctx->display = exynos_dpi_probe(dev);
-	if (IS_ERR(ctx->display))
-		return PTR_ERR(ctx->display);
+	if (IS_ERR(ctx->display)) {
+		ret = PTR_ERR(ctx->display);
+		goto err_del_component;
+	}
 
 	pm_runtime_enable(&pdev->dev);
 
-- 
1.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/exynos: free DP if probe fails to find a panel or bridge

[Ajay kumar]

Hi Gustavo,


On Fri, Nov 21, 2014 at 5:24 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> DP was leaked everytime function returns EPROBE_DEFER, free it before
> returning.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index 85762cf..6fd4a46 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
>         if (panel_node) {
>                 dp->panel = of_drm_find_panel(panel_node);
>                 of_node_put(panel_node);
> -               if (!dp->panel)
> -                       return -EPROBE_DEFER;
> +               if (!dp->panel) {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 if (bridge_node) {
>                         dp->bridge = of_drm_find_bridge(bridge_node);
>                         of_node_put(bridge_node);
> -                       if (!dp->bridge)
> -                               return -EPROBE_DEFER;
> -               } else
> -                       return -EPROBE_DEFER;
> +                       if (!dp->bridge) {
> +                               ret = -EPROBE_DEFER;
> +                               goto free_dp;
> +                       }
> +               } else {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         exynos_dp_display.ctx = dp;
> @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 exynos_drm_component_del(&pdev->dev,
>                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
>
> +free_dp:
> +       devm_kfree(dev, dp);
I guess the driver core takes care of freeing the devm memory when the
probe fails?
Will it not happen during PROBE_DEFER?

Inki/Jingoo - Is this change really necessary?

Ajay

>         return ret;
>  }
>
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/exynos: free DP if probe fails to find a panel or bridge

[Inki Dae]

2014-11-21 8:54 GMT+09:00 Gustavo Padovan <gustavo@padovan.org>:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> DP was leaked everytime function returns EPROBE_DEFER, free it before
> returning.

It seems that you misunderstood devm_* api.

Thanks,
Inki Dae

>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index 85762cf..6fd4a46 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
>         if (panel_node) {
>                 dp->panel = of_drm_find_panel(panel_node);
>                 of_node_put(panel_node);
> -               if (!dp->panel)
> -                       return -EPROBE_DEFER;
> +               if (!dp->panel) {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 if (bridge_node) {
>                         dp->bridge = of_drm_find_bridge(bridge_node);
>                         of_node_put(bridge_node);
> -                       if (!dp->bridge)
> -                               return -EPROBE_DEFER;
> -               } else
> -                       return -EPROBE_DEFER;
> +                       if (!dp->bridge) {
> +                               ret = -EPROBE_DEFER;
> +                               goto free_dp;
> +                       }
> +               } else {
> +                       ret = -EPROBE_DEFER;
> +                       goto free_dp;
> +               }
>         }
>
>         exynos_dp_display.ctx = dp;
> @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
>                 exynos_drm_component_del(&pdev->dev,
>                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
>
> +free_dp:
> +       devm_kfree(dev, dp);
> +
>         return ret;
>  }
>
> --
> 1.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/exynos: free DP if probe fails to find a panel or bridge

[Jingoo Han]

On Saturday, November 22, 2014 2:40 AM, Ajay kumar wrote:
> 
> On Fri, Nov 21, 2014 at 5:24 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > DP was leaked everytime function returns EPROBE_DEFER, free it before
> > returning.
> >
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > index 85762cf..6fd4a46 100644
> > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >         if (panel_node) {
> >                 dp->panel = of_drm_find_panel(panel_node);
> >                 of_node_put(panel_node);
> > -               if (!dp->panel)
> > -                       return -EPROBE_DEFER;
> > +               if (!dp->panel) {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 if (bridge_node) {
> >                         dp->bridge = of_drm_find_bridge(bridge_node);
> >                         of_node_put(bridge_node);
> > -                       if (!dp->bridge)
> > -                               return -EPROBE_DEFER;
> > -               } else
> > -                       return -EPROBE_DEFER;
> > +                       if (!dp->bridge) {
> > +                               ret = -EPROBE_DEFER;
> > +                               goto free_dp;
> > +                       }
> > +               } else {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         exynos_dp_display.ctx = dp;
> > @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 exynos_drm_component_del(&pdev->dev,
> >                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
> >
> > +free_dp:
> > +       devm_kfree(dev, dp);
> I guess the driver core takes care of freeing the devm memory when the
> probe fails?
> Will it not happen during PROBE_DEFER?
> 
> Inki/Jingoo - Is this change really necessary?

As far as I know, this devm_kfree() in unnecessary,
even though PROBE_DEFER is returned.
If I am wrong, please let me know. Thanks.

Best regards,
Jingoo Han

> 
> Ajay
> 
> >         return ret;
> >  }
> >
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drm/exynos: free DP if probe fails to find a panel or bridge

[Gustavo Padovan]

2014-11-22 Inki Dae <inki.dae@samsung.com>:

> 2014-11-21 8:54 GMT+09:00 Gustavo Padovan <gustavo@padovan.org>:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > DP was leaked everytime function returns EPROBE_DEFER, free it before
> > returning.
> 
> It seems that you misunderstood devm_* api.

Yeah, I though it would only free memory at unload of the module but it also
free it when the probe fails.

> 
> Thanks,
> Inki Dae
> 
> >
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  drivers/gpu/drm/exynos/exynos_dp_core.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > index 85762cf..6fd4a46 100644
> > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> > @@ -1336,8 +1336,10 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >         if (panel_node) {
> >                 dp->panel = of_drm_find_panel(panel_node);
> >                 of_node_put(panel_node);
> > -               if (!dp->panel)
> > -                       return -EPROBE_DEFER;
> > +               if (!dp->panel) {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
> > @@ -1346,10 +1348,14 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 if (bridge_node) {
> >                         dp->bridge = of_drm_find_bridge(bridge_node);
> >                         of_node_put(bridge_node);
> > -                       if (!dp->bridge)
> > -                               return -EPROBE_DEFER;
> > -               } else
> > -                       return -EPROBE_DEFER;
> > +                       if (!dp->bridge) {
> > +                               ret = -EPROBE_DEFER;
> > +                               goto free_dp;
> > +                       }
> > +               } else {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto free_dp;
> > +               }
> >         }
> >
> >         exynos_dp_display.ctx = dp;
> > @@ -1359,6 +1365,9 @@ static int exynos_dp_probe(struct platform_device *pdev)
> >                 exynos_drm_component_del(&pdev->dev,
> >                                                 EXYNOS_DEVICE_TYPE_CONNECTOR);
> >
> > +free_dp:
> > +       devm_kfree(dev, dp);
> > +
> >         return ret;
> >  }
> >
> > --
> > 1.9.3
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel

	Gustavo