Re: [PATCH 06/14] drm/i915: Cancel retire_worker on parking

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 02/05/2019 14:33, Chris Wilson wrote:
> 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@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@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.

In this case what is the point of cancel_delayed_work_*sync* and not 
just the async cancel?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx