On 15/12/2017 09:27, Chris Wilson wrote:
We want to exclude any GGTT objects from being present on our internal
lists to avoid the deadlock we may run into with our requirement for
struct_mutex during invalidate. However, if the gup_fast fails, we put
the userptr onto the workqueue and mark it as active, so that we
remember to serialise the worker upon mmu_invalidate.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104209
Signed-off-by: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Michał Winiarski <[email protected]>
---
  drivers/gpu/drm/i915/i915_gem_userptr.c | 40 +++++++++++++++++++++++++++++++--
  1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 382a77a1097e..562b869dc750 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -598,6 +598,39 @@ __i915_gem_userptr_get_pages_schedule(struct 
drm_i915_gem_object *obj)
        return ERR_PTR(-EAGAIN);
  }
+static int
+probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
+{
+       const unsigned long end = addr + len;
+       struct vm_area_struct *vma;
+       int ret = -EFAULT;
+
+       down_read(&mm->mmap_sem);
+       for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
+               if (vma->vm_start > addr)
+                       break;
+
+               /*
+                * Exclude any VMA that is backed only by struct_page, i.e.
+                * IO regions that include our own GGTT mmaps. We cannot handle
+                * such ranges, as we may encounter deadlocks around our
+                * struct_mutex on mmu_invalidate_range.
+                */
+               if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
+                       break;
+
+               if (vma->vm_end >= end) {
+                       ret = 0;
+                       break;
+               }
+
+               addr = vma->vm_end;
+       }
+       up_read(&mm->mmap_sem);
+
+       return ret;
+}
+
  static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
  {
        const int num_pages = obj->base.size >> PAGE_SHIFT;
@@ -632,9 +665,12 @@ static int i915_gem_userptr_get_pages(struct 
drm_i915_gem_object *obj)
                        return -EAGAIN;
        }
- pvec = NULL;
-       pinned = 0;
+       /* Quickly exclude any invalid VMA */
+       pinned = probe_range(mm, obj->userptr.ptr, obj->base.size);
+       if (pinned)
+               return pinned;
+ pvec = NULL;
        if (mm == current->mm) {
                pvec = kvmalloc_array(num_pages, sizeof(struct page *),
                                      GFP_KERNEL |


Okay as a band-aid, but open to exploitation, which I think was my issue last time you posted something similar? Anyways.. it's not worse so lesson learnt, of some sort.

Reviewed-by: Tvrtko Ursulin <[email protected]>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to