[igt-dev] [PATCH i-g-t v2 1/2] tests/gem_exec_fence: Check stored values only for valid workloads

[Kamil Konieczny]

Hi Karolina,

On 2022-07-26 at 12:13:11 +0200, Karolina Drobnik wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
> 
> test_fence_await verifies if a fence used to pipeline work signals
> correctly. await-hang and nb-await-hang test cases inject GPU hang,
> which causes an erroneous state, meaning the fence will be signaled
> without execution. The error causes an instant reset of the command
> streamer for the hanging workload. This revealed a problem with how
> we verify the fence state and results. The test assumes that the
> error notification happens after a hang is declared, which takes
> longer than submitting the next set of fences, making the test pass
> every time. With the immediate reset, this might not happen, so the

As I understand it, reset can happen only after hang was detected ?

> assertion fails, as there are no active fences in the GPU hang case.
> 
> Move the check for active fence to the path for non-hanging workload,
> and verify results only in this scenario.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Karolina Drobnik <karolina.drobnik at intel.com>
> ---
>  tests/i915/gem_exec_fence.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_fence.c b/tests/i915/gem_exec_fence.c
> index d46914c2..260aa82c 100644
> --- a/tests/i915/gem_exec_fence.c
> +++ b/tests/i915/gem_exec_fence.c
> @@ -350,18 +350,20 @@ static void test_fence_await(int fd, const intel_ctx_t *ctx,
>  	/* Long, but not too long to anger preemption disable checks */
>  	usleep(50 * 1000); /* 50 ms, typical preempt reset is 150+ms */
>  
> -	/* Check for invalidly completing the task early */
> -	igt_assert(fence_busy(spin->out_fence));

I tested this on drm-tip and this assert here did not trigger.
It is not blocker (it can be moved into hang-only block), so

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Regards,
Kamil

> -	for (int n = 0; n < i; n++)
> -		igt_assert_eq_u32(out[n], 0);
> +	if ((flags & HANG) == 0) {
> +		/* Check for invalidly completing the task early */
> +		igt_assert(fence_busy(spin->out_fence));
> +		for (int n = 0; n < i; n++)
> +			igt_assert_eq_u32(out[n], 0);
>  
> -	if ((flags & HANG) == 0)
>  		igt_spin_end(spin);
> +	}
>  
>  	igt_waitchildren();
>  
>  	gem_set_domain(fd, scratch, I915_GEM_DOMAIN_GTT, 0);
> -	while (i--)
> +	igt_assert(!fence_busy(spin->out_fence));
> +	while ((flags & HANG) == 0 && i--)
>  		igt_assert_eq_u32(out[i], i);
>  	munmap(out, 4096);
>  
> -- 
> 2.25.1
>