[v2] drm/i915: Don't mix srcu tag and negative error codes

Submitted by Chris Wilson on Sept. 12, 2019, 4:08 p.m.

Details

Message ID 20190912160834.30601-1-chris@chris-wilson.co.uk
State Accepted
Commit fda9fa19b09067b6a3ee6ff8d93e977957e0655c
Headers show
Series "drm/i915: Don't mix srcu tag and negative error codes" ( rev: 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Chris Wilson Sept. 12, 2019, 4:08 p.m.
While srcu may use an integer tag, it does not exclude potential error
codes and so may overlap with our own use of -EINTR. Use a separate
outparam to store the tag, and report the error code separately. While
changing the function signature allow the caller to choose whether or not
the potential wait may be interrupted.

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@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
Drop state parameters, the potential user evaporated.
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 6 ++----
 drivers/gpu/drm/i915/gt/intel_reset.c    | 8 +++-----
 drivers/gpu/drm/i915/gt/intel_reset.h    | 2 +-
 3 files changed, 6 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 82db2b783123..1748e63156a2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -245,11 +245,9 @@  vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 
 	wakeref = intel_runtime_pm_get(rpm);
 
-	srcu = intel_gt_reset_trylock(ggtt->vm.gt);
-	if (srcu < 0) {
-		ret = srcu;
+	ret = intel_gt_reset_trylock(ggtt->vm.gt, &srcu);
+	if (ret)
 		goto err_rpm;
-	}
 
 	ret = i915_mutex_lock_interruptible(dev);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 296bbc7745fb..8327220ac558 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1214,10 +1214,8 @@  void intel_gt_handle_error(struct intel_gt *gt,
 	intel_runtime_pm_put(&gt->i915->runtime_pm, wakeref);
 }
 
-int intel_gt_reset_trylock(struct intel_gt *gt)
+int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu)
 {
-	int srcu;
-
 	might_lock(&gt->reset.backoff_srcu);
 	might_sleep();
 
@@ -1232,10 +1230,10 @@  int intel_gt_reset_trylock(struct intel_gt *gt)
 
 		rcu_read_lock();
 	}
-	srcu = srcu_read_lock(&gt->reset.backoff_srcu);
+	*srcu = srcu_read_lock(&gt->reset.backoff_srcu);
 	rcu_read_unlock();
 
-	return srcu;
+	return 0;
 }
 
 void intel_gt_reset_unlock(struct intel_gt *gt, int tag)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
index 37a987b17108..52c00199e069 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.h
+++ b/drivers/gpu/drm/i915/gt/intel_reset.h
@@ -38,7 +38,7 @@  int intel_engine_reset(struct intel_engine_cs *engine,
 
 void __i915_request_reset(struct i915_request *rq, bool guilty);
 
-int __must_check intel_gt_reset_trylock(struct intel_gt *gt);
+int __must_check intel_gt_reset_trylock(struct intel_gt *gt, int *srcu);
 void intel_gt_reset_unlock(struct intel_gt *gt, int tag);
 
 void intel_gt_set_wedged(struct intel_gt *gt);

Comments

Chris Wilson <chris@chris-wilson.co.uk> writes:

> While srcu may use an integer tag, it does not exclude potential error
> codes and so may overlap with our own use of -EINTR. Use a separate
> outparam to store the tag, and report the error code separately. While
> changing the function signature allow the caller to choose whether or not
> the potential wait may be interrupted.
>
> 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@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
> Drop state parameters, the potential user evaporated.
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 6 ++----
>  drivers/gpu/drm/i915/gt/intel_reset.c    | 8 +++-----
>  drivers/gpu/drm/i915/gt/intel_reset.h    | 2 +-
>  3 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 82db2b783123..1748e63156a2 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -245,11 +245,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>  
>  	wakeref = intel_runtime_pm_get(rpm);
>  
> -	srcu = intel_gt_reset_trylock(ggtt->vm.gt);
> -	if (srcu < 0) {
> -		ret = srcu;
> +	ret = intel_gt_reset_trylock(ggtt->vm.gt, &srcu);
> +	if (ret)
>  		goto err_rpm;
> -	}
>  
>  	ret = i915_mutex_lock_interruptible(dev);
>  	if (ret)
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 296bbc7745fb..8327220ac558 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1214,10 +1214,8 @@ void intel_gt_handle_error(struct intel_gt *gt,
>  	intel_runtime_pm_put(&gt->i915->runtime_pm, wakeref);
>  }
>  
> -int intel_gt_reset_trylock(struct intel_gt *gt)
> +int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu)
>  {
> -	int srcu;
> -
>  	might_lock(&gt->reset.backoff_srcu);
>  	might_sleep();
>  
> @@ -1232,10 +1230,10 @@ int intel_gt_reset_trylock(struct intel_gt *gt)
>  
>  		rcu_read_lock();
>  	}
> -	srcu = srcu_read_lock(&gt->reset.backoff_srcu);
> +	*srcu = srcu_read_lock(&gt->reset.backoff_srcu);
>  	rcu_read_unlock();
>  
> -	return srcu;
> +	return 0;
>  }
>  
>  void intel_gt_reset_unlock(struct intel_gt *gt, int tag)
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
> index 37a987b17108..52c00199e069 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.h
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.h
> @@ -38,7 +38,7 @@ int intel_engine_reset(struct intel_engine_cs *engine,
>  
>  void __i915_request_reset(struct i915_request *rq, bool guilty);
>  
> -int __must_check intel_gt_reset_trylock(struct intel_gt *gt);
> +int __must_check intel_gt_reset_trylock(struct intel_gt *gt, int *srcu);
>  void intel_gt_reset_unlock(struct intel_gt *gt, int tag);
>  
>  void intel_gt_set_wedged(struct intel_gt *gt);
> -- 
> 2.23.0