Re: [i-g-t 1/1] tests/intel/xe_exec_fault_mode: wait for the completion

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Oct 24, 2024 at 05:23:22PM -0700, fei.yang@intel.com wrote:
> From: Fei Yang <fei.yang@intel.com>
> 
> The execution on GPU is out of order, the completion of the
> last submission doesn't mean all the jobs are completed. We
> need to make sure all the jobs are completed before moving
> on to unbinding the buffer, otherwise the test would run
> into CAT errors.
> 
> Signed-off-by: Fei Yang <fei.yang@intel.com>
> ---
>  tests/intel/xe_exec_fault_mode.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/intel/xe_exec_fault_mode.c b/tests/intel/xe_exec_fault_mode.c
> index 9cc51b7d3..d416c773b 100644
> --- a/tests/intel/xe_exec_fault_mode.c
> +++ b/tests/intel/xe_exec_fault_mode.c
> @@ -305,7 +305,15 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>  		}
>  	}
>  	if (!(flags & INVALID_FAULT)) {
> -		j = flags & INVALIDATE ? n_execs - 1 : 0;
> +		/*
> +		 * For !RACE cases xe_wait_ufence has already been called in above
> +		 * for-loop, we should only wait for the completion of the last
> +		 * submission here. For RACE cases we need to wait for all submissions
> +		 * to complete because the GuC scheduling can be out of order, the
> +		 * completion of the last submission doesn't mean all submission are
> +		 * completed.
> +		 */
> +		j = (flags & INVALIDATE && !(flags & RACE)) ? n_execs - 1 : 0;

This makes sense and xe_exec_threads has similar code to this in
function test_compute_mode() and given that is threaded test much more
likely to hit this race.

I checked xe_exec_compute_mode and I believe we need a similar patch for
that test too. While you are here can you fixup that test too?

This patch LGTM though, with that:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

>  
>  		for (i = j; i < n_execs; i++) {
>  			int64_t timeout = NSEC_PER_SEC;
> -- 
> 2.25.1
>