Re: [PATCH v7 i-g-t 2/9] lib/xe_eudebug: Don't fail in __wait_token if pipe was broken

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

Hi Dominik,

On 26.09.2025 09:47, Dominik Karol Piątkowski wrote:
> If safe_pipe_read returns -errno, print it and let the upper layer
> decide if this is acceptable. This will allow for debugging if something
> goes wrong, while reducing the false tripping of tests in which a dead
> client can occur.
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> ---
>   lib/xe/xe_eudebug.c | 39 +++++++++++++++++++++++++++++++--------
>   lib/xe/xe_eudebug.h |  2 ++
>   2 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/xe/xe_eudebug.c b/lib/xe/xe_eudebug.c
> index d29c82857..0c3262c41 100644
> --- a/lib/xe/xe_eudebug.c
> +++ b/lib/xe/xe_eudebug.c
> @@ -296,34 +296,56 @@ static void pipe_close(int pipe[2])
>   		close(pipe[1]);
>   }
>   
> +#define DEAD_CLIENT 0xccccdead
> +
>   static uint64_t __wait_token(int pipe[2], const uint64_t token, int timeout_ms)
>   {
>   	uint64_t in;
>   	int ret;
>   
>   	ret = safe_pipe_read(pipe, &in, sizeof(in), timeout_ms);
> -	igt_assert_f(ret > 0,
> -		     "Pipe read timeout waiting for token '%s:(%" PRId64 ")'\n",
> -		     token_to_str(token), token);
> +	if (ret < 0) {
> +		igt_info("Pipe read failed with error: %d waiting for token '%s:(%" PRId64 ")'\n",
> +			 ret, token_to_str(token), token);

igt_info seems quite loud considering that this error may be expected 
and will get passed down to another library function to decide what to 
do with it.

Nit: I think "Pipe poll" would be better here, with "Pipe read" bellow. 
Or just use the function name in both cases that is 'safe_pipe_read'.

With that:
Reviewed-by: Christoph Manszewski <christoph.manszewski@intel.com>

Regards,
Christoph


> +		return DEAD_CLIENT;
> +	} else if (ret == 0) {
> +		igt_info("Pipe read failed: EOF\n");
> +		return DEAD_CLIENT;
> +	}
>   
>   	igt_assert_eq(in, token);
>   
>   	ret = safe_pipe_read(pipe, &in, sizeof(in), timeout_ms);
> -	igt_assert_f(ret > 0,
> -		     "Pipe read timeout waiting for token value '%s:(%" PRId64 ")'\n",
> -		     token_to_str(token), token);
> +	if (ret < 0) {
> +		igt_info("Pipe read failed with error: %d waiting for token '%s:(%" PRId64 ")'\n",
> +			 ret, token_to_str(token), token);
> +		return DEAD_CLIENT;
> +	} else if (ret == 0) {
> +		igt_info("Pipe read failed: EOF\n");
> +		return DEAD_CLIENT;
> +	}
>   
>   	return in;
>   }
>   
>   static uint64_t client_wait_token(struct xe_eudebug_client *c, const uint64_t token)
>   {
> -	return __wait_token(c->p_in, token, c->timeout_ms);
> +	uint64_t ret = __wait_token(c->p_in, token, c->timeout_ms);
> +
> +	if (ret == DEAD_CLIENT)
> +		igt_assert(c->allow_dead_client);
> +
> +	return ret;
>   }
>   
>   static uint64_t wait_from_client(struct xe_eudebug_client *c, const uint64_t token)
>   {
> -	return __wait_token(c->p_out, token, c->timeout_ms);
> +	uint64_t ret = __wait_token(c->p_out, token, c->timeout_ms);
> +
> +	if (ret == DEAD_CLIENT)
> +		igt_assert(c->allow_dead_client);
> +
> +	return ret;
>   }
>   
>   static void token_signal(int pipe[2], const uint64_t token, const uint64_t value)
> @@ -1402,6 +1424,7 @@ struct xe_eudebug_client *xe_eudebug_client_create(int master_fd, xe_eudebug_cli
>   	c->ptr = data;
>   	c->master_fd = master_fd;
>   	c->timeout_ms = XE_EUDEBUG_DEFAULT_TIMEOUT_SEC * MSEC_PER_SEC;
> +	c->allow_dead_client = false;
>   	pthread_mutex_init(&c->lock, NULL);
>   
>   	igt_fork(child, 1) {
> diff --git a/lib/xe/xe_eudebug.h b/lib/xe/xe_eudebug.h
> index 3adde5f6c..8f7f66220 100644
> --- a/lib/xe/xe_eudebug.h
> +++ b/lib/xe/xe_eudebug.h
> @@ -66,6 +66,8 @@ struct xe_eudebug_client {
>   
>   	int timeout_ms;
>   
> +	bool allow_dead_client;
> +
>   	pthread_mutex_t lock;
>   };
>