[igt-dev] [PATCH i-g-t v2 01/10] lib/igt_dummyload: add igt_cork

[Antonio Argenziano]

On 10/02/18 01:26, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-02-09 23:38:56)
>> From: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>
>> The "cork" bo (imported bo with attached fence) is used in several
>> tests to stall execution. Moving it to a common place makes the codebase
>> cleaner.
>>
>> Note that the actual test updates is done in follow up patches as it is
>> simpler to do in one go after one more common function is added in the
>> next patch.
>>
>> v2: don't use new/free naming, don't use dynamic alloc (Chris)
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> ---
>>   lib/igt_dummyload.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_dummyload.h |  8 ++++++++
>>   2 files changed, 66 insertions(+)
>>

...

>> +/**
>> + * igt_cork_unplug:
>> + * @cork: cork state from igt_cork_plug()
>> + *
>> + * This function unblocks the execution by signaling the fence attached to the
>> + * imported bo and does the necessary post-processing.
>> + * NOTE: the imported bo handle returned by igt_cork_plug is not closed during
>> + * this phase.
>> + */
>> +void igt_cork_unplug(struct igt_cork *cork)
>> +{
>> +       if (!cork || !cork->device)
>> +               return;
> 
> plug does assert, but not unplug?

I think the idea was to allow calling this multiple times (say you cork 
from the main thread and unplug from a child thread) but I didn't see 
any evidence of that. Will add an assert.

> 
>> +
> 
> Should have a comment here explaining that the reason we keep fence and
> call vgem_fence_signal (as it would be autosignalled by close()) is so
> that we catch when the fence timed-out.
> 
> Now, we could set this up with sw_sync (as used in a few other places
> for corking) to avoid that timeout. Iirc, I suggested the library
> routines provide both, since both are in use.

Are you suggesting two function, one per method or expose one interface 
and use a parameter to chose between the two methods. Also, do you think 
we should let the user chose which corking method to use when measuring 
ring size?

Thanks,
Antonio

> 
>> +       vgem_fence_signal(cork->device, cork->fence);
>> +       close(cork->device);
>> +       cork->device = 0;
> 
> -1 for an invalid fd.
> -Chris
>