[Intel-gfx] [PATCH i-g-t] igt/gem_eio: Preserve batch between reset-stress iterations

[Tvrtko Ursulin]

On 08/08/2018 13:47, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-08-08 13:38:53)
>>
>> On 08/08/2018 12:31, Chris Wilson wrote:
>>> We can keep the original batch around and avoid recreating it between
>>> reset iterations to focus on the impact of resets.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>>> ---
>>>    tests/gem_eio.c | 30 +++++++++++++++++-------------
>>>    1 file changed, 17 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tests/gem_eio.c b/tests/gem_eio.c
>>> index de161332d..5250a414c 100644
>>> --- a/tests/gem_eio.c
>>> +++ b/tests/gem_eio.c
>>> @@ -650,35 +650,38 @@ static void reset_stress(int fd,
>>>                         uint32_t ctx0, unsigned int engine,
>>>                         unsigned int flags)
>>>    {
>>> +     const uint32_t bbe = MI_BATCH_BUFFER_END;
>>> +     struct drm_i915_gem_exec_object2 obj = {
>>> +             .handle = gem_create(fd, 4096)
>>> +     };
>>> +     struct drm_i915_gem_execbuffer2 execbuf = {
>>> +             .buffers_ptr = to_user_pointer(&obj),
>>> +             .buffer_count = 1,
>>> +             .flags = engine,
>>> +     };
>>> +     gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
>>> +
>>>        igt_until_timeout(5) {
>>> -             struct drm_i915_gem_execbuffer2 execbuf = { };
>>> -             struct drm_i915_gem_exec_object2 obj = { };
>>> -             uint32_t bbe = MI_BATCH_BUFFER_END;
>>> +             uint32_t ctx = context_create_safe(fd);
>>
>> There is still this per loop...
> 
> I thought that was intentional :)
> 
> It felt like the spirit of the test to try and mix up the contexts as
> much as possible; one constant, one fresh.

Yes definitely intentional (required), I was just puzzled why you are 
concerned with removing one gem_create when we have more ioctls in the 
loop anyway.

> 
>>>                igt_spin_t *hang;
>>>                unsigned int i;
>>> -             uint32_t ctx;
>>>    
>>>                gem_quiescent_gpu(fd);
>>>    
>>>                igt_require(i915_reset_control(flags & TEST_WEDGE ?
>>>                                               false : true));
>>>    
>>> -             ctx = context_create_safe(fd);
>>> -
>>>                /*
>>>                 * Start executing a spin batch with some queued batches
>>>                 * against a different context after it.
>>>                 */
>>>                hang = spin_sync(fd, ctx0, engine);
>>
>> ... and a ton of operations in this one, so I wonder why bother with one
>> batch?
> 
> Because I don't have spin_sync() in my pattern recognition matrix yet.
> One excuse is that it doesn't have any create verb in its name, so easy
> to forget its hidden costs.

Okay. :) I blame the poor name on it being local. :)


Regards,

Tvrtko