[igt-dev] [PATCH i-g-t 60/81] tests/i915/gem_ctx_create: Don't re-open the device in maximum()

[Zbigniew Kempczyński]

On Wed, Jul 07, 2021 at 09:46:22AM -0500, Jason Ekstrand wrote:
> In 00a64098aaae ("tests/i915/gem_ctx_create: Use
> __for_each_physical_engine to utilize all available engines"), the
> maximum tests were modified to re-open the device.  However, this is
> both unnecessary and broken because the execution loop re-uses the
> contexts created above it which do not exist on the newly re-opened DRM
> file.
> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> Fixes: 00a64098aaae ("tests/i915/gem_ctx_create: Use __for_each_physical_engine to utilize all available engines")
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
>  tests/i915/gem_ctx_create.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/i915/gem_ctx_create.c b/tests/i915/gem_ctx_create.c
> index 5b14f7afd..37061ebc5 100644
> --- a/tests/i915/gem_ctx_create.c
> +++ b/tests/i915/gem_ctx_create.c
> @@ -323,18 +323,9 @@ static void maximum(int fd, int ncpus, unsigned mode)
>  
>  	igt_fork(child, ncpus) {
>  		struct timespec start, end;
> -		int i915;
> -
> -		i915 = gem_reopen_driver(fd);
> -		/*
> -		 * Ensure the gpu is idle by launching
> -		 * a nop execbuf and stalling for it.
> -		 */
> -		gem_quiescent_gpu(i915);
> -		gem_context_copy_engines(fd, 0, i915, 0);
>  
>  		hars_petruska_f54_1_random_perturb(child);
> -		obj[0].handle = gem_create(i915, 4096);
> +		obj[0].handle = gem_create(fd, 4096);
>  
>  		clock_gettime(CLOCK_MONOTONIC, &start);
>  		for (int repeat = 0; repeat < 3; repeat++) {
> @@ -345,13 +336,13 @@ static void maximum(int fd, int ncpus, unsigned mode)
>  				execbuf.rsvd1 = contexts[i];
>  				for (unsigned long j = 0; j < all_nengine; j++) {
>  					execbuf.flags = all_engines[j];
> -					gem_execbuf(i915, &execbuf);
> +					gem_execbuf(fd, &execbuf);
>  				}
>  			}
>  		}
> -		gem_sync(i915, obj[0].handle);
> +		gem_sync(fd, obj[0].handle);
>  		clock_gettime(CLOCK_MONOTONIC, &end);
> -		gem_close(i915, obj[0].handle);
> +		gem_close(fd, obj[0].handle);
>  
>  		igt_info("[%d] Context execution: %.3f us\n", child,
>  			 elapsed(&start, &end) / (3 * count * all_nengine) * 1e6);
> -- 
> 2.31.1
> 

+Tomi

Looks ok. Only thing which is worrying me is your fix will start test running 
on pre-merge so it would increase CI time (especially on machines with a lot
of mem (+swap). But lets see if this will be really noticeable.

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

--
Zbigniew