Re: [PATCH] tests/gem_userptr_blits: Race between close and invalidate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 03, 2015 at 03:39:17PM +0100, Michał Winiarski wrote:
> It was possible for invalidate range start mmu notifier callback to race
> with releasing userptr object. If the object is released prior to
> taking a spinlock in the callback, we'll encounter a null pointer
> dereference.
> 
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> ---
>  tests/gem_userptr_blits.c | 68 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
> index be2fdf9..5864e4f 100644
> --- a/tests/gem_userptr_blits.c
> +++ b/tests/gem_userptr_blits.c
> @@ -1179,6 +1179,8 @@ static void test_unmap_cycles(int fd, int expected)
>  		test_unmap(fd, expected);
>  }
>  
> +#define MM_STRESS_LOOPS 100000
> +
>  struct stress_thread_data {
>  	unsigned int stop;
>  	int exit_code;
> @@ -1211,7 +1213,7 @@ static void test_stress_mm(int fd)
>  {
>  	int ret;
>  	pthread_t t;
> -	unsigned int loops = 100000;
> +	unsigned int loops = MM_STRESS_LOOPS;
>  	uint32_t handle;
>  	void *ptr;
>  	struct stress_thread_data stdata;
> @@ -1239,6 +1241,62 @@ static void test_stress_mm(int fd)
>  	igt_assert(stdata.exit_code == 0);
>  }
>  
> +struct userptr_close_thread_data {
> +	int fd;
> +	void *ptr;
> +	bool overlap;
> +	bool stop;
> +};
> +
> +static void *mm_userptr_close_thread(void *data)
> +{
> +	int ret;
> +	struct userptr_close_thread_data *t_data = (struct userptr_close_thread_data *)data;
> +	int fd = t_data->fd;
> +	void *ptr = t_data->ptr;
> +	int handle_num = t_data->overlap ? 2 : 1;
> +
> +	uint32_t handle[handle_num];
> +
> +	while (!t_data->stop) {
> +		for (int i = 0; i < handle_num; i++)
> +			ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle[i]);
> +			igt_assert(ret == 0);

Whoops. Let's just assert that igt_assert() can't be compiled out (that
would make a mockery of igt for starters) and allow us to use
expressions with side effects inside igt_assert().

static void userptr_close_thread(void *data)
{
	strct userptr_close_thread *t = data;
	const int nhandles = t->overlap ? 2 : 1;
	uint32_t handle[nhandles];

	/* Be pedantic and enforce the required memory barriers */
	pthread_mutex_lock(&t->mutex);
	while (!t->stop) {
		pthread_mutex_unlock(&t->mutex);

		for (int i = 0; i < nhandles; i++)
			igt_assert(gem_userptr(t->fd, t->ptr, PAGE_SIZE, 0, &handle[i]) == 0);
		for (int i = 0; i < nhandles; i++)
			gem_close(t->fd, handle[i]);

		pthread_mutex_lock(&t->mutex);
	}
	pthread_mutex_unlock(&t->mutex);

	return NULL;
}

Nice test!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux