[Intel-gfx] [PATCH 06/14] drm/i915: Cancel retire_worker on parking

Chris Wilson chris at chris-wilson.co.uk
Thu May 2 13:33:01 UTC 2019


Quoting Tvrtko Ursulin (2019-05-02 14:29:50)
> 
> On 01/05/2019 12:45, Chris Wilson wrote:
> > Replace the racy continuation check within retire_work with a definite
> > kill-switch on idling. The race was being exposed by gem_concurrent_blit
> > where the retire_worker would be terminated too early leaving us
> > spinning in debugfs/i915_drop_caches with nothing flushing the
> > retirement queue.
> > 
> > Although that the igt is trying to idle from one child while submitting
> > from another may be a contributing factor as to why  it runs so slowly...
> > 
> > Testcase: igt/gem_concurrent_blit
> > Fixes: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_pm.c             | 18 ++++++++++++------
> >   .../gpu/drm/i915/selftests/mock_gem_device.c   |  1 -
> >   2 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_pm.c b/drivers/gpu/drm/i915/i915_gem_pm.c
> > index ae91ad7cb31e..b239b55f84cd 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_pm.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_pm.c
> > @@ -30,15 +30,23 @@ static void idle_work_handler(struct work_struct *work)
> >   {
> >       struct drm_i915_private *i915 =
> >               container_of(work, typeof(*i915), gem.idle_work);
> > +     bool restart = true;
> >   
> > +     cancel_delayed_work_sync(&i915->gem.retire_work);
> >       mutex_lock(&i915->drm.struct_mutex);
> >   
> 
> You don't want to run another retire here? Since the retire worker might 
> have just been canceled I thought you should.

Why though? If there are retires outstanding, we won't sleep and want to
defer parking until after the next cycle.
-Chris


More information about the Intel-gfx mailing list