[Intel-gfx] [PATCH 18/23] drm/i915: Dirty hack to fix selftests locking inversion

[Maarten Lankhorst]

Op 03-07-2020 om 15:48 schreef Tvrtko Ursulin:
>
> On 03/07/2020 13:22, Maarten Lankhorst wrote:
>> Some i915 selftests still use i915_vma_lock() as inner lock, and
>> intel_context_create_request() intel_timeline->mutex as outer lock.
>> Fortunately for selftests this is not an issue, they should be fixed
>> but we can move ahead and cleanify lockdep now.
>
> Mentions and existence of "dirty hacks" will hopefully be removed from the series before it can be considered merge ready?
>
> Regards,
>
> Tvrtko
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_context.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
>> index 64948386630f..fe9fff5a63b1 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_context.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
>> @@ -459,6 +459,18 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
>>       rq = i915_request_create(ce);
>>       intel_context_unpin(ce);
>>   +    if (IS_ERR(rq))
>> +        return rq;
>> +
>> +    /*
>> +     * timeline->mutex should be the inner lock, but is used as outer lock.
>> +     * Hack around this to shut up lockdep in selftests..
>> +     */
>> +    lockdep_unpin_lock(&ce->timeline->mutex, rq->cookie);
>> +    mutex_release(&ce->timeline->mutex.dep_map, _RET_IP_);
>> +    mutex_acquire(&ce->timeline->mutex.dep_map, SINGLE_DEPTH_NESTING, 0, _RET_IP_);
>> +    rq->cookie = lockdep_pin_lock(&ce->timeline->mutex);
>> +
>>       return rq;
>>   }
>>  

Hey,

We're trying to invert the locking order with vma lock vs request lock, while this is a hack,
it will not affect normal running code, it's only meant to shut up lockdep in the selftests.

This is mainly so we can fix the selftests one by one, without breaking the world. Ideally
when mm.obj lands, we already corrected the lock ordering. We may keep this macro for selftests,
but until lock reordering in selftests is complete we will need this temporarily.

~Maarten