[PATCH i-g-t] tests/intel/gem_eio: Fix SIGABRT in kms subtest

[Krzysztof Niemiec]

Hi Kamil,

On 2025-06-05 at 20:48:01 GMT, Kamil Konieczny wrote:
> Subtest kms runs display helper in a separate process, which in
> turn prepares patterns for screen and there makes additional
> checks resulting in skip. This leads to a SIGABRT as igt_core
> require all such skips in main process, not in child.
> 
> Add this check before starting a kms subtest, also move other
> skip checks into one place.
> 
> Cc: Krzysztof Niemiec <krzysztof.niemiec at intel.com>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13658
> Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> ---
>  tests/intel/gem_eio.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/intel/gem_eio.c b/tests/intel/gem_eio.c
> index b65b914fa..d6384efe2 100644
> --- a/tests/intel/gem_eio.c
> +++ b/tests/intel/gem_eio.c
> @@ -1188,14 +1188,17 @@ igt_main
>  
>  		igt_fixture {
>  			igt_device_set_master(fd);
> +			intel_allocator_multiprocess_start();
> +		}
>  
> +		igt_subtest("kms") {
>  			igt_display_require(&display, fd);
>  			igt_display_require_output(&display);
> -			intel_allocator_multiprocess_start();
> -		}
> +			if (!gem_has_relocations(fd))
> +				igt_require(gem_has_contexts(fd)); /* for display_helper() */

This won't fix the bug. The bug isn't contingent on the conditions not
being met - igt_skip() checks if the call is in the main thread before
checking anything else:

<code snippet>

void __igt_skip_check(const char *file, const int line,
		      const char *func, const char *check,
		      const char *f, ...)
{
	va_list args;
	int err = errno;
	char *err_str = NULL;

	if (!igt_thread_is_main())
		assert(!"igt_require/skip allowed only in the main thread!");

</code snippet>

The change that's proposed here just duplicates the check outside the
test itself, but the check in display_helper() is retained.

Instead we need to make sure that we *never* call igt_require() inside the
thread, because even igt_require(true) would fail in a thread. In this
case it would probably mean rewriting setup_context_and_memory_region()
to be igt_require()-less, but we also need to check what other tests
rely on its internal require() for their requirements. But I assume
it's not many as most tests seem to respect that checking requirements is
the first thing that the test should do before it does anything else.

Thanks
Krzysztof

>  
> -		igt_subtest("kms")
>  			test_kms(fd, &display);
> +		}
>  
>  		igt_fixture {
>  			intel_allocator_multiprocess_stop();
> -- 
> 2.49.0
>