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

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

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.

How about short sleeps in a loop until a timeout?

Regards,

Tvrtko

>   	}
>   
> -	if (i915_sw_fence_done(timeout) || i915_sw_fence_done(not)) {
> -		pr_err("Fences signaled too early\n");
> +	if (time_after(jiffies, 2*end - start)) {
> +		pr_err("Timeout (now %lu) later than expected (%lu)\n",
> +		       jiffies, end);
>   		goto err;
>   	}
>   
> -	do {
> -		sleep = jiffies_to_usecs(end - jiffies + 1);
> -		usleep_range(sleep, 2 * sleep);
> -	} while (!time_after(jiffies, end));
> -
>   	if (i915_sw_fence_done(not)) {
>   		pr_err("No timeout fence signaled!\n");
>   		goto err;
>   	}
>   
> -	if (!i915_sw_fence_done(timeout)) {
> -		pr_err("Timeout fence unsignaled!\n");
> -		goto err;
> -	}
> -
> -skip:
>   	dma_fence_signal(dma);
>   
>   	if (!i915_sw_fence_done(timeout) || !i915_sw_fence_done(not)) {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx