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

[Daniel Vetter]

On Thu, Oct 08, 2015 at 07:31:39PM +0100, Tomas Elf wrote:
> 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.
> 
> Signed-off-by: Tomas Elf <tomas.elf at intel.com>

Should we instead just stop any irqs from the GT completely while we do
the reset (plus a synchronize_irq)? It's a bit heavy-weight, but probably
safer. Well not the entire GT, but all the rings under reset (as prep for
per-ring reset).

Just an idea which feels a bit safer design overall, but we can ofc also
lock down all the state properly with the irq-safe spinlocks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1666499..50c2dcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2738,6 +2738,8 @@ static void i915_gem_reset_ring_status(struct drm_i915_private *dev_priv,
>  static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>  					struct intel_engine_cs *ring)
>  {
> +	unsigned long flags;
> +
>  	while (!list_empty(&ring->active_list)) {
>  		struct drm_i915_gem_object *obj;
>  
> @@ -2753,6 +2755,8 @@ 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.
>  	 */
> +
> +	spin_lock_irqsave(&ring->execlist_lock, flags);
>  	while (!list_empty(&ring->execlist_queue)) {
>  		struct drm_i915_gem_request *submit_req;
>  
> @@ -2766,6 +2770,7 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>  
>  		i915_gem_request_unreference(submit_req);
>  	}
> +	spin_unlock_irqrestore(&ring->execlist_lock, flags);
>  
>  	/*
>  	 * We must free the requests after all the corresponding objects have
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch