Message ID | 1415289436-14797-1-git-send-email-thierry.reding@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Nov 06, 2014 at 04:57:14PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > 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@google.com> > Reported-by: Sean Paul <seanpaul@google.com> > Signed-off-by: Thierry Reding <treding@nvidia.com> Looks much better :-) Reviewed-by: Sean Paul <seanpaul@chromium.org> > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
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 }