On Sat, Jan 3, 2009 at 11:44 AM, Eric Anholt <e...@anholt.net> wrote:
> On Mon, 2008-12-29 at 09:08 +0000, Dave Airlie wrote:
>> From: Dave Airlie <airl...@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 <airl...@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
> e...@anholt.net eric.anh...@intel.com
>
>
>
------------------------------------------------------------------------------
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
