diff mbox series

[1/6] drm/i915: Reorder struct_mutex-vs-reset_lock in i915_gem_fault()

Message ID 20190221102924.13442-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [1/6] drm/i915: Reorder struct_mutex-vs-reset_lock in i915_gem_fault() | expand

Commit Message

Chris Wilson Feb. 21, 2019, 10:29 a.m. UTC
Annoyingly, struct_mutex was not entirely eliminated from the reset
pathway; for reasons of its own, intel_display_resumes() requires
struct_mutex to prepare the planes it already captured. To avoid the
immediate problem of a deadlock between the struct_mutex and the reset
srcu, we have to acquire the reset_lock before struct_mutex in
i915_gem_fault(). Now any wait underneath struct_mutex will result us in
having to forcibly reset all inflight rendering, less than ideal.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Mika Kuoppala Feb. 21, 2019, 12:05 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Annoyingly, struct_mutex was not entirely eliminated from the reset
> pathway; for reasons of its own, intel_display_resumes() requires
> struct_mutex to prepare the planes it already captured. To avoid the
> immediate problem of a deadlock between the struct_mutex and the reset
> srcu, we have to acquire the reset_lock before struct_mutex in
> i915_gem_fault(). Now any wait underneath struct_mutex will result us in
> having to forcibly reset all inflight rendering, less than ideal.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index cc88e7974422..62e4df1cbf87 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1834,9 +1834,15 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>  
>  	wakeref = intel_runtime_pm_get(dev_priv);
>  
> +	srcu = i915_reset_trylock(dev_priv);
> +	if (srcu < 0) {
> +		ret = srcu;
> +		goto err_rpm;
> +	}
> +
>  	ret = i915_mutex_lock_interruptible(dev);
>  	if (ret)
> -		goto err_rpm;
> +		goto err_reset;
>  
>  	/* Access to snoopable pages through the GTT is incoherent. */
>  	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
> @@ -1885,12 +1891,6 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>  	if (ret)
>  		goto err_unpin;
>  
> -	srcu = i915_reset_trylock(dev_priv);
> -	if (srcu < 0) {
> -		ret = srcu;
> -		goto err_fence;
> -	}
> -
>  	/* Finally, remap it using the new GTT offset */
>  	ret = remap_io_mapping(area,
>  			       area->vm_start + (vma->ggtt_view.partial.offset << PAGE_SHIFT),
> @@ -1898,7 +1898,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_reset;
> +		goto err_fence;
>  
>  	/* Mark as being mmapped into userspace for later revocation */
>  	assert_rpm_wakelock_held(dev_priv);
> @@ -1908,14 +1908,14 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>  
>  	i915_vma_set_ggtt_write(vma);
>  
> -err_reset:
> -	i915_reset_unlock(dev_priv, srcu);
>  err_fence:
>  	i915_vma_unpin_fence(vma);
>  err_unpin:
>  	__i915_vma_unpin(vma);
>  err_unlock:
>  	mutex_unlock(&dev->struct_mutex);
> +err_reset:
> +	i915_reset_unlock(dev_priv, srcu);
>  err_rpm:
>  	intel_runtime_pm_put(dev_priv, wakeref);
>  	i915_gem_object_unpin_pages(obj);
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cc88e7974422..62e4df1cbf87 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1834,9 +1834,15 @@  vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 
 	wakeref = intel_runtime_pm_get(dev_priv);
 
+	srcu = i915_reset_trylock(dev_priv);
+	if (srcu < 0) {
+		ret = srcu;
+		goto err_rpm;
+	}
+
 	ret = i915_mutex_lock_interruptible(dev);
 	if (ret)
-		goto err_rpm;
+		goto err_reset;
 
 	/* Access to snoopable pages through the GTT is incoherent. */
 	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
@@ -1885,12 +1891,6 @@  vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	if (ret)
 		goto err_unpin;
 
-	srcu = i915_reset_trylock(dev_priv);
-	if (srcu < 0) {
-		ret = srcu;
-		goto err_fence;
-	}
-
 	/* Finally, remap it using the new GTT offset */
 	ret = remap_io_mapping(area,
 			       area->vm_start + (vma->ggtt_view.partial.offset << PAGE_SHIFT),
@@ -1898,7 +1898,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_reset;
+		goto err_fence;
 
 	/* Mark as being mmapped into userspace for later revocation */
 	assert_rpm_wakelock_held(dev_priv);
@@ -1908,14 +1908,14 @@  vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 
 	i915_vma_set_ggtt_write(vma);
 
-err_reset:
-	i915_reset_unlock(dev_priv, srcu);
 err_fence:
 	i915_vma_unpin_fence(vma);
 err_unpin:
 	__i915_vma_unpin(vma);
 err_unlock:
 	mutex_unlock(&dev->struct_mutex);
+err_reset:
+	i915_reset_unlock(dev_priv, srcu);
 err_rpm:
 	intel_runtime_pm_put(dev_priv, wakeref);
 	i915_gem_object_unpin_pages(obj);