Re: [PATCH v2 i-g-t 3/3] tests/intel/xe_eudebug: Add basic-vm-bind-ufence-sigint-client test

["Manszewski, Christoph" <christoph.manszewski@intel.com>]

Hi Dominik

On 11.12.2024 14:50, Dominik Karol Piątkowski wrote:
> Add a test that gives user fence in application, holds it, sends SIGINT
> to client and checks if anything breaks.
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> ---
> v2:
> - Cleanup after the client in basic-vm-bind-ufence-sigint-client,
>    merge the test into test_basic_ufence
> ---
>   tests/intel/xe_eudebug.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/tests/intel/xe_eudebug.c b/tests/intel/xe_eudebug.c
> index 77ce905f0..c543eb064 100644
> --- a/tests/intel/xe_eudebug.c
> +++ b/tests/intel/xe_eudebug.c
> @@ -65,6 +65,7 @@ static void test_sysfs_toggle(int fd)
>   #define EXEC_QUEUES_PLACEMENTS		(1 << 8)
>   #define VM_BIND_DELAY_UFENCE_ACK	(1 << 9)
>   #define VM_BIND_UFENCE_RECONNECT	(1 << 10)
> +#define VM_BIND_UFENCE_SIGINT_CLIENT	(1 << 11)
>   #define TEST_DISCOVERY			(1 << 31)
>   
>   #define PAGE_SIZE SZ_4K
> @@ -2135,6 +2136,10 @@ static int wait_for_ufence_events(struct ufence_priv *priv, int timeout_ms)
>    * Description:
>    *	Give user fence in application, hold it, drop the debugger connection and check if anything
>    *	breaks. Expect that held acks are released when connection is dropped.
> + *
> + * SUBTEST: basic-vm-bind-ufence-sigint-client
> + * Description:
> + *	Give user fence in application, hold it, send SIGINT to client and check if anything breaks.
>    */
>   static void test_basic_ufence(int fd, unsigned int flags)
>   {
> @@ -2164,6 +2169,14 @@ static void test_basic_ufence(int fd, unsigned int flags)
>   	if (flags & VM_BIND_DELAY_UFENCE_ACK)
>   		sleep(XE_EUDEBUG_DEFAULT_TIMEOUT_SEC * 4 / 5);
>   
> +	if (flags & VM_BIND_UFENCE_SIGINT_CLIENT) {
> +		kill(c->pid, SIGINT);
> +		c->pid = 0;
> +		xe_eudebug_debugger_stop_worker(d, 1);
> +		xe_eudebug_event_log_print(d->log, true);
> +		goto end;
> +	}
> +
>   	if (flags & VM_BIND_UFENCE_RECONNECT) {
>   		filter = XE_EUDEBUG_FILTER_EVENT_VM_BIND | XE_EUDEBUG_FILTER_EVENT_VM |
>   				XE_EUDEBUG_FILTER_EVENT_OPEN;
> @@ -2181,6 +2194,7 @@ static void test_basic_ufence(int fd, unsigned int flags)
>   
>   	xe_eudebug_session_check(s, true, filter);
>   
> +end:

Well 'goto' has it's place, but I think we could do without it here. How 
about:

```
@@ -2149,7 +2166,28 @@ static void test_basic_ufence(int fd, unsigned 
int flags)
         xe_eudebug_debugger_wait_stage(s, 
STAGE_CLIENT_WAIT_ON_UFENCE_DONE);
         xe_eudebug_assert_f(d, wait_for_ufence_events(priv, 
XE_EUDEBUG_DEFAULT_TIMEOUT_SEC * MSEC_PER_SEC) == 0,
                             "missing ufence events\n");
-       ack_fences(d);
+
+       if (flags & VM_BIND_DELAY_UFENCE_ACK)
+               sleep(XE_EUDEBUG_DEFAULT_TIMEOUT_SEC * 4 / 5);
+
+       if (flags & VM_BIND_UFENCE_SIGINT_CLIENT) {
+               /* Killing client so client log will be empty */
+               filter = XE_EUDEBUG_FILTER_ALL;
+
+               kill(c->pid, SIGINT);
+               c->pid = 0;
+               c->done = 1;
+       } else if (flags & VM_BIND_UFENCE_RECONNECT) {
+               /* Reconnecting debugger after client is done, debugger 
log will be cut */
+               filter = XE_EUDEBUG_FILTER_EVENT_VM_BIND | 
XE_EUDEBUG_FILTER_EVENT_VM |
+                        XE_EUDEBUG_FILTER_EVENT_OPEN;
+
+               xe_eudebug_debugger_detach(d);
+               xe_eudebug_client_wait_done(c);
+               igt_assert_eq(xe_eudebug_debugger_attach(d, c), 0);
+       } else {
+               ack_fences(d);
+       }
```

We have one central place which determines the way we release the 
fences, the rest of the function is common.

Note: this code uses "XE_EUDEBUG_FILTER_ALL" which is currently broken - 
I've sent a fix here: https://patchwork.freedesktop.org/series/142438/

Thanks,
Christoph



>   	xe_eudebug_session_destroy(s);
>   	ufence_priv_destroy(priv);
>   }
> @@ -2842,6 +2856,9 @@ igt_main
>   	igt_subtest("basic-vm-bind-ufence-reconnect")
>   		test_basic_ufence(fd, VM_BIND_UFENCE_RECONNECT);
>   
> +	igt_subtest("basic-vm-bind-ufence-sigint-client")
> +		test_basic_ufence(fd, VM_BIND_UFENCE_SIGINT_CLIENT);
> +
>   	igt_subtest("vma-ufence")
>   		test_vma_ufence(fd, 0);
>