Re: [PATCH i-g-t 2/3] tests/xe_eudebug: Detect thread starvation on discovery

["Grzegorzek, Dominik" <dominik.grzegorzek@intel.com>]

On Thu, 2024-09-26 at 13:39 +0300, Mika Kuoppala wrote:
> Discovery tests spawns multiple clients and multiple
> debuggers and sometimes, especially with kernel debugs on,
> some thread really can starve more than one second.
> 
> Add a check that we dont start to assert missing
> resources if we didn't get any.
> 
> Another option would be to just skip starved thread
> checks and assert that some of the threads succeeded.
> But lets try this as a stopgap measure before giving
> up.
> 
> Increase worker time until we see an event.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  tests/intel/xe_eudebug.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/intel/xe_eudebug.c b/tests/intel/xe_eudebug.c
> index a2b2f3596..bf560462e 100644
> --- a/tests/intel/xe_eudebug.c
> +++ b/tests/intel/xe_eudebug.c
> @@ -1107,11 +1107,24 @@ static void *discovery_race_thread(void *data)
>  
>  		if (random() % 2) {
>  			struct drm_xe_eudebug_event *e = NULL;
> +			int max_worker_waits = 30;
>  			int i = -1;
>  
>  			xe_eudebug_debugger_start_worker(s->debugger);
> -			sleep(1);
> +
> +			/*
> +			 * Thread can starve for more than one second. Make
> +			 * sure we get atleast one event before stopping.
> +			 */
> +			do
> +				sleep(1);
> +			while (!READ_ONCE(s->debugger->event_count) &&
> +			       --max_worker_waits);
> +
> +			igt_assert(READ_ONCE(s->debugger->event_count));
> +
>  			xe_eudebug_debugger_stop_worker(s->debugger, 1);
> +

Yep, sleep(1) had some risk to be not enough. Your approach is more elegant.
Thanks for the fix!
 
Reviewed-by: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
>  			igt_debug("Resources discovered: %lu\n", s->debugger->event_count);
>  
>  			xe_eudebug_for_each_event(e, s->debugger->log) {