[Intel-gfx] [PATCH 1/2] drm/i915/gt: Add to timeline requires the timeline mutex

[Tvrtko Ursulin]

On 25/07/2019 14:14, Chris Wilson wrote:
> Modifying a remote context requires careful serialisation with requests
> on that context, and that serialisation requires us to take their
> timeline->mutex. Make it so.
> 
> Note that while struct_mutex rules, we can't create more than one
> request in parallel, but that age is soon coming to an end.
> 
> v2: Though it doesn't affect the current users, contexts may share
> timelines so check if we already hold the right mutex.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_context.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 9292b6ca5e9c..d64b45f7ec6d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -254,10 +254,18 @@ int intel_context_prepare_remote_request(struct intel_context *ce,
>   	/* Only suitable for use in remotely modifying this context */
>   	GEM_BUG_ON(rq->hw_context == ce);
>   
> -	/* Queue this switch after all other activity by this context. */
> -	err = i915_active_request_set(&tl->last_request, rq);
> -	if (err)
> -		return err;
> +	if (rq->timeline != tl) { /* beware timeline sharing */
> +		err = mutex_lock_interruptible_nested(&tl->mutex,
> +						      SINGLE_DEPTH_NESTING);
> +		if (err)
> +			return err;
> +
> +		/* Queue this switch after current activity by this context. */
> +		err = i915_active_request_set(&tl->last_request, rq);
> +		if (err)
> +			goto unlock;
> +	}
> +	lockdep_assert_held(&tl->mutex);
>   
>   	/*
>   	 * Guarantee context image and the timeline remains pinned until the
> @@ -267,7 +275,12 @@ int intel_context_prepare_remote_request(struct intel_context *ce,
>   	 * words transfer the pinned ce object to tracked active request.
>   	 */
>   	GEM_BUG_ON(i915_active_is_idle(&ce->active));
> -	return i915_active_ref(&ce->active, rq->fence.context, rq);
> +	err = i915_active_ref(&ce->active, rq->fence.context, rq);
> +
> +unlock:
> +	if (rq->timeline != tl)
> +		mutex_unlock(&tl->mutex);
> +	return err;
>   }
>   
>   struct i915_request *intel_context_create_request(struct intel_context *ce)
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko