diff mbox series

[22/51] drm: manage drm_minor cleanup with drmm_

Message ID 20200302222631.3861340-23-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series drm_device managed resources, v4 | expand

Commit Message

Daniel Vetter March 2, 2020, 10:26 p.m. UTC 
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.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c     | 74 +++++++++++++----------------------
 drivers/gpu/drm/drm_managed.c | 28 +++++++++++++
 include/drm/drm_managed.h     |  4 ++
 3 files changed, 59 insertions(+), 47 deletions(-)

Comments

Thomas Zimmermann March 11, 2020, 9:59 a.m. UTC | #1 
Hi

Am 02.03.20 um 23:26 schrieb Daniel Vetter:
> 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.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c     | 74 +++++++++++++----------------------
>  drivers/gpu/drm/drm_managed.c | 28 +++++++++++++
>  include/drm/drm_managed.h     |  4 ++
>  3 files changed, 59 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 23e5b0e7e041..29d106195ab3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -93,19 +93,35 @@ 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;
>  
>  	minor->type = type;
>  	minor->dev = dev;
>  
> +	r = drmm_add_action(dev, drm_minor_alloc_release, minor);
> +	if (r)
> +		return r;
> +
>  	idr_preload(GFP_KERNEL);
>  	spin_lock_irqsave(&drm_minor_lock, flags);
>  	r = idr_alloc(&drm_minors_idr,
> @@ -116,47 +132,18 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
>  	spin_unlock_irqrestore(&drm_minor_lock, flags);
>  	idr_preload_end();
>  
> -	if (r < 0)
> -		goto err_free;
> +	if (r < 0) {
> +		drmm_remove_action(dev, drm_minor_alloc_release, minor);
> +		return r;
> +	}
>  
>  	minor->index = 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;
> -}

TBH, I think you're reducing code quality by removing the rollback code
from init functions, just for the sake of it.

Specifically in this case here, you saved a few lines of code, but the
overall flow is way more complicated to follow. That's typically a
reliably source of bugs. This call to drmm_remove_action() just makes it
worse.

Rather, see my remark on OOP destruction in patch 21. For now, I'd focus
on the device cleanup and leave init functions as they are.

Best regards
Thomas

