[igt-dev] [PATCH i-g-t 1/2] lib/dummyload: Allow spin batches to be restarted

[Tvrtko Ursulin]

On 31/01/2018 15:49, Chris Wilson wrote:
> Quoting Chris Wilson (2018-01-31 15:44:41)
>> Quoting Tvrtko Ursulin (2018-01-31 12:34:40)
>>> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
>>> index ffa7e351dea3..b9f201d4afb6 100644
>>> --- a/lib/igt_dummyload.h
>>> +++ b/lib/igt_dummyload.h
>>> @@ -36,6 +36,12 @@ typedef struct igt_spin {
>>>          struct igt_list link;
>>>          uint32_t *batch;
>>>          int out_fence;
>>> +       struct drm_i915_gem_exec_object2 obj[2];
>>> +       struct drm_i915_gem_relocation_entry relocs[2];
>>> +       struct drm_i915_gem_execbuffer2 execbuf;
>>
>> I'm dubious whether we want to emit the dependency obj on resubmit. I
>> can see where it may be desired, and where it may be a hindrance.
>>
>> I think I'm coming down on the side that to reemit the dependency is
>> surprising, and would argue that if that is desired it should be an
>> explicit parameter to restart().
> 
> Stronger hint that it is surprising to remit the dependency obj is that
> it is owned by the caller, who may not realise it has been captured by
> the spin batch and will call gem_close() on it.

Hm, I kind of cannot bring myself to care, or at least see it as a 
problem. My thinking is, that if userspace uses restarts and nukes the 
dependency object in the meantime, it will get to know its mistake soon 
enough.

In other words, the only way in which the dependency has been "captured" 
is if caller explicitly uses the API.

So to me it still sounds logical to repeat the exactly same submission 
as was the original one.

But ultimately I care much more about getting this test done over API 
details so can change this if you insist.

Regards,

Tvrtko