[Intel-gfx] [PATCH i-g-t] tests/gem_ppgtt: Check for vm leaks with flink and ppgtt

[Tvrtko Ursulin]

On 04/20/2015 01:50 PM, Chris Wilson wrote:
> On Mon, Apr 20, 2015 at 01:14:14PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Using imported objects should not leak i915 vmas (and vms).
>>
>> In practice this simulates Xorg importing fbcon and leaking (or not) one vma
>> per Xorg startup cycle.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> ---
>>   tests/gem_ppgtt.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 100 insertions(+)
>>
>> diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
>> index 5bf773c..9b5b3ee 100644
>> --- a/tests/gem_ppgtt.c
>> +++ b/tests/gem_ppgtt.c
>> @@ -200,6 +200,103 @@ static void surfaces_check(dri_bo **bo, int count, uint32_t expected)
>>   	}
>>   }
>>
>> +static void flink_and_contexts(void)
>> +{
>> +	drm_intel_bufmgr *bufmgr;
>> +	int fd, fd2;
>> +	dri_bo *bo;
>> +	uint32_t flink_name;
>> +	unsigned int max_line = 0, num_lines = 0, cnt = 0;
>> +	int ret;
>> +
>> +	/*
>> +	 * Export a bo via flink and access it from a child process via a
>> +	 * different ppggtt context. Make sure when child exists that the vma
>> +	 * (and hence the vm), associated with its ppgtt context, is torn down.
>> +	 */
>> +
>> +	fd = drm_open_any();
>> +
>> +	bufmgr = drm_intel_bufmgr_gem_init(fd, 4096);
>> +	igt_assert(bufmgr);
>> +
>> +	bo = create_bo(bufmgr, 0);
>> +	igt_assert(bo);
>> +
>> +	ret = drm_intel_bo_flink(bo, &flink_name);
>> +	igt_assert(ret == 0);
>> +
>> +	igt_fork(child, 20) {
>> +		int devid;
>> +		drm_intel_bufmgr *bufmgr;
>> +		int fd;
>> +		dri_bo *bo, *import;
>> +		struct intel_batchbuffer *batch;
>> +
>> +		fd = drm_open_any();
>> +		devid = intel_get_drm_devid(fd);
>> +
>> +		bufmgr = drm_intel_bufmgr_gem_init(fd, 4096);
>> +		igt_assert(bufmgr);
>> +
>> +		bo = create_bo(bufmgr, 0);
>> +		import = drm_intel_bo_gem_create_from_name(bufmgr,
>> +							   "flink_and_contexts",
>> +							   flink_name);
>> +		igt_assert(bo && import);
>> +
>> +		batch = intel_batchbuffer_alloc(bufmgr, devid);
>> +		igt_assert(batch);
>> +
>> +		intel_copy_bo(batch, bo, import, SIZE);
>
> Do we care about any differentiation between the default per-file
> context and explicitly created contexts?

Don't know - would expect so - but I am new in this area.

>> +		intel_batchbuffer_free(batch);
>> +
>> +		dri_bo_unreference(import);
>> +		dri_bo_unreference(bo);
>> +
>> +		drm_intel_bufmgr_destroy(bufmgr);
>> +		close(fd);
>> +
>> +		exit(0);
>> +	}
>> +
>> +	igt_waitchildren();
>> +
>> +	/*
>> +	 * Count the longest line in the file which lists the vmas per object.
>> +	 * This might be a bit fragile so maybe there is a better way.
>> +	 */
>
> So what I was thinking was something along the lines of:
>
> /* record the return value from exec[0].offset */
> intel_copy_bo(&leaked_vma_offset);
>
> gem_close(exec[0].handle);
>
> /* flush execbuffer and the vma should be gone */
> gem_sync();
>
> intel_copy_bo(&new_vma_offset);
> igt_assert(new_vma_offset == leaked_vma_offset);
>
> Will take a bit of judicious planning (like doing a self-copy on the dst
> first to make sure it bound ahead of the leak and not reallocating the
> batch handle)

Hm.. not bad!

> Or alternatively, we know the name of the flinked object, so we can do
> an explicit search! Ok, that seems more robust, but you do some sync
> point in there (either gem_sync() every child before exit or
> gem_quiescent_gpu()).

Anything to avoid debugfs parsing I think. Well not anything, if 
exec[].offset is not retrievable from intel_batchbuffer helpers I'll 
reconsider it. Definitely need to at least count ppgtt sections since 
the IGT already failed (to fail) on one older kernel with the poor 
character counting approach.

Thanks for the ideas!

Regards,

Tvrtko