[Intel-gfx] [PATCH 04/11] drm/i915/execlists: Pull CSB reset under the timeline.lock

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-06-04 15:25:26)
> 
> On 31/05/2018 19:51, Chris Wilson wrote:
> > In the following patch, we will process the CSB interrupt under the
> > timeline.lock and not under the tasklet lock. This also means that we
> 
> Implied tasklet lock? Or tasklet serialization?

We were using the tasklet to provide exclusive access to the port[], so
were using it as a serialisation primitive and lock.
 
> > will need to protect access to common variables such as
> > execlists->csb_head with the timeline.lock during reset.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/intel_lrc.c | 7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index d207a1bf9dc9..ec54c29b610f 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1927,8 +1927,7 @@ static void execlists_reset(struct intel_engine_cs *engine,
> >                 engine->name, request ? request->global_seqno : 0,
> >                 intel_engine_get_seqno(engine));
> >   
> > -     /* See execlists_cancel_requests() for the irq/spinlock split. */
> > -     local_irq_save(flags);
> > +     spin_lock_irqsave(&engine->timeline.lock, flags);
> >   
> >       /*
> >        * Catch up with any missed context-switch interrupts.
> > @@ -1943,14 +1942,12 @@ static void execlists_reset(struct intel_engine_cs *engine,
> >       reset_irq(engine);
> 
> There is a synchronize_hardirq in this one so this could be a deadlock I 
> think depending on lock ordering. If another CPU enters the irq handler 
> and tries to take the timeline lock which this thread already has, then 
> they deadlock each other.

Nothing, and that would explain the deadlock in the other thread.
-Chris