[Intel-gfx] [PATCH 02/18] drm/i915/gt: Mark up the nested engine-pm timeline lock as irqsafe

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-19 09:43:29)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > We use a fake timeline->mutex lock to reassure lockdep that the timeline
> > is always locked when emitting requests. However, the use inside
> > __engine_park() may be inside hardirq and so lockdep now complains about
> > the mixed irq-state of the nested locked. Disable irqs around the
> > lockdep tracking to keep it happy.
> >
> > Fixes: 6c69a45445af ("drm/i915/gt: Mark context->active_count as protected by timeline->mutex")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > index 5f03f7dcad72..5d003751968b 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > @@ -37,9 +37,15 @@ static int __engine_unpark(struct intel_wakeref *wf)
> >       return 0;
> >  }
> >  
> > +#if IS_ENABLED(CONFIG_LOCKDEP)
> > +
> >  static inline void __timeline_mark_lock(struct intel_context *ce)
> >  {
> > +     unsigned long flags;
> > +
> > +     local_irq_save(flags);
> >       mutex_acquire(&ce->timeline->mutex.dep_map, 2, 0, _THIS_IP_);
> > +     local_irq_restore(flags);
> 
> I am starting to have second thoughts. One could argue that the
> net effect is on positive side.
> 
> But we diverge on behaviour now.

Are you worrying about the #if-#else and accidentally sticking more code
in there?
-Chris