Re: [PATCH 04/12] drm: extract legacy ctxbitmap flushing

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 23, 2014 at 05:26:39PM +0200, David Herrmann wrote:
> The ctxbitmap code is only used by legacy drivers so lets try to keep it
> as separated as possible. Furthermore, the locking is non-obvious and
> kinda weird with ctxlist_mutex *and* struct_mutex. Keeping all ctxbitmap
> access in one file is much easier to review and makes drm_release() more
> readable.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

I've started to sprinkle _legacy_ over all the functions only used for
non-kms drivers, so that the dragon dungeons are clearly marked off. With
that this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>.
-Daniel

> ---
>  drivers/gpu/drm/drm_context.c | 30 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_fops.c    | 20 +-------------------
>  include/drm/drmP.h            |  1 +
>  3 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
> index a4b017b..c045505 100644
> --- a/drivers/gpu/drm/drm_context.c
> +++ b/drivers/gpu/drm/drm_context.c
> @@ -111,6 +111,36 @@ void drm_ctxbitmap_cleanup(struct drm_device * dev)
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> +/**
> + * drm_ctxbitmap_flush() - Flush all contexts owned by a file
> + * @dev: DRM device to operate on
> + * @file: Open file to flush contexts for
> + *
> + * This iterates over all contexts on @dev and drops them if they're owned by
> + * @file. Note that after this call returns, new contexts might be added if
> + * the file is still alive.
> + */
> +void drm_ctxbitmap_flush(struct drm_device *dev, struct drm_file *file)
> +{
> +	struct drm_ctx_list *pos, *tmp;
> +
> +	mutex_lock(&dev->ctxlist_mutex);
> +
> +	list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) {
> +		if (pos->tag == file &&
> +		    pos->handle != DRM_KERNEL_CONTEXT) {
> +			if (dev->driver->context_dtor)
> +				dev->driver->context_dtor(dev, pos->handle);
> +
> +			drm_ctxbitmap_free(dev, pos->handle);
> +			list_del(&pos->head);
> +			kfree(pos);
> +		}
> +	}
> +
> +	mutex_unlock(&dev->ctxlist_mutex);
> +}
> +
>  /*@}*/
>  
>  /******************************************************************/
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 8e73519..fb81d1c 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -452,25 +452,7 @@ int drm_release(struct inode *inode, struct file *filp)
>  	if (dev->driver->driver_features & DRIVER_GEM)
>  		drm_gem_release(dev, file_priv);
>  
> -	mutex_lock(&dev->ctxlist_mutex);
> -	if (!list_empty(&dev->ctxlist)) {
> -		struct drm_ctx_list *pos, *n;
> -
> -		list_for_each_entry_safe(pos, n, &dev->ctxlist, head) {
> -			if (pos->tag == file_priv &&
> -			    pos->handle != DRM_KERNEL_CONTEXT) {
> -				if (dev->driver->context_dtor)
> -					dev->driver->context_dtor(dev,
> -								  pos->handle);
> -
> -				drm_ctxbitmap_free(dev, pos->handle);
> -
> -				list_del(&pos->head);
> -				kfree(pos);
> -			}
> -		}
> -	}
> -	mutex_unlock(&dev->ctxlist_mutex);
> +	drm_ctxbitmap_flush(dev, file_priv);
>  
>  	mutex_lock(&dev->master_mutex);
>  
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d1730c5..d91e09f 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1242,6 +1242,7 @@ extern int drm_rmctx(struct drm_device *dev, void *data,
>  extern int drm_ctxbitmap_init(struct drm_device *dev);
>  extern void drm_ctxbitmap_cleanup(struct drm_device *dev);
>  extern void drm_ctxbitmap_free(struct drm_device *dev, int ctx_handle);
> +extern void drm_ctxbitmap_flush(struct drm_device *dev, struct drm_file *file);
>  
>  extern int drm_setsareactx(struct drm_device *dev, void *data,
>  			   struct drm_file *file_priv);
> -- 
> 2.0.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch