Re: [PATCH v3 7/8] drm/i915: Grab execlist spinlock to avoid post-reset concurrency issues.

["Daniel, Thomas" <thomas.daniel@intel.com>]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Tomas Elf
> Sent: Friday, October 23, 2015 2:09 PM
> To: Intel-GFX@Lists.FreeDesktop.Org
> Subject: [Intel-gfx] [PATCH v3 7/8] drm/i915: Grab execlist spinlock to avoid
> post-reset concurrency issues.
> 
> Grab execlist lock when cleaning up execlist queues after GPU reset to avoid
> concurrency problems between the context event interrupt handler and the
> reset
> path immediately following a GPU reset.
> 
> * v2 (Chris Wilson):
> Do execlist check and use simpler form of spinlock functions.
> 
> * v3 (Tvrtko Ursulin):
> Don't hold the execlist_lock while dereferencing the requests since that might
> lead to sleep while IRQs are disabled.
> 
> Signed-off-by: Tomas Elf <tomas.elf@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c
> index e57061a..b492603 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2753,18 +2753,16 @@ static void i915_gem_reset_ring_cleanup(struct
> drm_i915_private *dev_priv,
>  	 * are the ones that keep the context and ringbuffer backing objects
>  	 * pinned in place.
>  	 */
> -	while (!list_empty(&ring->execlist_queue)) {
> -		struct drm_i915_gem_request *submit_req;
> 
> -		submit_req = list_first_entry(&ring->execlist_queue,
> -				struct drm_i915_gem_request,
> -				execlist_link);
> -		list_del(&submit_req->execlist_link);
> +	if (i915.enable_execlists) {
> +		spin_lock_irq(&ring->execlist_lock);
> 
> -		if (submit_req->ctx != ring->default_context)
> -			intel_lr_context_unpin(submit_req);
> +		/* list_splice_tail_init checks for empty lists */
> +		list_splice_tail_init(&ring->execlist_queue,
> +				      &ring->execlist_retired_req_list);
> 
> -		i915_gem_request_unreference(submit_req);
> +		spin_unlock_irq(&ring->execlist_lock);
> +		intel_execlists_retire_requests(ring);
>  	}
> 
>  	/*
> --
> 1.9.1

LGTM
Reviewed-by: Thomas Daniel <thomas.daniel@intel.com>


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