Re: [PATCH] drm/i915/selftests: Reorder error cleanup for whitelist checking

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Reorder the error paths so that we unwind all the locals from any error
> path and so avoid setting off divers alarum in case we find an error in
> case we find an error.

Divers alarum got past me. But for finding an error in case
we find error: It is fitting enough that I am not sure if it is
a mistake.

The error paths are better,
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=111048
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  .../gpu/drm/i915/gt/selftest_workarounds.c    | 35 ++++++++++---------
>  1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index b933d831eeb1..fa01ea7855de 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -287,7 +287,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
>  					const char *name)
>  {
>  	struct drm_i915_private *i915 = engine->i915;
> -	struct i915_gem_context *ctx;
> +	struct i915_gem_context *ctx, *tmp;
>  	struct igt_spinner spin;
>  	intel_wakeref_t wakeref;
>  	int err;
> @@ -295,56 +295,59 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
>  	pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n",
>  		engine->whitelist.count, name);
>  
> -	err = igt_spinner_init(&spin, i915);
> -	if (err)
> -		return err;
> -
>  	ctx = kernel_context(i915);
>  	if (IS_ERR(ctx))
>  		return PTR_ERR(ctx);
>  
> +	err = igt_spinner_init(&spin, i915);
> +	if (err)
> +		goto out_ctx;
> +
>  	err = check_whitelist(ctx, engine);
>  	if (err) {
>  		pr_err("Invalid whitelist *before* %s reset!\n", name);
> -		goto out;
> +		goto out_spin;
>  	}
>  
>  	err = switch_to_scratch_context(engine, &spin);
>  	if (err)
> -		goto out;
> +		goto out_spin;
>  
>  	with_intel_runtime_pm(&i915->runtime_pm, wakeref)
>  		err = reset(engine);
>  
>  	igt_spinner_end(&spin);
> -	igt_spinner_fini(&spin);
>  
>  	if (err) {
>  		pr_err("%s reset failed\n", name);
> -		goto out;
> +		goto out_spin;
>  	}
>  
>  	err = check_whitelist(ctx, engine);
>  	if (err) {
>  		pr_err("Whitelist not preserved in context across %s reset!\n",
>  		       name);
> -		goto out;
> +		goto out_spin;
>  	}
>  
> +	tmp = kernel_context(i915);
> +	if (IS_ERR(tmp)) {
> +		err = PTR_ERR(tmp);
> +		goto out_spin;
> +	}
>  	kernel_context_close(ctx);
> -
> -	ctx = kernel_context(i915);
> -	if (IS_ERR(ctx))
> -		return PTR_ERR(ctx);
> +	ctx = tmp;
>  
>  	err = check_whitelist(ctx, engine);
>  	if (err) {
>  		pr_err("Invalid whitelist *after* %s reset in fresh context!\n",
>  		       name);
> -		goto out;
> +		goto out_spin;
>  	}
>  
> -out:
> +out_spin:
> +	igt_spinner_fini(&spin);
> +out_ctx:
>  	kernel_context_close(ctx);
>  	return err;
>  }
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx