[Intel-gfx] [PATCH] tests/gem_userptr_blits: subtests for MAP_FIXED mappings of regular bo

[Chris Wilson]

On Mon, Jun 29, 2015 at 03:15:12PM +0100, Tvrtko Ursulin wrote:
> 
> On 06/29/2015 03:07 PM, Chris Wilson wrote:
> >On Mon, Jun 29, 2015 at 03:01:12PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 06/29/2015 11:59 AM, Michał Winiarski wrote:
> >>>When the the memory backing the userptr object is freed by the user, it's
> >>>possible to trigger recursive deadlock caused by operations done on
> >>>different BO mapped in that region, triggering invalidate.
> >>>
> >>>Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> >>>---
> >>>  tests/gem_userptr_blits.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 83 insertions(+)
> >>>
> >>>diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
> >>>index 1f2cc96..3fe8f90 100644
> >>>--- a/tests/gem_userptr_blits.c
> >>>+++ b/tests/gem_userptr_blits.c
> >>>@@ -640,6 +640,80 @@ static void test_forked_access(int fd)
> >>>  	free(ptr2);
> >>>  }
> >>>
> >>>+static int test_map_fixed_invalidate(int fd, bool overlap)
> >>>+{
> >>>+	void *ptr;
> >>>+	void *map;
> >>>+	int i;
> >>>+	int num_handles = overlap ? 2 : 1;
> >>>+	uint32_t handle[num_handles];
> >>>+	uint32_t mmap_handle;
> >>>+	struct drm_i915_gem_mmap_gtt mmap_arg;
> >>>+
> >>>+	igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
> >>>+	for (i=0; i<num_handles; i++)
> >>>+		igt_assert(gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle[i]) == 0);
> >>>+	free(ptr);
> >>
> >>I am not sure we can rely on free triggering munmap(2) here, I think
> >>this is just glibc implementation detail. So I would suggest
> >>allocating with mmap and freeing with munmap.
> >
> >The MAP_FIXED itself should be sufficient to invalidate any prior vma at
> >that address, so we don't depend upon the free() here to zap
> 
> Yeah, but does the free trigger an invalidate, or does it not? Or in
> other words, is the one from MAP_FIXED the first one, or not? Better
> to be explicit than undefined in the test case, that was my point.

Yes. But I would prefer MAP_FIXED to be the first invalidate, but that
looks like we then need to leak the ptr.
 
> >userptr->pages. In this instance userptr->pages doesn't get populated
> >before the MAP_FIXED anyway.
> 
> Hmmm... did not even think about that. Why does MAP_FIXED trigger get_pages?

MAP_FIXED doesn't. Part of the tests is to make sure that MAP_FIXED
places a GTT object over top of the userptr is that the userptr then
reports EFAULT when used. My worrying about obj->pages prior to the
invalidation is that there are few paths through cancel_userptr() that
depend upon whether the object has been used (i.e. has pages) or whether
the object is active on the GPU. That's the purpose of doing a copy or
set-domain or nothing prior to the MAP_FIXED.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre