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

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

On 15/01/2019 10:30, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-15 10:19:11)
>>
>> On 14/01/2019 21:17, Chris Wilson wrote:
>>> @@ -620,40 +682,24 @@ static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
>>>                        return -EAGAIN;
>>>        }
>>>    
>>> -     pvec = NULL;
>>> -     pinned = 0;
>>> +     if (mm == current->mm && try_fast_gup(obj) == 0)
>>> +             return 0;
>>
>> Is try_fast_gup guaranteed to fail on ranges probe_range would have
>> rejected?
> 
> If a page isn't present, it will fail. For our purposes that means
> fast_gup fails for GGTT mmaps, as well as other ranges.

What if it has been pre-faulted?

>>> -     if (mm == current->mm) {
>>> -             pvec = kvmalloc_array(num_pages, sizeof(struct page *),
>>> -                                   GFP_KERNEL |
>>> -                                   __GFP_NORETRY |
>>> -                                   __GFP_NOWARN);
>>> -             if (pvec) /* defer to worker if malloc fails */
>>> -                     pinned = __get_user_pages_fast(obj->userptr.ptr,
>>> -                                                    num_pages,
>>> -                                                    !i915_gem_object_is_readonly(obj),
>>> -                                                    pvec);
>>> -     }
>>> +     /* lockdep doesn't yet automatically allow nesting of readers */
>>> +     down_read_nested(&mm->mmap_sem, SINGLE_DEPTH_NESTING);
>>
>> Why is nesting needed - comment needs to explain.
> 
> We get called with mm->mmap_sem held for reading on a fault path, and
> without mm->mmap_sem held otherwise. lockdep doesn't allow nesting of
> readers, hence the comment.

Something like that in the comment yes please. :)

Regards,

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