Re: [PATCH v2 5/5] drm/i915: Add cpu fault handler for mmap_offset

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Abdiel Janulgue (2019-10-07 10:19:20)
> +static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
> +{
> +       struct vm_area_struct *area = vmf->vma;
> +       struct i915_mmap_offset *priv = area->vm_private_data;
> +       struct drm_i915_gem_object *obj = priv->obj;
> +       vm_fault_t vmf_ret;
> +       unsigned long size = area->vm_end - area->vm_start;
> +       bool write = area->vm_flags & VM_WRITE;
> +       int i, ret;
> +
> +       /* Sanity check that we allow writing into this object */
> +       if (i915_gem_object_is_readonly(obj) && write)
> +               return VM_FAULT_SIGBUS;
> +
> +       ret = i915_gem_object_pin_pages(obj);
> +       if (ret)
> +               return i915_error_to_vmf_fault(ret);
> +
> +       for (i = 0; i < size >> PAGE_SHIFT; i++) {
> +               struct page *page = i915_gem_object_get_page(obj, i);
> +
> +               vmf_ret = vmf_insert_pfn(area,
> +                                        (unsigned long)area->vm_start + i * PAGE_SIZE,
> +                                        page_to_pfn(page));
> +               if (vmf_ret & VM_FAULT_ERROR)
> +                       break;

So why are we using vmf_insert_pfn + VM_PFNMAP for this? It does seem to
be page backed. And since you are prefaulting, you may legitimately try
to double insert the same page and hit an error. You should only bail if
you fail to insert the fault address.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx