Re: [PATCH] drm/i915/selftests: Wait for the dma-fence timeout

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 17/01/2018 12:44, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-17 12:27:50)
>>
>> On 17/01/2018 12:15, Chris Wilson wrote:
>>> When testing that the timeout fired, we need to be sure we have waited
>>> just long enough for the timeout to have occurred and for the softirq
>>> (on another cpu) to have completed. Sleeping for an arbitrary amount is
>>> prone to error, so wait for the timeout instead and complain if it was
>>> too late.
>>>
>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=104670
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 30 +++++++++-----------------
>>>    1 file changed, 10 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
>>> index 4fb51deb81a1..f171db16a1c4 100644
>>> --- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
>>> +++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
>>> @@ -661,7 +661,7 @@ static int test_dma_fence(void *arg)
>>>    {
>>>        struct i915_sw_fence *timeout = NULL, *not = NULL;
>>>        unsigned long delay = i915_selftest.timeout_jiffies;
>>> -     unsigned long end, sleep;
>>> +     unsigned long start, end;
>>>        struct dma_fence *dma;
>>>        int err;
>>>    
>>> @@ -688,36 +688,26 @@ static int test_dma_fence(void *arg)
>>>        }
>>>    
>>>        /* We round the timeout for the fence up to the next second */
>>> -     end = round_jiffies_up(jiffies + delay);
>>> +     start = jiffies;
>>> +     end = round_jiffies_up(start + delay);
>>>    
>>> -     sleep = jiffies_to_usecs(delay) / 3;
>>> -     usleep_range(sleep, 2 * sleep);
>>> -     if (time_after(jiffies, end)) {
>>> -             pr_debug("Slept too long, delay=%lu, skipping!\n", delay);
>>> -             goto skip;
>>> +     if (wait_event_interruptible(timeout->wait,
>>> +                                  i915_sw_fence_done(timeout))) {
>>> +             err = -EINTR;
>>> +             goto err;
>>
>> But can now hang forever if broken.
> 
> Which is why I allowed it to be interrupted. Maybe wait_event_killable()?
> 
>> How about short sleeps in a loop until a timeout?
> 
> I kind of like the idea of using the waitqueue now :)
> 
> How about if I attach a timer to kick the waitqueue instead.

I would hate to be the trigger of overkills. But I think it is nice if 
the test times out on it's own. So either option is fine by me. (Short 
sleep polling loop, or timer kick with timeout checking.)

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx