All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Recursive i915_reset_trylock() verboten
@ 2019-02-12 13:08 Chris Wilson
  2019-02-12 13:08 ` [PATCH 2/2] drm/i915: Detect potential i915_reset_trylock() lockups Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Chris Wilson @ 2019-02-12 13:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

We cannot nest i915_reset_trylock() as the inner may wait for the
I915_RESET_BACKOFF which in turn is waiting upon sync_srcu who is
waiting for our outermost lock. As we take the reset srcu around the
fence update, we have to defer taking it in i915_gem_fault() until after
we acquire the pin on the fence to avoid nesting. This is a little ugly,
but still works. If a reset occurs between i915_vma_pin_fence() and the
second reset lock, the reset will restore the fence register back to the
pinned value before the reset lock allows us to proceed (our mmap won't
be revoked as we haven't yet marked it as being a userfault as that
requires us to hold the reset lock), so the pagefault is still
serialised with the revocation in reset.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109605
Fixes: 2caffbf11762 ("drm/i915: Revoke mmaps and prevent access to fence registers across reset")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c8c355bec091..ae1467a74a08 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1923,16 +1923,16 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	if (ret)
 		goto err_unpin;
 
+	ret = i915_vma_pin_fence(vma);
+	if (ret)
+		goto err_unpin;
+
 	srcu = i915_reset_trylock(dev_priv);
 	if (srcu < 0) {
 		ret = srcu;
-		goto err_unpin;
+		goto err_fence;
 	}
 
-	ret = i915_vma_pin_fence(vma);
-	if (ret)
-		goto err_reset;
-
 	/* Finally, remap it using the new GTT offset */
 	ret = remap_io_mapping(area,
 			       area->vm_start + (vma->ggtt_view.partial.offset << PAGE_SHIFT),
@@ -1940,7 +1940,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 			       min_t(u64, vma->size, area->vm_end - area->vm_start),
 			       &ggtt->iomap);
 	if (ret)
-		goto err_fence;
+		goto err_reset;
 
 	/* Mark as being mmapped into userspace for later revocation */
 	assert_rpm_wakelock_held(dev_priv);
@@ -1950,10 +1950,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 
 	i915_vma_set_ggtt_write(vma);
 
-err_fence:
-	i915_vma_unpin_fence(vma);
 err_reset:
 	i915_reset_unlock(dev_priv, srcu);
+err_fence:
+	i915_vma_unpin_fence(vma);
 err_unpin:
 	__i915_vma_unpin(vma);
 err_unlock:
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 1/2] drm/i915: Recursive i915_reset_trylock() verboten
@ 2019-02-12 12:47 Chris Wilson
  2019-02-12 12:47 ` [PATCH 2/2] drm/i915: Detect potential i915_reset_trylock() lockups Chris Wilson
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2019-02-12 12:47 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

We cannot nest i915_reset_trylock() as the inner may wait for the
I915_RESET_BACKOFF which in turn is waiting upon sync_srcu who is
waiting for our outermost lock. As we take the reset srcu around the
fence update, we have to defer taking it in i915_gem_fault() until after
we acquire the pin on the fence to avoid nesting. This is a little ugly,
but still works. If a reset occurs between i915_vma_pin_fence() and the
second reset lock, the reset will restore the fence register back to the
pinned value before the reset lock allows us to proceed (our mmap won't
be revoked as we haven't yet marked it as being a userfault as that
requires us to hold the reset lock), so the pagefault is still
serialised with the revocation in reset.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109605
Fixes: 2caffbf11762 ("drm/i915: Revoke mmaps and prevent access to fence registers across reset")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c8c355bec091..ae1467a74a08 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1923,16 +1923,16 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	if (ret)
 		goto err_unpin;
 
+	ret = i915_vma_pin_fence(vma);
+	if (ret)
+		goto err_unpin;
+
 	srcu = i915_reset_trylock(dev_priv);
 	if (srcu < 0) {
 		ret = srcu;
-		goto err_unpin;
+		goto err_fence;
 	}
 
-	ret = i915_vma_pin_fence(vma);
-	if (ret)
-		goto err_reset;
-
 	/* Finally, remap it using the new GTT offset */
 	ret = remap_io_mapping(area,
 			       area->vm_start + (vma->ggtt_view.partial.offset << PAGE_SHIFT),
@@ -1940,7 +1940,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 			       min_t(u64, vma->size, area->vm_end - area->vm_start),
 			       &ggtt->iomap);
 	if (ret)
-		goto err_fence;
+		goto err_reset;
 
 	/* Mark as being mmapped into userspace for later revocation */
 	assert_rpm_wakelock_held(dev_priv);
@@ -1950,10 +1950,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 
 	i915_vma_set_ggtt_write(vma);
 
-err_fence:
-	i915_vma_unpin_fence(vma);
 err_reset:
 	i915_reset_unlock(dev_priv, srcu);
+err_fence:
+	i915_vma_unpin_fence(vma);
 err_unpin:
 	__i915_vma_unpin(vma);
 err_unlock:
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-12 16:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-12 13:08 [PATCH 1/2] drm/i915: Recursive i915_reset_trylock() verboten Chris Wilson
2019-02-12 13:08 ` [PATCH 2/2] drm/i915: Detect potential i915_reset_trylock() lockups Chris Wilson
2019-02-12 14:15   ` Mika Kuoppala
2019-02-12 13:37 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Recursive i915_reset_trylock() verboten Patchwork
2019-02-12 13:37 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-02-12 13:57 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-12 16:18 ` ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2019-02-12 12:47 [PATCH 1/2] " Chris Wilson
2019-02-12 12:47 ` [PATCH 2/2] drm/i915: Detect potential i915_reset_trylock() lockups Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.