Re: [PATCH i-g-t] tests/intel/xe_exec_fault_mode: Fix misuse of __xe_wait_ufence

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, Apr 19, 2024 at 11:57:39AM -0700, Brian Welty wrote:
> When confirming that xe_exec() user_fences completed, this is performed
> in a loop.  When using __xe_wait_ufence(), it returns an updated timeout
> value to reflect how much time was remaining.  As side-effect, when calling
> in a loop without resetting the timeout, the timeout value will get
> progressively smaller over many iterations.
> We don't want a smaller timeout to be used on each subsequent call to
> __xe_wait_ufence(), so fix here is to reset the timeout inside the loop.
> This issue is visible in simulation runs due to the slower execution time,
> where later iterations of the wait are failing due to timeout being too
> small.
> 
> Signed-off-by: Brian Welty <brian.welty@intel.com>
> ---
>  tests/intel/xe_exec_fault_mode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/intel/xe_exec_fault_mode.c b/tests/intel/xe_exec_fault_mode.c
> index 40fe1743e4..c0d082e7fa 100644
> --- a/tests/intel/xe_exec_fault_mode.c
> +++ b/tests/intel/xe_exec_fault_mode.c
> @@ -273,11 +273,12 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>  		}
>  	}
>  	if (!(flags & INVALID_FAULT)) {
> -		int64_t timeout = ONE_SEC;
> +		int64_t timeout;

Should we just move the whole variable declaration down inside the loop?
There doesn't appear to be a need to declare it at this level.

Either way,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

>  
>  		j = flags & INVALIDATE ? n_execs - 1 : 0;
>  
>  		for (i = j; i < n_execs; i++) {
> +			timeout = ONE_SEC;
>  			if (flags & INVALID_VA && !(flags & ENABLE_SCRATCH))
>  				igt_assert_eq(__xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
>  							       exec_queues[i % n_exec_queues], &timeout), -EIO);
> -- 
> 2.43.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation