[Intel-gfx] [PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[Tvrtko Ursulin]

On 03/07/2018 10:52, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-07-03 10:27:47)
>>
>> On 03/07/2018 09:30, Chris Wilson wrote:
>>> @@ -169,6 +177,8 @@ static int igt_ppgtt_alloc(void *arg)
>>>                ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
>>>        }
>>>    
>>> +     schedule_locked(i915);
>>> +
>>
>> Is it needed in this test? I glanced over and couldn't spot anything
>> struct mutexy in page table only manipulations it does.
> 
> struct_mutex is the guard for all drm_mm/i915_address_space, so while we
> may not need it exactly in this instance, it is the current mutex to
> use. I do have per-vm mutexes in the queue, it's quite scary.

I meant mutex dropping doesn't seem to be needed for this test, unless I 
am missing something.

>>> @@ -988,14 +1010,14 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
>>>        GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
>>>        GEM_BUG_ON(ppgtt->vm.closed);
>>>    
>>> -     err = func(dev_priv, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
>>> +     err = func(i915, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
>>
>> How about a less hacky solution where func is called unlocked it is
>> responsible to take struct_mutex across the parts which need it?
> 
> Convenience. Single threaded tests that didn't want to care about the
> details of struct_mutex. And then CI sets itself up in annoying ways!

Yeah, I get the convenience just worry it ends up that little bit harder 
to maintain. Okay I guess for testing..

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

Regards,

Tvrtko