> -
> -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 +665,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 +682,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 +695,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 +759,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 514d5bd42446..0883615c2088 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -142,6 +142,34 @@ int __drmm_add_action(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(__drmm_add_action);
>  
> +void drmm_remove_action(struct drm_device *dev,
> +			drmres_release_t action,
> +			void *data)
> +{
> +	struct drmres *dr = NULL, *tmp;
> +	unsigned long flags;
> +
> +	if (!data)
> +		return;
> +
> +	spin_lock_irqsave(&dev->managed.lock, flags);
> +	list_for_each_entry(tmp, &dev->managed.resources, node.entry) {
> +		if (tmp->node.release == action &&
> +		    *(void **)&tmp->data == data) {
> +			dr = tmp;
> +			del_dr(dev, dr);
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&dev->managed.lock, flags);
> +
> +	if (WARN_ON(!dr))
> +		return;
> +
> +	kfree(dr);
> +}
> +EXPORT_SYMBOL(drmm_remove_action);
> +
>  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 89e6fce9f689..5280209dff92 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -17,6 +17,10 @@ int __must_check __drmm_add_action(struct drm_device *dev,
>  				   drmres_release_t action,
>  				   void *data, const char *name);
>  
> +void drmm_remove_action(struct drm_device *dev,
> +			drmres_release_t action,
> +			void *data);
> +
>  void drmm_add_final_kfree(struct drm_device *dev, void *parent);
>  
>  void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
>
Daniel Vetter March 16, 2020, 9:07 a.m. UTC | #2 
On Wed, Mar 11, 2020 at 10:59:10AM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 02.03.20 um 23:26 schrieb Daniel Vetter:
> > 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.
> > 
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_drv.c     | 74 +++++++++++++----------------------
> >  drivers/gpu/drm/drm_managed.c | 28 +++++++++++++
> >  include/drm/drm_managed.h     |  4 ++
> >  3 files changed, 59 insertions(+), 47 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 23e5b0e7e041..29d106195ab3 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -93,19 +93,35 @@ 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;
> >  
> >  	minor->type = type;
> >  	minor->dev = dev;
> >  
> > +	r = drmm_add_action(dev, drm_minor_alloc_release, minor);
> > +	if (r)
> > +		return r;
> > +
> >  	idr_preload(GFP_KERNEL);
> >  	spin_lock_irqsave(&drm_minor_lock, flags);
> >  	r = idr_alloc(&drm_minors_idr,
> > @@ -116,47 +132,18 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> >  	spin_unlock_irqrestore(&drm_minor_lock, flags);
> >  	idr_preload_end();
> >  
> > -	if (r < 0)
> > -		goto err_free;
> > +	if (r < 0) {
> > +		drmm_remove_action(dev, drm_minor_alloc_release, minor);
> > +		return r;
> > +	}
> >  
> >  	minor->index = 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;
> > -}
> 
> TBH, I think you're reducing code quality by removing the rollback code
> from init functions, just for the sake of it.
> 
> Specifically in this case here, you saved a few lines of code, but the
> overall flow is way more complicated to follow. That's typically a
> reliably source of bugs. This call to drmm_remove_action() just makes it
> worse.
> 
> Rather, see my remark on OOP destruction in patch 21. For now, I'd focus
> on the device cleanup and leave init functions as they are.

Ah, I can simplify this with add_action_or_reset. This removes the only
user of drmm_remove_action, which I think is actually a good thing :-)
-Daniel

> 
> Best regards
> Thomas
> 
> > -
> > -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 +665,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 +682,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 +695,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 +759,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 514d5bd42446..0883615c2088 100644
> > --- a/drivers/gpu/drm/drm_managed.c
> > +++ b/drivers/gpu/drm/drm_managed.c
> > @@ -142,6 +142,34 @@ int __drmm_add_action(struct drm_device *dev,
> >  }
> >  EXPORT_SYMBOL(__drmm_add_action);
> >  
> > +void drmm_remove_action(struct drm_device *dev,
> > +			drmres_release_t action,
> > +			void *data)
> > +{
> > +	struct drmres *dr = NULL, *tmp;
> > +	unsigned long flags;
> > +
> > +	if (!data)
> > +		return;
> > +
> > +	spin_lock_irqsave(&dev->managed.lock, flags);
> > +	list_for_each_entry(tmp, &dev->managed.resources, node.entry) {
> > +		if (tmp->node.release == action &&
> > +		    *(void **)&tmp->data == data) {
> > +			dr = tmp;
> > +			del_dr(dev, dr);
> > +			break;
> > +		}
> > +	}
> > +	spin_unlock_irqrestore(&dev->managed.lock, flags);
> > +
> > +	if (WARN_ON(!dr))
> > +		return;
> > +
> > +	kfree(dr);
> > +}
> > +EXPORT_SYMBOL(drmm_remove_action);
> > +
> >  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 89e6fce9f689..5280209dff92 100644
> > --- a/include/drm/drm_managed.h
> > +++ b/include/drm/drm_managed.h
> > @@ -17,6 +17,10 @@ int __must_check __drmm_add_action(struct drm_device *dev,
> >  				   drmres_release_t action,
> >  				   void *data, const char *name);
> >  
> > +void drmm_remove_action(struct drm_device *dev,
> > +			drmres_release_t action,
> > +			void *data);
> > +
> >  void drmm_add_final_kfree(struct drm_device *dev, void *parent);
> >  
> >  void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 23e5b0e7e041..29d106195ab3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -93,19 +93,35 @@  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;
 
 	minor->type = type;
 	minor->dev = dev;
 
+	r = drmm_add_action(dev, drm_minor_alloc_release, minor);
+	if (r)
+		return r;
+
 	idr_preload(GFP_KERNEL);
 	spin_lock_irqsave(&drm_minor_lock, flags);
 	r = idr_alloc(&drm_minors_idr,
@@ -116,47 +132,18 @@  static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
 	spin_unlock_irqrestore(&drm_minor_lock, flags);
 	idr_preload_end();
 
-	if (r < 0)
-		goto err_free;
+	if (r < 0) {
+		drmm_remove_action(dev, drm_minor_alloc_release, minor);
+		return r;
+	}
 
 	minor->index = 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 +665,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 +682,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 +695,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 +759,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 514d5bd42446..0883615c2088 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -142,6 +142,34 @@  int __drmm_add_action(struct drm_device *dev,
 }
 EXPORT_SYMBOL(__drmm_add_action);
 
+void drmm_remove_action(struct drm_device *dev,
+			drmres_release_t action,
+			void *data)
+{
+	struct drmres *dr = NULL, *tmp;
+	unsigned long flags;
+
+	if (!data)
+		return;
+
+	spin_lock_irqsave(&dev->managed.lock, flags);
+	list_for_each_entry(tmp, &dev->managed.resources, node.entry) {
+		if (tmp->node.release == action &&
+		    *(void **)&tmp->data == data) {
+			dr = tmp;
+			del_dr(dev, dr);
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&dev->managed.lock, flags);
+
+	if (WARN_ON(!dr))
+		return;
+
+	kfree(dr);
+}
+EXPORT_SYMBOL(drmm_remove_action);
+
 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 89e6fce9f689..5280209dff92 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -17,6 +17,10 @@  int __must_check __drmm_add_action(struct drm_device *dev,
 				   drmres_release_t action,
 				   void *data, const char *name);
 
+void drmm_remove_action(struct drm_device *dev,
+			drmres_release_t action,
+			void *data);
+
 void drmm_add_final_kfree(struct drm_device *dev, void *parent);
 
 void *drmm_kmalloc(struct drm_device *dev, size_t size, gfp_t gfp) __malloc;