[igt-dev] [PATCH i-g-t 1/3] tests/i915/api_intel_allocator: Exercise allocator in multiprocess mode

[Zbigniew Kempczyński]

On Wed, Jun 09, 2021 at 03:15:20PM +0200, Andrzej Turko wrote:
> Test creation and usage of allocators in multiprocess mode
> for contexts shared with the parent process as well as
> private for the child.
> 
> This new test discovers a bug in the allocator implementation:
> allocator is not correctly initialized if it is opened by
> a child process using its private file descriptor.
> 
> Signed-off-by: Andrzej Turko <andrzej.turko at linux.intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
>  tests/i915/api_intel_allocator.c | 47 ++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tests/i915/api_intel_allocator.c b/tests/i915/api_intel_allocator.c
> index ea4ba8bb1..b43467a33 100644
> --- a/tests/i915/api_intel_allocator.c
> +++ b/tests/i915/api_intel_allocator.c
> @@ -621,6 +621,50 @@ static void execbuf_with_allocator(int fd)
>  	igt_assert(intel_allocator_close(ahnd) == true);
>  }
>  
> +static void multiprocess(int fd, uint8_t type) {

Call this test fork-reopen. It will suggest what test is really doing.

> +	uint64_t p_ahnd, sh_ahnd, fd_ahnd, ctx_ahnd;
> +	uint64_t sh_left, sh_right, fd_left, fd_right;
> +	uint64_t offset;
> +
> +	intel_allocator_multiprocess_start();
> +
> +	p_ahnd = intel_allocator_open(fd, 0, type);
> +	offset = intel_allocator_alloc(p_ahnd, 1, 123, 0);
> +	if (type == INTEL_ALLOCATOR_SIMPLE)
> +		igt_assert(intel_allocator_is_allocated(p_ahnd, 1, 123, offset));
> +
> +	igt_fork(child, 1) {
> +

Unnecessary empty line.

> +		sh_ahnd = intel_allocator_open(fd, 0, type);
> +		if (type == INTEL_ALLOCATOR_SIMPLE)
> +			igt_assert(intel_allocator_is_allocated(sh_ahnd, 1, 123, offset));
> +
> +		ctx_ahnd = intel_allocator_open(fd, 1, type);
> +		igt_assert(!intel_allocator_is_allocated(ctx_ahnd, 1, 123, offset));
> +		intel_allocator_alloc(ctx_ahnd, 2, 123, 0);
> +
> +		fd = gem_reopen_driver(fd);
> +		fd_ahnd = intel_allocator_open(fd, 0, type);
> +		igt_assert(!intel_allocator_is_allocated(fd_ahnd, 1, 123, offset));
> +		intel_allocator_alloc(fd_ahnd, 2, 123, 0);
> +
> +

Same.

> +		intel_allocator_get_address_range(sh_ahnd, &sh_left, &sh_right);
> +		intel_allocator_get_address_range(fd_ahnd, &fd_left, &fd_right);
> +		igt_assert(sh_left == fd_left && sh_right == fd_right);

On the beginning I wondered why we just haven't crashed on allocator thread
but quick dig shows gem_aperture_size() allows running on not opened i915 fd.

We definitely want to blow allocator thread if underlying fd is not valid opened
i915. So series should be prepended with patch which checks on intel_allocator_open
do fd is valid or not. For not valid just asserts and see how it explodes.

In this case above 3 lines won't be necessary, we got an assert on open.

--
Zbigniew

> +
> +		intel_allocator_close(sh_ahnd);
> +		intel_allocator_close(ctx_ahnd);
> +		intel_allocator_close(fd_ahnd);
> +
> +	}
> +
> +	igt_waitchildren();
> +	intel_allocator_close(p_ahnd);
> +
> +	intel_allocator_multiprocess_stop();
> +}
> +
>  struct allocators {
>  	const char *name;
>  	uint8_t type;
> @@ -672,6 +716,9 @@ igt_main
>  				igt_dynamic("reserve")
>  					reserve(fd, a->type);
>  			}
> +
> +			igt_dynamic("multiprocess")
> +					multiprocess(fd, a->type);
>  		}
>  	}
>  
> -- 
> 2.25.1
>