Re: [PATCH 2/3] igt/gem_userptr_blits: Shared memory allocations

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 07/11/2014 10:40 AM, Chris Wilson wrote:
> The forked tests allocate the bo (and thus for userptr, the memory) in
> the parent and pass them to all children. The difference for userptr is
> that we allocate system memory which the kernel then copies into each
> child. As the children need to access the memory for their checks, it
> does need to be shared - so allocate the userptr from shared memory!
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80208
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/gem_userptr_blits.c | 39 ++++++++++++++++++++++++++++++---------
>   1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
> index 14efdda..9cf681b 100644
> --- a/tests/gem_userptr_blits.c
> +++ b/tests/gem_userptr_blits.c
> @@ -321,17 +321,32 @@ create_userptr(int fd, uint32_t val, uint32_t *ptr)
>   }
>
>   static void **handle_ptr_map;
> -static unsigned int num_handle_ptr_map;
> +static int *handle_size_map;
> +static unsigned int num_handle_map;
>
> -static void add_handle_ptr(uint32_t handle, void *ptr)
> +static void reset_handle_ptr(void)
>   {
> -	if (handle >= num_handle_ptr_map) {
> +	free(handle_ptr_map);
> +	free(handle_size_map);
> +	num_handle_map = 0;
> +}
> +
> +static void add_handle_ptr(uint32_t handle, void *ptr, int size)
> +{
> +	if (handle >= num_handle_map) {
>   		handle_ptr_map = realloc(handle_ptr_map,
>   					 (handle + 1000) * sizeof(void*));
> -		num_handle_ptr_map = handle + 1000;
> +		igt_assert(handle_ptr_map);
> +
> +		handle_size_map = realloc(handle_size_map,
> +					 (handle + 1000) * sizeof(int));
> +		igt_assert(handle_size_map);
> +
> +		num_handle_map = handle + 1000;
>   	}
>
>   	handle_ptr_map[handle] = ptr;
> +	handle_size_map[handle] = size;
>   }
>
>   static void *get_handle_ptr(uint32_t handle)
> @@ -341,10 +356,10 @@ static void *get_handle_ptr(uint32_t handle)
>
>   static void free_handle_ptr(uint32_t handle)
>   {
> -	igt_assert(handle < num_handle_ptr_map);
> +	igt_assert(handle < num_handle_map);
>   	igt_assert(handle_ptr_map[handle]);
>
> -	free(handle_ptr_map[handle]);
> +	munmap(handle_ptr_map[handle], handle_size_map[handle]);
>   	handle_ptr_map[handle] = NULL;
>   }
>
> @@ -354,12 +369,15 @@ static uint32_t create_userptr_bo(int fd, int size)
>   	uint32_t handle;
>   	int ret;
>
> -	ret = posix_memalign(&ptr, PAGE_SIZE, size);
> -	igt_assert(ret == 0);
> +	ptr = mmap(NULL, size,
> +		   PROT_READ | PROT_WRITE,
> +		   MAP_ANONYMOUS | MAP_SHARED,
> +		   -1, 0);
> +	igt_assert(ptr != MAP_FAILED);

I wasn't sure about this straight away, but man page says "...on Linux, 
the mapping will be created at a nearby page boundary..." so it's fine.

>   	ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, &handle);
>   	igt_assert(ret == 0);
> -	add_handle_ptr(handle, ptr);
> +	add_handle_ptr(handle, ptr, size);
>
>   	return handle;
>   }
> @@ -641,6 +659,7 @@ static void sigbus(int sig, siginfo_t *info, void *param)
>   				MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
>   		if ((unsigned long)addr == ptr) {
>   			memset(addr, counter, sizeof(linear));
> +			munmap(addr, sizeof(linear));
>   			return;
>   		}
>   	}
> @@ -708,6 +727,8 @@ static int test_dmabuf(void)
>   	close(fd1);
>   	close(fd2);
>
> +	reset_handle_ptr();
> +
>   	return 0;
>   }
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>