[Intel-gfx] [igt-dev] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit

[Petri Latvala]

On Wed, Feb 12, 2020 at 05:28:40PM -0800, Dale B Stimson wrote:
> At the start of igt_main, failure of the initial tests for successful
> initialization transfer control to the end of an igt_fixture block.
> From there, execution of the main per-engine loop is attempted.
> Instead, the test should be caused to exit.
> 
> If initialization fails, exit.
> 
> Signed-off-by: Dale B Stimson <dale.b.stimson at intel.com>
> ---
>  tests/i915/gem_ctx_isolation.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 07ffbb84a..b11158dab 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -898,10 +898,13 @@ igt_main
>  	int fd = -1;
>  	struct eng_mmio_base_table_s *mbp = NULL;
>  	uint32_t mmio_base = 0;
> +	/* igt_fixture block is skipped if --list-subtests, so start with true. */
> +	bool init_successful = true;
>  
>  	igt_fixture {
>  		int gen;
>  
> +		init_successful = false;
>  		fd = drm_open_driver(DRIVER_INTEL);
>  		igt_require_gem(fd);
>  		igt_require(gem_has_contexts(fd));
> @@ -916,8 +919,20 @@ igt_main
>  		igt_skip_on(gen > LAST_KNOWN_GEN);
>  
>  		mbp = gem_engine_mmio_base_info_get(fd);
> +		init_successful = true;
>  	}
>  
> +	if (!init_successful) {
> +		igt_exit_early();
> +	}
> +

NAK. All this dancing around the infrastructure just makes changing
the infrastructure later be awkward and produce weird errors.

If something in the fixture failed, with this code you never enter the
subtest, making the test result 'notrun' instead of the correct 'skip'
or 'fail'.

What is the problem this is trying to solve? Incorrect engine list
used? If you have a subtest per static engine, all CI does is execute
per static engine. Converting this test to use dynamic subtests for
engines is the way forward.


-- 
Petri Latvala