| Message ID | 20180809063449.4474-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/selftests: Hold rpm for unparking | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > The call to i915_gem_unpark() checks that we hold a rpm wakeref before > taking a long term wakeref for i915->gt.awake. We should therefore make > sure we do hold the wakeref when directly calling unpark to disable > the retire worker. We as a caller need to hold the rpm as i915_gem_unpark does intel_runtime_pm_get_noresume() ? As I don't see any other checks before gt.awake being set. -Mika > > Fixes: 932cac10c8fb ("drm/i915/selftests: Prevent background reaping of active objects") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > --- > .../gpu/drm/i915/selftests/i915_gem_object.c | 20 +++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > index d9eca1b02aee..6d3516d5bff9 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > @@ -499,6 +499,19 @@ static bool assert_mmap_offset(struct drm_i915_private *i915, > return err == expected; > } > > +static void disable_retire_worker(struct drm_i915_private *i915) > +{ > + mutex_lock(&i915->drm.struct_mutex); > + if (!i915->gt.active_requests++) { > + intel_runtime_pm_get(i915); > + i915_gem_unpark(i915); > + intel_runtime_pm_put(i915); > + } > + mutex_unlock(&i915->drm.struct_mutex); > + cancel_delayed_work_sync(&i915->gt.retire_work); > + cancel_delayed_work_sync(&i915->gt.idle_work); > +} > + > static int igt_mmap_offset_exhaustion(void *arg) > { > struct drm_i915_private *i915 = arg; > @@ -509,12 +522,7 @@ static int igt_mmap_offset_exhaustion(void *arg) > int loop, err; > > /* Disable background reaper */ > - mutex_lock(&i915->drm.struct_mutex); > - if (!i915->gt.active_requests++) > - i915_gem_unpark(i915); > - mutex_unlock(&i915->drm.struct_mutex); > - cancel_delayed_work_sync(&i915->gt.retire_work); > - cancel_delayed_work_sync(&i915->gt.idle_work); > + disable_retire_worker(i915); > GEM_BUG_ON(!i915->gt.awake); > > /* Trim the device mmap space to only a page */ > -- > 2.18.0
Quoting Mika Kuoppala (2018-08-09 11:40:35) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > The call to i915_gem_unpark() checks that we hold a rpm wakeref before > > taking a long term wakeref for i915->gt.awake. We should therefore make > > sure we do hold the wakeref when directly calling unpark to disable > > the retire worker. > > We as a caller need to hold the rpm as i915_gem_unpark does > intel_runtime_pm_get_noresume() ? Correct. noresume says that the caller must have down the synchronous rpm_get for us. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > The call to i915_gem_unpark() checks that we hold a rpm wakeref before > taking a long term wakeref for i915->gt.awake. We should therefore make > sure we do hold the wakeref when directly calling unpark to disable > the retire worker. > > Fixes: 932cac10c8fb ("drm/i915/selftests: Prevent background reaping of active objects") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c index d9eca1b02aee..6d3516d5bff9 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c @@ -499,6 +499,19 @@ static bool assert_mmap_offset(struct drm_i915_private *i915, return err == expected; } +static void disable_retire_worker(struct drm_i915_private *i915) +{ + mutex_lock(&i915->drm.struct_mutex); + if (!i915->gt.active_requests++) { + intel_runtime_pm_get(i915); + i915_gem_unpark(i915); + intel_runtime_pm_put(i915); + } + mutex_unlock(&i915->drm.struct_mutex); + cancel_delayed_work_sync(&i915->gt.retire_work); + cancel_delayed_work_sync(&i915->gt.idle_work); +} + static int igt_mmap_offset_exhaustion(void *arg) { struct drm_i915_private *i915 = arg; @@ -509,12 +522,7 @@ static int igt_mmap_offset_exhaustion(void *arg) int loop, err; /* Disable background reaper */ - mutex_lock(&i915->drm.struct_mutex); - if (!i915->gt.active_requests++) - i915_gem_unpark(i915); - mutex_unlock(&i915->drm.struct_mutex); - cancel_delayed_work_sync(&i915->gt.retire_work); - cancel_delayed_work_sync(&i915->gt.idle_work); + disable_retire_worker(i915); GEM_BUG_ON(!i915->gt.awake); /* Trim the device mmap space to only a page */
The call to i915_gem_unpark() checks that we hold a rpm wakeref before taking a long term wakeref for i915->gt.awake. We should therefore make sure we do hold the wakeref when directly calling unpark to disable the retire worker. Fixes: 932cac10c8fb ("drm/i915/selftests: Prevent background reaping of active objects") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> --- .../gpu/drm/i915/selftests/i915_gem_object.c | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)