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

[Dave Airlie]

On Sat, Jan 3, 2009 at 11:44 AM, Eric Anholt <eric at anholt.net> wrote:
> 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?

I'm going to say yes :-), that's a real pain in the ass, we do need to
fix the accounting one
way or the other. Not doing accounting at all sounds like an equally bad plan.

Dave.

>
>> 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
>
>
>