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