On Thu, Nov 06, 2014 at 04:57:14PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> The DRM driver's ->load() implementation didn't do a good job (no job at
> all really) cleaning up on failure. Fix that by undoing any prior setup
> when an error occurs. This requires a bit of rework to make it possible
> to clean up fbdev midway.
>
> This was tested by injecting errors at various points during the
> initialization sequence and verifying that error cleanup didn't crash
> and no memory leaked (using kmemleak).
>
> Reported-by: Stéphane Marchesin <marcheu@xxxxxxxxxx>
> Reported-by: Sean Paul <seanpaul@xxxxxxxxxx>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
Looks much better :-)
Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/tegra/drm.c | 20 ++++++++++++++++----
> drivers/gpu/drm/tegra/drm.h | 1 +
> drivers/gpu/drm/tegra/fb.c | 20 +++++++++++++++++---
> 3 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 59736bb810cd..e1632fb03a89 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -42,13 +42,13 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
>
> err = tegra_drm_fb_prepare(drm);
> if (err < 0)
> - return err;
> + goto config;
>
> drm_kms_helper_poll_init(drm);
>
> err = host1x_device_init(device);
> if (err < 0)
> - return err;
> + goto fbdev;
>
> /*
> * We don't use the drm_irq_install() helpers provided by the DRM
> @@ -59,13 +59,25 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
>
> err = drm_vblank_init(drm, drm->mode_config.num_crtc);
> if (err < 0)
> - return err;
> + goto device;
>
> err = tegra_drm_fb_init(drm);
> if (err < 0)
> - return err;
> + goto vblank;
>
> return 0;
> +
> +vblank:
> + drm_vblank_cleanup(drm);
> +device:
> + host1x_device_exit(device);
> +fbdev:
> + drm_kms_helper_poll_fini(drm);
> + tegra_drm_fb_free(drm);
> +config:
> + drm_mode_config_cleanup(drm);
> + kfree(tegra);
> + return err;
> }
>
> static int tegra_drm_unload(struct drm_device *drm)
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index b994c017971d..ef2faaef5936 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -288,6 +288,7 @@ bool tegra_fb_is_bottom_up(struct drm_framebuffer *framebuffer);
> int tegra_fb_get_tiling(struct drm_framebuffer *framebuffer,
> struct tegra_bo_tiling *tiling);
> int tegra_drm_fb_prepare(struct drm_device *drm);
> +void tegra_drm_fb_free(struct drm_device *drm);
> int tegra_drm_fb_init(struct drm_device *drm);
> void tegra_drm_fb_exit(struct drm_device *drm);
> #ifdef CONFIG_DRM_TEGRA_FBDEV
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index 3513d12d5aa1..0474241ac2a4 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -289,6 +289,11 @@ static struct tegra_fbdev *tegra_fbdev_create(struct drm_device *drm)
> return fbdev;
> }
>
> +static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
> +{
> + kfree(fbdev);
> +}
> +
> static int tegra_fbdev_init(struct tegra_fbdev *fbdev,
> unsigned int preferred_bpp,
> unsigned int num_crtc,
> @@ -322,7 +327,7 @@ fini:
> return err;
> }
>
> -static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
> +static void tegra_fbdev_exit(struct tegra_fbdev *fbdev)
> {
> struct fb_info *info = fbdev->base.fbdev;
>
> @@ -345,7 +350,7 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
> }
>
> drm_fb_helper_fini(&fbdev->base);
> - kfree(fbdev);
> + tegra_fbdev_free(fbdev);
> }
>
> void tegra_fbdev_restore_mode(struct tegra_fbdev *fbdev)
> @@ -393,6 +398,15 @@ int tegra_drm_fb_prepare(struct drm_device *drm)
> return 0;
> }
>
> +void tegra_drm_fb_free(struct drm_device *drm)
> +{
> +#ifdef CONFIG_DRM_TEGRA_FBDEV
> + struct tegra_drm *tegra = drm->dev_private;
> +
> + tegra_fbdev_free(tegra->fbdev);
> +#endif
> +}
> +
> int tegra_drm_fb_init(struct drm_device *drm)
> {
> #ifdef CONFIG_DRM_TEGRA_FBDEV
> @@ -413,6 +427,6 @@ void tegra_drm_fb_exit(struct drm_device *drm)
> #ifdef CONFIG_DRM_TEGRA_FBDEV
> struct tegra_drm *tegra = drm->dev_private;
>
> - tegra_fbdev_free(tegra->fbdev);
> + tegra_fbdev_exit(tegra->fbdev);
> #endif
> }
> --
> 2.1.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
