On Wed, Mar 15, 2017 at 06:20:16PM +0000, Tvrtko Ursulin wrote:
>
> On 15/03/2017 14:01, Chris Wilson wrote:
> >Check that we have disabled irqs before we take the spin_lock around
> >reassigned the breadcrumbs.irq_wait.
> >
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/intel_breadcrumbs.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >index 3f222dee4c25..35529b35a276 100644
> >--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >@@ -301,8 +301,11 @@ static inline void __intel_breadcrumbs_next(struct intel_engine_cs *engine,
> > {
> > struct intel_breadcrumbs *b = &engine->breadcrumbs;
> >
> >+ GEM_BUG_ON(!irqs_disabled());
> >+
> > spin_lock(&b->irq_lock);
> > GEM_BUG_ON(!b->irq_armed);
> >+ GEM_BUG_ON(!b->irq_wait);
> > b->irq_wait = to_wait(next);
> > spin_unlock(&b->irq_lock);
> >
> >@@ -395,8 +398,10 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
> > }
> >
> > if (first) {
> >- spin_lock(&b->irq_lock);
> > GEM_BUG_ON(rb_first(&b->waiters) != &wait->node);
> >+ GEM_BUG_ON(!irqs_disabled());
> >+
> >+ spin_lock(&b->irq_lock);
> > b->irq_wait = wait;
> > /* After assigning ourselves as the new bottom-half, we must
> > * perform a cursory check to prevent a missed interrupt.
> >
>
> A single GEM_BUG_ON(!irqs_disabled()) at the top of
> __intel_engine_add_wait might be more logical?
I wanted to associate it with b->irq_lock, that was my thinking.
b->rb_lock also sadly has to be irqsafe.
__intel_breadcrumbs_next() also serves remove_wait, did you mean to
remove the assert there as well?
We can safely ignore this patch, it should be catered by lockdep fairly
well, I was just being paranoid and going through the possible causes
and documenting my progress.
> As a weakly related side note, there is a stale comment mentioning
> b->lock in intel_engine_enable_signalling.
Ta.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
