[igt-dev] [PATCH i-g-t 2/2] tests/i915/gem_spin_batch: Removing context persistence

[Newsome, Jasmine]

On 19/02/2022 01:32, Jasmine Newsome wrote:
> The spin all test relied on context persistence unecessarily by trying 
> to destroy contexts while keeping spinners active.
> The current implementation of context persistence in i915 can cause 
> failures, and persistence is not needed for this test.

Could you please expand a bit on "current implementation" and "can cause failures"?

Also from the subject of "Removing context persistence" I was expecting to see usage of I915_CONTEXT_PARAM_PERSISTENCE to actually change the mode.

My concern is that the pattern of destroying contexts while keeping things active on the GPU is very wide spread in IGT and possibly exists in userspace as well.

Has the wider story been analysed by the architects here and what is the plan? Do we actually know no userspace actually depends on it?

[Comes back later, after spotting the cover letter.]

So it's only GuC, not i915, so please say that in this commit message since cover letters are not saved in git history.

Regards,

Tvrtko

----------------------------------------------

Ok thanks. I will update the commit message and resend. 

Regards,
Jasmine

> 
> Moving intel_ctx_destroy after igt_spin_end.
> 
> Signed-off-by: Jasmine Newsome <jasmine.newsome at intel.com>
> ---
>   tests/i915/gem_spin_batch.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/i915/gem_spin_batch.c b/tests/i915/gem_spin_batch.c 
> index 653812c7..707d69b6 100644
> --- a/tests/i915/gem_spin_batch.c
> +++ b/tests/i915/gem_spin_batch.c
> @@ -161,8 +161,6 @@ static void spin_all(int i915, const intel_ctx_t *ctx, unsigned int flags)
>   				    .engine = e->flags,
>   				    .flags = (IGT_SPIN_POLL_RUN |
>   					      IGT_SPIN_NO_PREEMPTION));
> -		if (flags & PARALLEL_SPIN_NEW_CTX)
> -			intel_ctx_destroy(i915, ctx);
>   
>   		igt_spin_busywait_until_started(spin);
>   		igt_list_move(&spin->link, &list); @@ -172,6 +170,8 @@ static void 
> spin_all(int i915, const intel_ctx_t *ctx, unsigned int flags)
>   		igt_assert(gem_bo_busy(i915, spin->handle));
>   		ahnd = spin->ahnd;
>   		igt_spin_end(spin);
> +		if (flags & PARALLEL_SPIN_NEW_CTX)
> +			intel_ctx_destroy(i915, spin->opts.ctx);
>   		gem_sync(i915, spin->handle);
>   		igt_spin_free(i915, spin);
>   		put_ahnd(ahnd);