From: Dave Airlie airlied@redhat.com

If we unplug a udl device, the usb callback with deinit the mode_config struct, however userspace will still have an open file descriptor and a framebuffer on that device. When userspace closes the fd, we'll oops because it'll try and look stuff up in the object idr which we've destroyed.

This punts destroying the mode objects until release time instead.

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/udl/udl_drv.c | 1 + drivers/gpu/drm/udl/udl_drv.h | 1 + drivers/gpu/drm/udl/udl_main.c | 8 +++++++- 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index 22cd2d13e272..ff47f890e6ad 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -52,6 +52,7 @@ static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, .load = udl_driver_load, .unload = udl_driver_unload, + .release = udl_driver_release,

/* gem hooks */ .gem_free_object_unlocked = udl_gem_free_object, diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index e9e9b1ff678e..4ae67d882eae 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -104,6 +104,7 @@ void udl_urb_completion(struct urb *urb);

int udl_driver_load(struct drm_device *dev, unsigned long flags); void udl_driver_unload(struct drm_device *dev); +void udl_driver_release(struct drm_device *dev);

int udl_fbdev_init(struct drm_device *dev); void udl_fbdev_cleanup(struct drm_device *dev); diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 9086d0d1b880..1f8ef34ade24 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -379,6 +379,12 @@ void udl_driver_unload(struct drm_device *dev) udl_free_urb_list(dev);

udl_fbdev_cleanup(dev); - udl_modeset_cleanup(dev); kfree(udl); } + +void udl_driver_release(struct drm_device *dev) +{ + udl_modeset_cleanup(dev); + drm_dev_fini(dev); + kfree(dev); +}

From: Dave Airlie airlied@redhat.com

This should help with some of the lifetime issues, and move us away from load/unload.

Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/udl/udl_drv.c | 56 +++++++++++++++++++++++++++------- drivers/gpu/drm/udl/udl_drv.h | 9 +++--- drivers/gpu/drm/udl/udl_fb.c | 2 +- drivers/gpu/drm/udl/udl_main.c | 23 ++------------ 4 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index ff47f890e6ad..15c0e3eeaf3b 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -48,10 +48,16 @@ static const struct file_operations udl_driver_fops = { .llseek = noop_llseek, };

+static void udl_driver_release(struct drm_device *dev) +{ + udl_fini(dev); + udl_modeset_cleanup(dev); + drm_dev_fini(dev); + kfree(dev); +} + static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, - .load = udl_driver_load, - .unload = udl_driver_unload, .release = udl_driver_release,

/* gem hooks */ @@ -75,28 +81,56 @@ static struct drm_driver driver = { .patchlevel = DRIVER_PATCHLEVEL, };

+static struct udl_device *udl_driver_create(struct usb_interface *interface) +{ + struct usb_device *udev = interface_to_usbdev(interface); + struct udl_device *udl; + int r; + + udl = kzalloc(sizeof(struct udl_device), GFP_KERNEL); + if (!udl) + return ERR_PTR(-ENOMEM); + + r = drm_dev_init(&udl->drm, &driver, &interface->dev); + if (r) { + kfree(udl); + return ERR_PTR(r); + } + + udl->udev = udev; + udl->drm.dev_private = udl; + + r = udl_init(udl); + if (r) { + drm_dev_fini(&udl->drm); + kfree(udl); + return ERR_PTR(r); + } + + usb_set_intfdata(interface, udl); + return udl; +} + + static int udl_usb_probe(struct usb_interface *interface, const struct usb_device_id *id) { - struct usb_device *udev = interface_to_usbdev(interface); - struct drm_device *dev; int r;

- dev = drm_dev_alloc(&driver, &interface->dev); - if (IS_ERR(dev)) - return PTR_ERR(dev); + struct udl_device *udl = udl_driver_create(interface); + if (IS_ERR(udl)) + return PTR_ERR(udl);

- r = drm_dev_register(dev, (unsigned long)udev); + r = drm_dev_register(&udl->drm, 0); if (r) goto err_free;

- usb_set_intfdata(interface, dev); - DRM_INFO("Initialized udl on minor %d\n", dev->primary->index); + DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);

return 0;

err_free: - drm_dev_put(dev); + drm_dev_put(&udl->drm); return r; }

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index b3e08e876d62..35c1f33fbc1a 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -50,8 +50,8 @@ struct urb_list { struct udl_fbdev;

struct udl_device { + struct drm_device drm; struct device *dev; - struct drm_device *ddev; struct usb_device *udev; struct drm_crtc *crtc;

@@ -71,7 +71,7 @@ struct udl_device { atomic_t cpu_kcycles_used; /* transpired during pixel processing */ };

-#define to_udl(x) ((x)->dev_private) +#define to_udl(x) container_of(x, struct udl_device, drm)

struct udl_gem_object { struct drm_gem_object base; @@ -104,9 +104,8 @@ struct urb *udl_get_urb(struct drm_device *dev); int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len); void udl_urb_completion(struct urb *urb);

-int udl_driver_load(struct drm_device *dev, unsigned long flags); -void udl_driver_unload(struct drm_device *dev); -void udl_driver_release(struct drm_device *dev); +int udl_init(struct udl_device *udl); +void udl_fini(struct drm_device *dev);

int udl_fbdev_init(struct drm_device *dev); void udl_fbdev_cleanup(struct drm_device *dev); diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index 590323ea261f..4ab101bf1df0 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -213,7 +213,7 @@ static int udl_fb_open(struct fb_info *info, int user) struct udl_device *udl = to_udl(dev);

/* If the USB device is gone, we don't accept new opens */ - if (drm_dev_is_unplugged(udl->ddev)) + if (drm_dev_is_unplugged(&udl->drm)) return -ENODEV;

ufbdev->fb_count++; diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 862c099d7c4c..6743eaef4594 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -311,20 +311,12 @@ int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len) return ret; }

-int udl_driver_load(struct drm_device *dev, unsigned long flags) +int udl_init(struct udl_device *udl) { - struct usb_device *udev = (void*)flags; - struct udl_device *udl; + struct drm_device *dev = &udl->drm; int ret = -ENOMEM;

DRM_DEBUG("\n"); - udl = kzalloc(sizeof(struct udl_device), GFP_KERNEL); - if (!udl) - return -ENOMEM; - - udl->udev = udev; - udl->ddev = dev; - dev->dev_private = udl;

mutex_init(&udl->gem_lock);

@@ -358,7 +350,6 @@ int udl_driver_load(struct drm_device *dev, unsigned long flags) err: if (udl->urbs.count) udl_free_urb_list(dev); - kfree(udl); DRM_ERROR("%d\n", ret); return ret; } @@ -369,7 +360,7 @@ int udl_drop_usb(struct drm_device *dev) return 0; }

-void udl_driver_unload(struct drm_device *dev) +void udl_fini(struct drm_device *dev) { struct udl_device *udl = to_udl(dev);

@@ -379,12 +370,4 @@ void udl_driver_unload(struct drm_device *dev) udl_free_urb_list(dev);

udl_fbdev_cleanup(dev); - kfree(udl); -} - -void udl_driver_release(struct drm_device *dev) -{ - udl_modeset_cleanup(dev); - drm_dev_fini(dev); - kfree(dev); }