[Intel-gfx] [PATCH] drm/i915: Serialise i915_active_wait() with its retirement

[Tvrtko Ursulin]

On 29/11/2019 11:28, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-11-29 11:15:46)
>>
>> On 29/11/2019 09:39, Chris Wilson wrote:
>>> As the i915_active.retire() may be running on another CPU as we detect
>>> that the i915_active is idle, we may not wait for the retirement itself.
>>> Wait for the remote callback by waiting for the retirement worker.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> ---
>>>    drivers/gpu/drm/i915/i915_active.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
>>> index 479195ecbc6c..e8630ee33336 100644
>>> --- a/drivers/gpu/drm/i915/i915_active.c
>>> +++ b/drivers/gpu/drm/i915/i915_active.c
>>> @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
>>>        if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
>>>                return -EINTR;
>>>    
>>> +     flush_work(&ref->work);
>>>        return 0;
>>>    }
>>>    
>>>
>>
>> Hm, but wake_up_war is in the worker so how does wait_var_event wake the
>> waiter up before it has been retired?
> 
> Remember the wait_event pattern is to skip the wait if COND is already
> met. So since the first thing the retirement does is the
> atomic_dec_and_test(), we can see ref->count == 0 very early, long
> before ref->retire() is called. Our selftest is checking that if
> i915_active_wait() reports completion, the callback has also run and
> that the i915_active can then be destroyed.

True!

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko