[PATCH i-g-t v5] tests/intel/xe_exec_fault_mode: Don't return early

[PATCH i-g-t v5] tests/intel/xe_exec_fault_mode: Don't return early

[Nirmoy Das]

Tests that are causing pagefaults should wait for exec to queue
ban/finish otherwise pending engine resets because of on-going
pagefaults would cause failure in subsequent tests to fail.

Not all execs will generate page faults and in such case reading ban
property is not enough but the signal should either -EIO or 0.
so read that instead.

v2: specify timeout reason and iterate over exec_queues(Andrzej)
v3: increase timeout
v4: check for signal status to be -EIO/0.
v5: code rearrangement and other minor fixes(Andrzej)

Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1630
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
 tests/intel/xe_exec_fault_mode.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/intel/xe_exec_fault_mode.c b/tests/intel/xe_exec_fault_mode.c
index 1f1f1e50b..9cc51b7d3 100644
--- a/tests/intel/xe_exec_fault_mode.c
+++ b/tests/intel/xe_exec_fault_mode.c
@@ -324,9 +324,19 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
 	xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
 		       bind_exec_queues[0], NSEC_PER_SEC);
 
-	if (!(flags & INVALID_FAULT) && !(flags & INVALID_VA)) {
+	if (flags & INVALID_FAULT) {
+		for (i = 0; i < n_execs; i++) {
+			int ret;
+			int64_t timeout = NSEC_PER_SEC;
+
+			ret = __xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
+					       exec_queues[i % n_exec_queues], &timeout);
+			igt_assert(ret == -EIO || ret == 0);
+		}
+	} else if (!(flags & INVALID_VA)) {
 		for (i = j; i < n_execs; i++)
-				igt_assert_eq(data[i].data, 0xc0ffee);
+			igt_assert_eq(data[i].data, 0xc0ffee);
+
 	}
 
 	for (i = 0; i < n_exec_queues; i++) {
-- 
2.42.0

RE: [PATCH i-g-t v5] tests/intel/xe_exec_fault_mode: Don't return early

[Upadhyay, Tejas]

> -----Original Message-----
> From: Das, Nirmoy <nirmoy.das@intel.com>
> Sent: Wednesday, September 4, 2024 2:15 PM
> To: igt-dev@lists.freedesktop.org
> Cc: kamil.konieczny@linux.intel.com; Das, Nirmoy <nirmoy.das@intel.com>;
> Hajda, Andrzej <andrzej.hajda@intel.com>; Brost, Matthew
> <matthew.brost@intel.com>; Upadhyay, Tejas <tejas.upadhyay@intel.com>
> Subject: [PATCH i-g-t v5] tests/intel/xe_exec_fault_mode: Don't return early
> 
> Tests that are causing pagefaults should wait for exec to queue ban/finish

Typo here it seems, " exec queue to ban/finish "

> otherwise pending engine resets because of on-going pagefaults would cause
> failure in subsequent tests to fail.
> 
> Not all execs will generate page faults and in such case reading ban property
> is not enough but the signal should either -EIO or 0.
> so read that instead.
> 
> v2: specify timeout reason and iterate over exec_queues(Andrzej)
> v3: increase timeout
> v4: check for signal status to be -EIO/0.
> v5: code rearrangement and other minor fixes(Andrzej)
> 
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1630
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  tests/intel/xe_exec_fault_mode.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/intel/xe_exec_fault_mode.c
> b/tests/intel/xe_exec_fault_mode.c
> index 1f1f1e50b..9cc51b7d3 100644
> --- a/tests/intel/xe_exec_fault_mode.c
> +++ b/tests/intel/xe_exec_fault_mode.c
> @@ -324,9 +324,19 @@ test_exec(int fd, struct
> drm_xe_engine_class_instance *eci,
>  	xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
>  		       bind_exec_queues[0], NSEC_PER_SEC);
> 
> -	if (!(flags & INVALID_FAULT) && !(flags & INVALID_VA)) {
> +	if (flags & INVALID_FAULT) {
> +		for (i = 0; i < n_execs; i++) {
> +			int ret;
> +			int64_t timeout = NSEC_PER_SEC;
> +
> +			ret = __xe_wait_ufence(fd, &data[i].exec_sync,
> USER_FENCE_VALUE,
> +					       exec_queues[i % n_exec_queues],
> &timeout);
> +			igt_assert(ret == -EIO || ret == 0);
> +		}
> +	} else if (!(flags & INVALID_VA)) {
>  		for (i = j; i < n_execs; i++)
> -				igt_assert_eq(data[i].data, 0xc0ffee);
> +			igt_assert_eq(data[i].data, 0xc0ffee);
> +
>  	}
> 
>  	for (i = 0; i < n_exec_queues; i++) {

With that fixed, I see its helping in syncing test progress and helps resolving wait fence timeout. Following "wait fence timeout -62" failure sometime " [174.208129] xe 0000:00:02.0: [drm] Schedule disable failed to respond " warning is also thrown. This might also help there.
LGTM,
Reviewed-by: Tejas Upadhyay <tejas.upadhyay@intel.com>

> --
> 2.42.0

Re: [PATCH i-g-t v5] tests/intel/xe_exec_fault_mode: Don't return early

[Andrzej Hajda]

On 04.09.2024 10:45, Nirmoy Das wrote:
> Tests that are causing pagefaults should wait for exec to queue
> ban/finish otherwise pending engine resets because of on-going
> pagefaults would cause failure in subsequent tests to fail.
>
> Not all execs will generate page faults and in such case reading ban
> property is not enough but the signal should either -EIO or 0.
> so read that instead.
>
> v2: specify timeout reason and iterate over exec_queues(Andrzej)
> v3: increase timeout
> v4: check for signal status to be -EIO/0.
> v5: code rearrangement and other minor fixes(Andrzej)
>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1630
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej

> ---
>   tests/intel/xe_exec_fault_mode.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/tests/intel/xe_exec_fault_mode.c b/tests/intel/xe_exec_fault_mode.c
> index 1f1f1e50b..9cc51b7d3 100644
> --- a/tests/intel/xe_exec_fault_mode.c
> +++ b/tests/intel/xe_exec_fault_mode.c
> @@ -324,9 +324,19 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>   	xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
>   		       bind_exec_queues[0], NSEC_PER_SEC);
>   
> -	if (!(flags & INVALID_FAULT) && !(flags & INVALID_VA)) {
> +	if (flags & INVALID_FAULT) {
> +		for (i = 0; i < n_execs; i++) {
> +			int ret;
> +			int64_t timeout = NSEC_PER_SEC;
> +
> +			ret = __xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
> +					       exec_queues[i % n_exec_queues], &timeout);
> +			igt_assert(ret == -EIO || ret == 0);
> +		}
> +	} else if (!(flags & INVALID_VA)) {
>   		for (i = j; i < n_execs; i++)
> -				igt_assert_eq(data[i].data, 0xc0ffee);
> +			igt_assert_eq(data[i].data, 0xc0ffee);
> +
>   	}
>   
>   	for (i = 0; i < n_exec_queues; i++) {