[Intel-gfx] [PATCH] drm/i915: use drm vma accounting functions to make sure VMs don't get lost

[Eric Anholt]

On Mon, 2008-12-29 at 09:08 +0000, Dave Airlie wrote:
> From: Dave Airlie <airlied at linux.ie>
> 
> The gem gtt mapping code didn't use the drm_vm.c accounting open/close,
> it did call the initial drm_vm_open_locked, so it should always do this.
> 
> I'm not 100% sure it doesn't need a special open/close pair, but this at 
> least makes /proc/dri/0/vma not end up with lots of crap in it. I'm still 
> not getting kms + EXA on i915 working at all.

This introduces a lock order reversal between execbuf grabbing
struct_mutex and then copy_to/from_user grabbing the mm lock, and
drm_vm_open grabbing struct_mutex while mm lock is already held.

I guess we need to pull the copy_to/from_user out from execbuf's locked
section?  I don't think that's going to be pretty.  Do we want
finer-grained locks?

> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  drivers/gpu/drm/drm_vm.c        |    9 ++++-----
>  drivers/gpu/drm/i915/i915_drv.c |    2 ++
>  include/drm/drmP.h              |    2 ++
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index 3ffae02..132b03d 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -38,9 +38,6 @@
>  #include <linux/efi.h>
>  #endif
>  
> -static void drm_vm_open(struct vm_area_struct *vma);
> -static void drm_vm_close(struct vm_area_struct *vma);
> -
>  static pgprot_t drm_io_prot(uint32_t map_type, struct vm_area_struct *vma)
>  {
>  	pgprot_t tmp = vm_get_page_prot(vma->vm_flags);
> @@ -420,7 +417,7 @@ void drm_vm_open_locked(struct vm_area_struct *vma)
>  	}
>  }
>  
> -static void drm_vm_open(struct vm_area_struct *vma)
> +void drm_vm_open(struct vm_area_struct *vma)
>  {
>  	struct drm_file *priv = vma->vm_file->private_data;
>  	struct drm_device *dev = priv->minor->dev;
> @@ -429,6 +426,7 @@ static void drm_vm_open(struct vm_area_struct *vma)
>  	drm_vm_open_locked(vma);
>  	mutex_unlock(&dev->struct_mutex);
>  }
> +EXPORT_SYMBOL(drm_vm_open);
>  
>  /**
>   * \c close method for all virtual memory types.
> @@ -438,7 +436,7 @@ static void drm_vm_open(struct vm_area_struct *vma)
>   * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
>   * free it.
>   */
> -static void drm_vm_close(struct vm_area_struct *vma)
> +void drm_vm_close(struct vm_area_struct *vma)
>  {
>  	struct drm_file *priv = vma->vm_file->private_data;
>  	struct drm_device *dev = priv->minor->dev;
> @@ -458,6 +456,7 @@ static void drm_vm_close(struct vm_area_struct *vma)
>  	}
>  	mutex_unlock(&dev->struct_mutex);
>  }
> +EXPORT_SYMBOL(drm_vm_close);
>  
>  /**
>   * mmap DMA memory.
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f8b3df0..61d8ed6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -94,6 +94,8 @@ static int i915_resume(struct drm_device *dev)
>  
>  static struct vm_operations_struct i915_gem_vm_ops = {
>  	.fault = i915_gem_fault,
> +	.open = drm_vm_open,
> +	.close = drm_vm_close,
>  };
>  
>  static struct drm_driver driver = {
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index afb7858..9a736e3 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1058,6 +1058,8 @@ extern int drm_release(struct inode *inode, struct file *filp);
>  extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
>  extern int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma);
>  extern void drm_vm_open_locked(struct vm_area_struct *vma);
> +extern void drm_vm_open(struct vm_area_struct *vma);
> +extern void drm_vm_close(struct vm_area_struct *vma);
>  extern unsigned long drm_core_get_map_ofs(struct drm_map * map);
>  extern unsigned long drm_core_get_reg_ofs(struct drm_device *dev);
>  extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090102/d14ffa6e/attachment.sig>