| Message ID | 1386202780-25731-1-git-send-email-imirkin@alum.mit.edu (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi On Thu, Dec 5, 2013 at 1:19 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote: > All instances of drm_dev_register are followed by drm_dev_free on > failure. Don't free dev->control/render/primary on failure, as they will > be freed by drm_dev_free since commit 8f6599da8e (drm: delay minor > destruction to drm_dev_free()). > > Reported-by: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> > --- > > Bruno, would be nice if you could test this out on your setup without > the patch that makes load succeed. Ideally this will make drm not die > in the case that nouveau load fails. > > drivers/gpu/drm/drm_stub.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c > index f53d524..cd427b8 100644 > --- a/drivers/gpu/drm/drm_stub.c > +++ b/drivers/gpu/drm/drm_stub.c > @@ -536,17 +536,17 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) { > ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER); > if (ret) > - goto err_control_node; > + goto err_agp; > } > > ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY); > if (ret) > - goto err_render_node; > + goto err_agp; > > if (dev->driver->load) { > ret = dev->driver->load(dev, flags); > if (ret) > - goto err_primary_node; > + goto err_agp; > } > > /* setup grouping for legacy outputs */ > @@ -565,12 +565,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > err_unload: > if (dev->driver->unload) > dev->driver->unload(dev); > -err_primary_node: > - drm_put_minor(dev->primary); > -err_render_node: > - drm_put_minor(dev->render); > -err_control_node: > - drm_put_minor(dev->control); No, please use drm_unplug_minor() here. Otherwise, our unload ordering is wrong. Nice catch, David > err_agp: > if (dev->driver->bus->agp_destroy) > dev->driver->bus->agp_destroy(dev); > -- > 1.8.3.2 >
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index f53d524..cd427b8 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -536,17 +536,17 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) { ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER); if (ret) - goto err_control_node; + goto err_agp; } ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY); if (ret) - goto err_render_node; + goto err_agp; if (dev->driver->load) { ret = dev->driver->load(dev, flags); if (ret) - goto err_primary_node; + goto err_agp; } /* setup grouping for legacy outputs */ @@ -565,12 +565,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) err_unload: if (dev->driver->unload) dev->driver->unload(dev); -err_primary_node: - drm_put_minor(dev->primary); -err_render_node: - drm_put_minor(dev->render); -err_control_node: - drm_put_minor(dev->control); err_agp: if (dev->driver->bus->agp_destroy) dev->driver->bus->agp_destroy(dev);
All instances of drm_dev_register are followed by drm_dev_free on failure. Don't free dev->control/render/primary on failure, as they will be freed by drm_dev_free since commit 8f6599da8e (drm: delay minor destruction to drm_dev_free()). Reported-by: Bruno Prémont <bonbons@linux-vserver.org> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> --- Bruno, would be nice if you could test this out on your setup without the patch that makes load succeed. Ideally this will make drm not die in the case that nouveau load fails. drivers/gpu/drm/drm_stub.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)