drm: manage drm_minor cleanup with drmm_

The cleanup here is somewhat tricky, since we can't tell apart the
allocated minor index from 0. So register a cleanup action first, and
if the index allocation fails, unregister that cleanup action again to
avoid bad mistakes.

The kdev for the minor already handles NULL, so no problem there.

Hence add drmm_remove_action() to the drm_managed library.

v2: Make pointer math around void ** consistent with what Laurent
suggested.

v3: Use drmm_add_action_or_reset and remove drmm_remove_action. Noticed
because of some questions from Thomas. This also means we need to move
the drmm_add_action_or_reset helper earlier in the series.

v4: Uh ... fix slightly embarrassing bug CI spotted.

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200324203936.3330994-1-daniel.vetter@ffwll.ch

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index d1e930a1fc62d4e6d3985846bbd0a6570ce4d149..60a17de75ad07f3e861bae24d3ecf6425a2ef5a8 100644 (file)
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -93,13 +93,25 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
        }
 }
 
+static void drm_minor_alloc_release(struct drm_device *dev, void *data)
+{
+       struct drm_minor *minor = data;
+       unsigned long flags;
+
+       put_device(minor->kdev);
+
+       spin_lock_irqsave(&drm_minor_lock, flags);
+       idr_remove(&drm_minors_idr, minor->index);
+       spin_unlock_irqrestore(&drm_minor_lock, flags);
+}
+
 static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
 {
        struct drm_minor *minor;
        unsigned long flags;
        int r;
 
-       minor = kzalloc(sizeof(*minor), GFP_KERNEL);
+       minor = drmm_kzalloc(dev, sizeof(*minor), GFP_KERNEL);
        if (!minor)
                return -ENOMEM;
 
@@ -117,46 +129,20 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
        idr_preload_end();
 
        if (r < 0)
-               goto err_free;
+               return r;
 
        minor->index = r;
 
+       r = drmm_add_action_or_reset(dev, drm_minor_alloc_release, minor);
+       if (r)
+               return r;
+
        minor->kdev = drm_sysfs_minor_alloc(minor);
-       if (IS_ERR(minor->kdev)) {
-               r = PTR_ERR(minor->kdev);
-               goto err_index;
-       }
+       if (IS_ERR(minor->kdev))
+               return PTR_ERR(minor->kdev);
 
        *drm_minor_get_slot(dev, type) = minor;
        return 0;
-
-err_index:
-       spin_lock_irqsave(&drm_minor_lock, flags);
-       idr_remove(&drm_minors_idr, minor->index);
-       spin_unlock_irqrestore(&drm_minor_lock, flags);
-err_free:
-       kfree(minor);
-       return r;
-}
-
-static void drm_minor_free(struct drm_device *dev, unsigned int type)
-{
-       struct drm_minor **slot, *minor;
-       unsigned long flags;
-
-       slot = drm_minor_get_slot(dev, type);
-       minor = *slot;
-       if (!minor)
-               return;
-
-       put_device(minor->kdev);
-
-       spin_lock_irqsave(&drm_minor_lock, flags);
-       idr_remove(&drm_minors_idr, minor->index);
-       spin_unlock_irqrestore(&drm_minor_lock, flags);
-
-       kfree(minor);
-       *slot = NULL;
 }
 
 static int drm_minor_register(struct drm_device *dev, unsigned int type)
@@ -678,16 +664,16 @@ int drm_dev_init(struct drm_device *dev,
        if (drm_core_check_feature(dev, DRIVER_RENDER)) {
                ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
                if (ret)
-                       goto err_minors;
+                       goto err;
        }
 
        ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
        if (ret)
-               goto err_minors;
+               goto err;
 
        ret = drm_legacy_create_map_hash(dev);
        if (ret)
-               goto err_minors;
+               goto err;
 
        drm_legacy_ctxbitmap_init(dev);
 
@@ -695,7 +681,7 @@ int drm_dev_init(struct drm_device *dev,
                ret = drm_gem_init(dev);
                if (ret) {
                        DRM_ERROR("Cannot initialize graphics execution manager (GEM)\n");
-                       goto err_ctxbitmap;
+                       goto err;
                }
        }
 
@@ -708,10 +694,6 @@ int drm_dev_init(struct drm_device *dev,
 err_setunique:
        if (drm_core_check_feature(dev, DRIVER_GEM))
                drm_gem_destroy(dev);
-err_ctxbitmap:
-err_minors:
-       drm_minor_free(dev, DRM_MINOR_PRIMARY);
-       drm_minor_free(dev, DRM_MINOR_RENDER);
 err:
        drm_managed_release(dev);
 
@@ -776,9 +758,6 @@ void drm_dev_fini(struct drm_device *dev)
 
        if (drm_core_check_feature(dev, DRIVER_GEM))
                drm_gem_destroy(dev);
-
-       drm_minor_free(dev, DRM_MINOR_PRIMARY);
-       drm_minor_free(dev, DRM_MINOR_RENDER);
 }
 EXPORT_SYMBOL(drm_dev_fini);
 

diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 6bce1c892df3d18e92fb2c72cc51eea18726f685..7246a8318137fccd2d0077729142294662b3a01e 100644 (file)
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -149,6 +149,20 @@ int __drmm_add_action(struct drm_device *dev,
 }
 EXPORT_SYMBOL(__drmm_add_action);
 
+int __drmm_add_action_or_reset(struct drm_device *dev,
+                              drmres_release_t action,
+                              void *data, const char *name)
+{
+       int ret;
+
+       ret = __drmm_add_action(dev, action, data, name);
+       if (ret)
+               action(dev, data);
+
+       return ret;
+}
+EXPORT_SYMBOL(__drmm_add_action_or_reset);
+
 void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp)
 {
        struct drmres *dr;

diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index 89e6fce9f689466edc743ee4b42083b26033f75f..2d1e29a2200caeaae76ecca99cb6c00c093a4ddd 100644 (file)
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -17,7 +17,14 @@ int __must_check __drmm_add_action(struct drm_device *dev,
                                   drmres_release_t action,
                                   void *data, const char *name);
 
-void drmm_add_final_kfree(struct drm_device *dev, void *parent);
+#define drmm_add_action_or_reset(dev, action, data) \
+       __drmm_add_action_or_reset(dev, action, data, #action)
+
+int __must_check __drmm_add_action_or_reset(struct drm_device *dev,
+                                           drmres_release_t action,
+                                           void *data, const char *name);
+
+void drmm_add_final_kfree(struct drm_device *dev, void *container);
 
 void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
 static inline void *drmm_kzalloc(struct drm_device *dev, size_t size, gfp_t gfp)