[Intel-gfx] [PATCH] drm/i915/selftests: Always free spinner on __sseu_prepare error

[Sripada, Radhakrishna]

Hi Chris,

Looks clean.

Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>

-Radhakrishna(RK) Sripada
> -----Original Message-----
> From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> Sent: Friday, February 15, 2019 11:50 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: Chris Wilson <chris at chris-wilson.co.uk>; Sripada, Radhakrishna
> <radhakrishna.sripada at intel.com>; Ursulin, Tvrtko
> <tvrtko.ursulin at intel.com>; Joonas Lahtinen
> <joonas.lahtinen at linux.intel.com>
> Subject: [PATCH] drm/i915/selftests: Always free spinner on
> __sseu_prepare error
> 
> Prepare a nice little onion unwind to ensure that we always free the spinner
> if we __sseu_prepare fails.
> 
> Fixes: c06ee6ff2cbc ("drm/i915/selftests: Context SSEU reconfiguration
> tests")
> Reported-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  .../gpu/drm/i915/selftests/i915_gem_context.c | 69 +++++++++----------
>  1 file changed, 34 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index d00d0bb07784..7eb58a9d1319 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -710,47 +710,45 @@ __sseu_prepare(struct drm_i915_private *i915,
>  	       unsigned int flags,
>  	       struct i915_gem_context *ctx,
>  	       struct intel_engine_cs *engine,
> -	       struct igt_spinner **spin_out)
> +	       struct igt_spinner **spin)
>  {
> -	int ret = 0;
> -
> -	if (flags & (TEST_BUSY | TEST_RESET)) {
> -		struct igt_spinner *spin;
> -		struct i915_request *rq;
> +	struct i915_request *rq;
> +	int ret;
> 
> -		spin = kzalloc(sizeof(*spin), GFP_KERNEL);
> -		if (!spin) {
> -			ret = -ENOMEM;
> -			goto out;
> -		}
> +	*spin = NULL;
> +	if (!(flags & (TEST_BUSY | TEST_RESET)))
> +		return 0;
> 
> -		ret = igt_spinner_init(spin, i915);
> -		if (ret)
> -			return ret;
> +	*spin = kzalloc(sizeof(**spin), GFP_KERNEL);
> +	if (!*spin)
> +		return -ENOMEM;
> 
> -		rq = igt_spinner_create_request(spin, ctx, engine,
> MI_NOOP);
> -		if (IS_ERR(rq)) {
> -			ret = PTR_ERR(rq);
> -			igt_spinner_fini(spin);
> -			kfree(spin);
> -			goto out;
> -		}
> +	ret = igt_spinner_init(*spin, i915);
> +	if (ret)
> +		goto err_free;
> 
> -		i915_request_add(rq);
> +	rq = igt_spinner_create_request(*spin, ctx, engine, MI_NOOP);
> +	if (IS_ERR(rq)) {
> +		ret = PTR_ERR(rq);
> +		goto err_fini;
> +	}
> 
> -		if (!igt_wait_for_spinner(spin, rq)) {
> -			pr_err("%s: Spinner failed to start!\n", name);
> -			igt_spinner_end(spin);
> -			igt_spinner_fini(spin);
> -			kfree(spin);
> -			ret = -ETIMEDOUT;
> -			goto out;
> -		}
> +	i915_request_add(rq);
> 
> -		*spin_out = spin;
> +	if (!igt_wait_for_spinner(*spin, rq)) {
> +		pr_err("%s: Spinner failed to start!\n", name);
> +		ret = -ETIMEDOUT;
> +		goto err_end;
>  	}
> 
> -out:
> +	return 0;
> +
> +err_end:
> +	igt_spinner_end(*spin);
> +err_fini:
> +	igt_spinner_fini(*spin);
> +err_free:
> +	kfree(fetch_and_zero(spin));
>  	return ret;
>  }
> 
> @@ -897,22 +895,23 @@ __sseu_test(struct drm_i915_private *i915,
> 
>  	ret = __sseu_prepare(i915, name, flags, ctx, engine, &spin);
>  	if (ret)
> -		goto out;
> +		goto out_context;
> 
>  	ret = __i915_gem_context_reconfigure_sseu(ctx, engine, sseu);
>  	if (ret)
> -		goto out;
> +		goto out_spin;
> 
>  	ret = __sseu_finish(i915, name, flags, ctx, kctx, engine, obj,
>  			    hweight32(sseu.slice_mask), spin);
> 
> -out:
> +out_spin:
>  	if (spin) {
>  		igt_spinner_end(spin);
>  		igt_spinner_fini(spin);
>  		kfree(spin);
>  	}
> 
> +out_context:
>  	kernel_context_close(kctx);
> 
>  	return ret;
> --
> 2.20.1