[PATCH] drm/i915: Split out GTT fault handler to make it generic

* [PATCH] drm/i915: Split out GTT fault handler to make it generic
@ 2019-06-04 10:37 Abdiel Janulgue
  2019-06-04 11:00 ` Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Abdiel Janulgue @ 2019-06-04 10:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Allow reuse of the fault-handling code in preparation for having
multiple fault handlers depending on the mmaping type and backing
storage.

Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 208 ++++++++++++++---------
 1 file changed, 132 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c7b9b34de01b..ed20fab66d2f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -197,6 +197,133 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
 	return view;
 }
 
+struct gem_object_fault {
+	struct drm_i915_gem_object *obj;
+	pgoff_t page_offset;
+	intel_wakeref_t wakeref;
+	int srcu;
+	bool pin_vma;
+	vm_fault_t error_ret;
+};
+
+static vm_fault_t __vm_fault_from_error(struct drm_i915_private *i915,
+					int fault_ret)
+{
+	switch (fault_ret) {
+	case -EIO:
+		/*
+		 * We eat errors when the gpu is terminally wedged to avoid
+		 * userspace unduly crashing (gl has no provisions for mmaps to
+		 * fail). But any other -EIO isn't ours (e.g. swap in failure)
+		 * and so needs to be reported.
+		 */
+		if (!i915_terminally_wedged(i915))
+			return VM_FAULT_SIGBUS;
+		/* else: fall through */
+	case -EAGAIN:
+		/*
+		 * EAGAIN means the gpu is hung and we'll wait for the error
+		 * handler to reset everything when re-faulting in
+		 * i915_mutex_lock_interruptible.
+		 */
+	case 0:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		/*
+		 * EBUSY is ok: this just means that another thread
+		 * already did the job.
+		 */
+		return VM_FAULT_NOPAGE;
+	case -ENOMEM:
+		return VM_FAULT_OOM;
+	case -ENOSPC:
+	case -EFAULT:
+		return VM_FAULT_SIGBUS;
+	default:
+		WARN_ONCE(fault_ret, "unhandled error in %s: %i\n", __func__,
+			  fault_ret);
+		return VM_FAULT_SIGBUS;
+	}
+}
+
+static int __prepare_object_fault(struct vm_fault *vmf,
+				  bool pin_vma,
+				  struct gem_object_fault *f)
+{
+	struct vm_area_struct *area = vmf->vma;
+	struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	bool write = area->vm_flags & VM_WRITE;
+	int ret;
+
+	/* Sanity check that we allow writing into this object */
+	if (i915_gem_object_is_readonly(obj) && write) {
+		f->error_ret = VM_FAULT_SIGBUS;
+		return -EACCES;
+	}
+
+	f->obj = obj;
+	/* We don't use vmf->pgoff since that has the fake offset */
+	f->page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
+
+	trace_i915_gem_object_fault(obj, f->page_offset, true, write);
+
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		goto err;
+
+	f->wakeref = intel_runtime_pm_get(dev_priv);
+
+	f->srcu = i915_reset_trylock(dev_priv);
+	if (f->srcu < 0) {
+		ret = f->srcu;
+		goto err_rpm;
+	}
+
+	f->pin_vma = pin_vma;
+	if (f->pin_vma) {
+		ret = i915_mutex_lock_interruptible(dev);
+		if (ret)
+			goto err_reset;
+	}
+
+	/* Access to snoopable pages through the GTT is incoherent. */
+	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
+		ret = -EFAULT;
+		goto err_unlock;
+	}
+
+	return 0;
+
+err_unlock:
+	if (f->pin_vma)
+		mutex_unlock(&dev->struct_mutex);
+err_reset:
+	i915_reset_unlock(dev_priv, f->srcu);
+err_rpm:
+	intel_runtime_pm_put(dev_priv, f->wakeref);
+	i915_gem_object_unpin_pages(obj);
+err:
+	f->error_ret = __vm_fault_from_error(dev_priv, ret);
+	return ret;
+}
+
+static vm_fault_t __object_fault_fini(int fault_ret, struct gem_object_fault *f)
+{
+	struct drm_device *dev = f->obj->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (f->pin_vma)
+		mutex_unlock(&dev->struct_mutex);
+	i915_reset_unlock(dev_priv, f->srcu);
+	intel_runtime_pm_put(dev_priv, f->wakeref);
+	i915_gem_object_unpin_pages(f->obj);
+
+	return __vm_fault_from_error(dev_priv, fault_ret);
+}
+
 /**
  * i915_gem_fault - fault a page into the GTT
  * @vmf: fault info
@@ -223,43 +350,13 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *i915 = to_i915(dev);
 	struct i915_ggtt *ggtt = &i915->ggtt;
-	bool write = area->vm_flags & VM_WRITE;
-	intel_wakeref_t wakeref;
 	struct i915_vma *vma;
 	pgoff_t page_offset;
-	int srcu;
+	struct gem_object_fault fault;
 	int ret;
 
-	/* Sanity check that we allow writing into this object */
-	if (i915_gem_object_is_readonly(obj) && write)
-		return VM_FAULT_SIGBUS;
-
-	/* We don't use vmf->pgoff since that has the fake offset */
-	page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
-
-	trace_i915_gem_object_fault(obj, page_offset, true, write);
-
-	ret = i915_gem_object_pin_pages(obj);
-	if (ret)
-		goto err;
-
-	wakeref = intel_runtime_pm_get(i915);
-
-	srcu = i915_reset_trylock(i915);
-	if (srcu < 0) {
-		ret = srcu;
-		goto err_rpm;
-	}
-
-	ret = i915_mutex_lock_interruptible(dev);
-	if (ret)
-		goto err_reset;
-
-	/* Access to snoopable pages through the GTT is incoherent. */
-	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(i915)) {
-		ret = -EFAULT;
-		goto err_unlock;
-	}
+	if (__prepare_object_fault(vmf, true, &fault))
+		return fault.error_ret;
 
 	/* Now pin it into the GTT as needed */
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
@@ -291,7 +388,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	}
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
-		goto err_unlock;
+		goto err;
 	}
 
 	ret = i915_vma_pin_fence(vma);
@@ -322,49 +419,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	i915_vma_unpin_fence(vma);
 err_unpin:
 	__i915_vma_unpin(vma);
-err_unlock:
-	mutex_unlock(&dev->struct_mutex);
-err_reset:
-	i915_reset_unlock(i915, srcu);
-err_rpm:
-	intel_runtime_pm_put(i915, wakeref);
-	i915_gem_object_unpin_pages(obj);
 err:
-	switch (ret) {
-	case -EIO:
-		/*
-		 * We eat errors when the gpu is terminally wedged to avoid
-		 * userspace unduly crashing (gl has no provisions for mmaps to
-		 * fail). But any other -EIO isn't ours (e.g. swap in failure)
-		 * and so needs to be reported.
-		 */
-		if (!i915_terminally_wedged(i915))
-			return VM_FAULT_SIGBUS;
-		/* else: fall through */
-	case -EAGAIN:
-		/*
-		 * EAGAIN means the gpu is hung and we'll wait for the error
-		 * handler to reset everything when re-faulting in
-		 * i915_mutex_lock_interruptible.
-		 */
-	case 0:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	case -ENOMEM:
-		return VM_FAULT_OOM;
-	case -ENOSPC:
-	case -EFAULT:
-		return VM_FAULT_SIGBUS;
-	default:
-		WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
-		return VM_FAULT_SIGBUS;
-	}
+	return __object_fault_fini(ret, &fault);
 }
 
 void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
-- 
2.19.1

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread