[Intel-gfx] [PATCH v2 11/15] drm/i915: Exercise request cancellation using a mock selftest

[Chris Wilson]

On Wed, Feb 22, 2017 at 01:46:10PM +0000, Tvrtko Ursulin wrote:
> 
> On 22/02/2017 11:46, Chris Wilson wrote:
> >Add a mock selftest to preempt a request and check that we cancel it,
> >requeue the request and then complete its execution.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >---
> > drivers/gpu/drm/i915/selftests/i915_gem_request.c | 59 +++++++++++++++++++++++
> > drivers/gpu/drm/i915/selftests/mock_request.c     | 19 ++++++++
> > drivers/gpu/drm/i915/selftests/mock_request.h     |  2 +
> > 3 files changed, 80 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> >index 9d056a86723d..c803ef60a127 100644
> >--- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> >+++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> >@@ -26,6 +26,7 @@
> >
> > #include "../i915_selftest.h"
> >
> >+#include "mock_context.h"
> > #include "mock_gem_device.h"
> >
> > static int igt_add_request(void *arg)
> >@@ -181,12 +182,70 @@ static int igt_fence_wait(void *arg)
> > 	return err;
> > }
> >
> >+static int igt_request_rewind(void *arg)
> >+{
> >+	struct drm_i915_private *i915 = arg;
> >+	struct drm_i915_gem_request *request, *vip;
> >+	struct i915_gem_context *ctx[2];
> >+	int err = -EINVAL;
> >+
> >+	mutex_lock(&i915->drm.struct_mutex);
> >+	ctx[0] = mock_context(i915, "A");
> >+	request = mock_request(i915->engine[RCS], ctx[0], 2 * HZ);
> >+	if (!request) {
> >+		err = -ENOMEM;
> >+		goto err_device;
> 
> Leaks ctx[0].
> 
> >+	}
> >+	i915_add_request(request);
> >+
> >+	ctx[1] = mock_context(i915, "B");
> >+	vip = mock_request(i915->engine[RCS], ctx[1], 0);
> >+	if (!vip) {
> >+		err = -ENOMEM;
> >+		goto err_locked;
> 
> Leaks the request.

That's been handed over to the device.

> >+bool mock_cancel_request(struct drm_i915_gem_request *request)
> >+{
> >+	struct mock_request *mock = container_of(request, typeof(*mock), base);
> >+	struct mock_engine *engine =
> >+		container_of(request->engine, typeof(*engine), base);
> >+	bool was_queued;
> >+
> >+	spin_lock_irq(&engine->hw_lock);
> >+	was_queued = !list_empty(&mock->link);
> 
> I suggest a better name for the mock request. Even just req would be
> better IMO.

A bit late, the pattern has been set for the file. Currently
request for drm_i915_gem_request and mock for mock_request. I definitely
don't like the idea of req and request in the same function.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre