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

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

On 22/02/2017 13:59, Chris Wilson wrote:
> 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@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.

Pattern? Try grep "mock_request *" on selftests/* ! :)

Okay.. I r-b'ed some of those so I'll shut up.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx