On Fri, Mar 03, 2017 at 07:26:34PM +0200, Mika Kuoppala wrote: > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > @@ -768,7 +776,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) > > struct intel_breadcrumbs *b = &engine->breadcrumbs; > > > > cancel_fake_irq(engine); > > - spin_lock_irq(&b->lock); > > + spin_lock_irq(&b->irq_lock); > > In here I was thinking that you want both locks help, but > then can't find a reason why. Perhaps just to ensure that > the wait tree stays still. Right, here we are just rearming the irq, so I didn't think taking the rb_lock helped with clarity. (Just the opposite, this showed nicely the divison in labour between the locks :) > > if (b->irq_enabled) > > irq_enable(engine); > > @@ -787,7 +795,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) > > if (b->irq_armed) > > enable_fake_irq(b); > > > > - spin_unlock_irq(&b->lock); > > + spin_unlock_irq(&b->irq_lock); > > } > > > > void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine) > > @@ -811,7 +819,7 @@ bool intel_breadcrumbs_busy(struct intel_engine_cs *engine) > > struct intel_breadcrumbs *b = &engine->breadcrumbs; > > bool busy = false; > > > > - spin_lock_irq(&b->lock); > > + spin_lock_irq(&b->rb_lock); > > Wrong lock taken and relased in this function? Here we can simply take the outer rb_lock, as that guarantees the rbtree won't change and therefore also the first_wait/irq_wait cannot be lost. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
