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

[Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>]

Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>

> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx