[Intel-gfx] [PATCH v3] drm: Provide a driver hook for drm_dev_release()

[Noralf Trønnes]

Den 21.01.2017 11:58, skrev Chris Wilson:
> Some state is coupled into the device lifetime outside of the
> load/unload timeframe and requires teardown during final unreference
> from drm_dev_release(). For example, dmabufs hold both a device and
> module reference and may live longer than expected (i.e. the current
> pattern of the driver tearing down its state and then releasing a
> reference to the drm device) and yet touch driver private state when
> destroyed.
>
> v2: Export drm_dev_fini() and move the responsible for finalizing the
> drm_device and freeing it to the release callback. (If no callback is
> provided, the core will call drm_dev_fini() and kfree(dev) as before.)
> v3: Remember to add drm_dev_fini() to drm_drv.h
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>   drivers/gpu/drm/drm_drv.c | 56 +++++++++++++++++++++++++++++++++--------------
>   include/drm/drm_drv.h     | 11 ++++++++++
>   2 files changed, 50 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 1b11ab628da7..517718e4f6e4 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -553,6 +553,39 @@ int drm_dev_init(struct drm_device *dev,
>   EXPORT_SYMBOL(drm_dev_init);
>   
>   /**
> + * drm_dev_fini - Finalize a dead DRM device
> + * @dev: DRM device
> + *
> + * Finalize a dead DRM device. This is the converse to drm_dev_init() and
> + * frees up all state allocated by it. All driver state should be finalized
> + * first. Note that this function does not free the @dev, that is left to the
> + * caller. drm_dev_fini() should only

Looks like the end of the sentence is missing.

Noralf.

> + *
> + * The ref-count of @dev must be zero, and drm_dev_fini() should only be called
> + * from a drm_driver->release() callback.
> + */
> +void drm_dev_fini(struct drm_device *dev)
> +{
> +	if (drm_core_check_feature(dev, DRIVER_GEM))
> +		drm_gem_destroy(dev);
> +
> +	drm_legacy_ctxbitmap_cleanup(dev);
> +	drm_ht_remove(&dev->map_hash);
> +	drm_fs_inode_free(dev->anon_inode);
> +
> +	drm_minor_free(dev, DRM_MINOR_PRIMARY);
> +	drm_minor_free(dev, DRM_MINOR_RENDER);
> +	drm_minor_free(dev, DRM_MINOR_CONTROL);
> +
> +	mutex_destroy(&dev->master_mutex);
> +	mutex_destroy(&dev->ctxlist_mutex);
> +	mutex_destroy(&dev->filelist_mutex);
> +	mutex_destroy(&dev->struct_mutex);
> +	kfree(dev->unique);
> +}
> +EXPORT_SYMBOL(drm_dev_fini);
> +
> +/**
>    * drm_dev_alloc - Allocate new DRM device
>    * @driver: DRM driver to allocate device for
>    * @parent: Parent device object
> @@ -598,23 +631,12 @@ static void drm_dev_release(struct kref *ref)
>   {
>   	struct drm_device *dev = container_of(ref, struct drm_device, ref);
>   
> -	if (drm_core_check_feature(dev, DRIVER_GEM))
> -		drm_gem_destroy(dev);
> -
> -	drm_legacy_ctxbitmap_cleanup(dev);
> -	drm_ht_remove(&dev->map_hash);
> -	drm_fs_inode_free(dev->anon_inode);
> -
> -	drm_minor_free(dev, DRM_MINOR_PRIMARY);
> -	drm_minor_free(dev, DRM_MINOR_RENDER);
> -	drm_minor_free(dev, DRM_MINOR_CONTROL);
> -
> -	mutex_destroy(&dev->master_mutex);
> -	mutex_destroy(&dev->ctxlist_mutex);
> -	mutex_destroy(&dev->filelist_mutex);
> -	mutex_destroy(&dev->struct_mutex);
> -	kfree(dev->unique);
> -	kfree(dev);
> +	if (dev->driver->release) {
> +		dev->driver->release(dev);
> +	} else {
> +		drm_dev_fini(dev);
> +		kfree(dev);
> +	}
>   }
>   
>   /**
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 34ece393c639..bc5865ad0bbd 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -103,6 +103,15 @@ struct drm_driver {
>   	 *
>   	 */
>   	void (*unload) (struct drm_device *);
> +
> +	/**
> +	 * @release:
> +	 *
> +	 * Optional callback for destroying device state after the final
> +	 * reference is released, i.e. the device is being destroyed.
> +	 */
> +	void (*release) (struct drm_device *);
> +
>   	int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv);
>   	int (*dma_quiescent) (struct drm_device *);
>   	int (*context_dtor) (struct drm_device *dev, int context);
> @@ -451,6 +460,8 @@ extern unsigned int drm_debug;
>   int drm_dev_init(struct drm_device *dev,
>   		 struct drm_driver *driver,
>   		 struct device *parent);
> +void drm_dev_fini(struct drm_device *dev);
> +
>   struct drm_device *drm_dev_alloc(struct drm_driver *driver,
>   				 struct device *parent);
>   int drm_dev_register(struct drm_device *dev, unsigned long flags);