[Intel-gfx] [PATCH] drm/i915: Always double check for a missed interrupt for new bottom halves

[Chris Wilson]

On Tue, Jul 05, 2016 at 02:38:55PM +0100, Tvrtko Ursulin wrote:
> On 05/07/16 12:29, Chris Wilson wrote:
> >@@ -233,7 +231,15 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
> >  		GEM_BUG_ON(rb_first(&b->waiters) != &wait->node);
> >  		b->first_wait = wait;
> >  		smp_store_mb(b->tasklet, wait->tsk);
> >-		first = __intel_breadcrumbs_enable_irq(b);
> >+		/* After assigning ourselves as the new bottom-half, we must
> >+		 * perform a cursory check to prevent a missed interrupt.
> >+		 * Either we miss the interrupt whilst programming the hardware,
> >+		 * or if there was a previous waiter (for a later seqno) they
> >+		 * may be woken instead of us (due to the inherent race
> >+		 * in the unlocked read of b->tasklet in the irq handler) and
> >+		 * so we miss the wake up.
> >+		 */
> >+		__intel_breadcrumbs_enable_irq(b);
> >  	}
> >  	GEM_BUG_ON(!b->tasklet);
> >  	GEM_BUG_ON(!b->first_wait);
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Even knowing the nature of the bug, I've found it very hard to hit. I've
written a test case that at the very least should exercise the multiple
waiters case, but making it hit the window where we swap the bottom
halves is a nigh-on impossible task (yet CI managed to hit it
almost consistently!).

I'm just thankful we do have some GEM tests in CI that did manage to hit
it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre