Re: [PATCH 3/3] drm/i915/userptr: Probe vma range before gup

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 15/01/2019 10:41, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-15 10:27:02)
>>
>> On 14/01/2019 21:17, Chris Wilson wrote:
>>> +     if (err)
>>> +             goto err_unlock;
>>> +
>>> +     err = __i915_gem_userptr_get_pages_schedule(obj);
>>> +     if (err == -EAGAIN)
>>>                __i915_gem_userptr_set_active(obj, true);
>>>    
>>> -     if (IS_ERR(pages))
>>> -             release_pages(pvec, pinned);
>>> -     kvfree(pvec);
>>> +err_unlock:
>>> +     up_read(&mm->mmap_sem);
>>
>> May be safer to drop the lock earlier, immediately after probe. I don't
>> see holding it while queuing the worker and doing internal book-keeping
>> is useful and might just create more lock chain dependencies.
> 
> Hmm, I thought we need to cover up to set-active (probe + queue + insert
> into rbtree) as I thought the mmu-invalidate was under the mmap_sem wlock.

We have our own lock for set active so I don't see that we need mmap_sem 
for it. Certainly wasn't needed before this patch so don't see that 
would change now.

Btw, what you said regarding nested mmap_sem.. do we have lock inversion 
with it and obj->mm.lock then? I mean both mmap_sem -> obj->mm.lock and 
obj->mm.lock -> mmap_sem chains?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx