[Intel-gfx] [PATCH igt 1/5] igt/gem_eio: inflight wedged requires long plugging

[Michał Winiarski]

On Mon, Sep 11, 2017 at 09:55:58AM +0100, Chris Wilson wrote:
> As our hangcheck may exceed 10s to declare the device wedged, we need to
> hold the plugging fence indefinitely. This makes using vgem as our input
> fence unusable, so resort to using sw_sync. At the same time, we can
> then check that the async result is also -EIO.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102616
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  tests/gem_eio.c | 79 ++++++++++++++++++++++-----------------------------------
>  1 file changed, 31 insertions(+), 48 deletions(-)
> 
> diff --git a/tests/gem_eio.c b/tests/gem_eio.c
> index 15120842..249510e7 100644
> --- a/tests/gem_eio.c
> +++ b/tests/gem_eio.c
> @@ -39,7 +39,7 @@
>  
>  #include <drm.h>
>  
> -#include "igt_vgem.h"
> +#include "sw_sync.h"
>  
>  IGT_TEST_DESCRIPTION("Test that specific ioctls report a wedged GPU (EIO).");
>  
> @@ -158,66 +158,49 @@ static void test_wait(int fd)
>  	trigger_reset(fd);
>  }
>  
> -struct cork {
> -	int device;
> -	uint32_t handle;
> -	uint32_t fence;
> -};
> -
> -static void plug(int fd, struct cork *c)
> -{
> -	struct vgem_bo bo;
> -	int dmabuf;
> -
> -	c->device = __drm_open_driver(DRIVER_VGEM);
> -	igt_require(c->device != -1);
> -
> -	bo.width = bo.height = 1;
> -	bo.bpp = 4;
> -	vgem_create(c->device, &bo);
> -	c->fence = vgem_fence_attach(c->device, &bo, VGEM_FENCE_WRITE);
> -
> -	dmabuf = prime_handle_to_fd(c->device, bo.handle);
> -	c->handle = prime_fd_to_handle(fd, dmabuf);
> -	close(dmabuf);
> -}
> -
> -static void unplug(struct cork *c)
> -{
> -	vgem_fence_signal(c->device, c->fence);
> -	close(c->device);
> -}
> -
> -static void test_inflight(int fd)
> +static void test_inflight_external(int fd)
>  {
>  	struct drm_i915_gem_execbuffer2 execbuf;
> -	struct drm_i915_gem_exec_object2 obj[2];
> +	struct drm_i915_gem_exec_object2 obj;
>  	uint32_t bbe = MI_BATCH_BUFFER_END;
>  	igt_hang_t hang;
> -	struct cork cork;
> +	int timeline, fence;
> +
> +	igt_require_sw_sync();
> +	igt_require(gem_has_exec_fence(fd));
> +
> +	timeline = sw_sync_timeline_create();
> +	fence = sw_sync_timeline_create_fence(timeline, 1);
>  
>  	igt_require(i915_reset_control(false));
>  	hang = igt_hang_ring(fd, I915_EXEC_DEFAULT);
>  
> -	plug(fd, &cork);
> -
> -	memset(obj, 0, sizeof(obj));
> -	obj[0].handle = cork.handle;
> -	obj[1].handle = gem_create(fd, 4096);
> -	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
> +	memset(&obj, 0, sizeof(obj));
> +	obj.handle = gem_create(fd, 4096);
> +	gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
>  
>  	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(obj);
> -	execbuf.buffer_count = 2;
> +	execbuf.buffers_ptr = to_user_pointer(&obj);
> +	execbuf.buffer_count = 1;
> +	execbuf.flags = I915_EXEC_FENCE_IN | I915_EXEC_FENCE_OUT;
> +	execbuf.rsvd2 = (uint32_t)fence;
>  
> -	gem_execbuf(fd, &execbuf);
> +	gem_execbuf_wr(fd, &execbuf);
> +	close(fence);
>  
> -	igt_post_hang_ring(fd, hang);
> -	unplug(&cork); /* only now submit our batches */
> -	igt_assert_eq(__gem_wait(fd, obj[1].handle, -1), 0);
> +	fence = execbuf.rsvd2 >> 32;
> +	igt_assert(fence != -1);
> +
> +	igt_post_hang_ring(fd, hang); /* wedged, with an unready batch */
> +	sw_sync_timeline_inc(timeline, 1); /* only now submit our batches */
> +
> +	igt_assert_eq(__gem_wait(fd, obj.handle, -1), 0);

Is there any reason for rolling our own __gem_wait here? :>
(not a part of this patch... just curious)

> +	igt_assert_eq(sync_fence_status(fence), -EIO);
> +	close(fence);

I'd leave the cork & plug interface intact to make sure the intention is clear
(replacing just the inner workings), but that's just me.
You could add a comment stating that we're using the fence to plug the GPU, but
I guess the intent is kind of obvious here... So:

Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>

-Michał

>  
>  	igt_require(i915_reset_control(true));
>  	trigger_reset(fd);
> +	close(timeline);
>  }
>  
>  igt_main
> @@ -241,8 +224,8 @@ igt_main
>  	igt_subtest("wait")
>  		test_wait(fd);
>  
> -	igt_subtest("in-flight")
> -		test_inflight(fd);
> +	igt_subtest("in-flight-external")
> +		test_inflight_external(fd);
>  
>  	igt_fixture
>  		close(fd);
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx