public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Petri Latvala <petri.latvala@intel.com>
To: Dale B Stimson <dale.b.stimson@intel.com>
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit
Date: Thu, 13 Feb 2020 10:29:55 +0200	[thread overview]
Message-ID: <20200213082955.GT25209@platvala-desk.ger.corp.intel.com> (raw)
In-Reply-To: <20200213012840.31472-6-dale.b.stimson@intel.com>

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@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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2020-02-13  8:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-13  1:28 [igt-dev] [PATCH i-g-t v2 0/5] mmio_base via debugfs infrastructure + gem_ctx_isolation Dale B Stimson
2020-02-13  1:28 ` [igt-dev] [PATCH i-g-t v2 1/5] i915/gem_mmio_base.c - get mmio_base from debugfs (if possible) Dale B Stimson
2020-02-13  1:28 ` [igt-dev] [PATCH i-g-t v2 2/5] i915/gem_ctx_isolation: Check engine relative registers Dale B Stimson
2020-02-13  1:28 ` [igt-dev] [PATCH i-g-t v2 3/5] i915/gem_ctx_isolation: Check engine relative registers - Part 2 Dale B Stimson
2020-02-13  1:28 ` [Intel-gfx] [PATCH i-g-t v2 4/5] lib/igt_core.c - Introduce igt_exit_early() Dale B Stimson
2020-02-13  1:28 ` [igt-dev] [PATCH i-g-t v2 5/5] i915/gem_ctx_isolation.c - If initialization fails, exit Dale B Stimson
2020-02-13  8:29   ` Petri Latvala [this message]
2020-02-13 19:29     ` Dale B Stimson
2020-02-14 11:12       ` Petri Latvala
2020-02-13  2:12 ` [igt-dev] ✓ Fi.CI.BAT: success for mmio_base via debugfs infrastructure + gem_ctx_isolation Patchwork
2020-02-16  5:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200213082955.GT25209@platvala-desk.ger.corp.intel.com \
    --to=petri.latvala@intel.com \
    --cc=dale.b.stimson@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox