[PATCH] prime_mmap_coherency: Add return error tests for prime sync ioctl

[Tiago Vignatti]

On 03/17/2016 06:01 PM, Chris Wilson wrote:
> On Thu, Mar 17, 2016 at 03:18:03PM -0300, Tiago Vignatti wrote:
>> This patch adds ioctl-errors subtest to be used for exercising prime sync ioctl
>> errors.
>>
>> The subtest constantly interrupts via signals a function doing concurrent blit
>> to stress out the right usage of prime_sync_*, making sure these ioctl errors
>> are handled accordingly. Important to note that in case of failure (e.g. in a
>> case where the ioctl wouldn't try again in a return error) this test does not
>> reliably catch the problem with 100% of accuracy.
>>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>
>> ---
>>
>> Chris, your unpolished dma-buf patch for adding return error into the ioctl
>> calls lgtm. Let me know if you think this kind of test is useful now in igt.
>>
>> Thanks
>>
>> Tiago
>>
>>   tests/prime_mmap_coherency.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 87 insertions(+)
>>
>> diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c
>> index 180d8a4..bae2144 100644
>> --- a/tests/prime_mmap_coherency.c
>> +++ b/tests/prime_mmap_coherency.c
>> @@ -180,6 +180,88 @@ static void test_write_flush(bool expect_stale_cache)
>>   	munmap(ptr_cpu, width * height);
>>   }
>>
>> +static void blit_and_cmp(void)
>> +{
>> +	drm_intel_bo *bo_1;
>> +	drm_intel_bo *bo_2;
>> +	uint32_t *ptr_cpu;
>> +	uint32_t *ptr2_cpu;
>> +	int dma_buf_fd, dma_buf2_fd, i;
>> +	int local_fd;
>> +	drm_intel_bufmgr *local_bufmgr;
>> +	struct intel_batchbuffer *local_batch;
>> +
>> +	/* recreate process local variables */
>> +	local_fd = drm_open_driver(DRIVER_INTEL);
>> +	local_bufmgr = drm_intel_bufmgr_gem_init(local_fd, 4096);
>> +	igt_assert(local_bufmgr);
>> +
>> +	local_batch = intel_batchbuffer_alloc(local_bufmgr, intel_get_drm_devid(local_fd));
>> +	igt_assert(local_batch);
>> +
>> +	bo_1 = drm_intel_bo_alloc(local_bufmgr, "BO 1", width * height * 4, 4096);
>> +	dma_buf_fd = prime_handle_to_fd_for_mmap(local_fd, bo_1->handle);
>> +	igt_skip_on(errno == EINVAL);
>> +
>> +	ptr_cpu = mmap(NULL, width * height, PROT_READ | PROT_WRITE,
>> +			MAP_SHARED, dma_buf_fd, 0);
>> +	igt_assert(ptr_cpu != MAP_FAILED);
>> +
>> +	bo_2 = drm_intel_bo_alloc(local_bufmgr, "BO 2", width * height * 4, 4096);
>> +	dma_buf2_fd = prime_handle_to_fd_for_mmap(local_fd, bo_2->handle);
>> +
>> +	ptr2_cpu = mmap(NULL, width * height, PROT_READ | PROT_WRITE,
>> +			MAP_SHARED, dma_buf2_fd, 0);
>> +	igt_assert(ptr2_cpu != MAP_FAILED);
>> +
>> +	/* Fill up BO 1 with '1's and BO 2 with '0's */
>> +	prime_sync_start(dma_buf_fd, true);
>> +	memset(ptr_cpu, 0x11, width * height);
>> +	prime_sync_end(dma_buf_fd, true);
>> +
>> +	prime_sync_start(dma_buf2_fd, true);
>> +	memset(ptr2_cpu, 0x00, width * height);
>> +	prime_sync_end(dma_buf2_fd, true);
>> +
>> +	/* Copy BO 1 into BO 2, using blitter. */
>> +	intel_copy_bo(local_batch, bo_2, bo_1, width * height);
>> +	usleep(0); /* let someone else claim the mutex */
>> +
>> +	/* Compare BOs. If prime_sync_* were executed properly, the caches
>> +	 * should be synced. */
>> +	prime_sync_start(dma_buf2_fd, true);
>
> Maybe false here? Note that it makes any difference for the driver atm.

oh, my bad.

>> +	for (i = 0; i < (width * height) / 4; i++)
>> +		igt_fail_on_f(ptr2_cpu[i] != 0x11111111, "Found 0x%08x at offset 0x%08x\n", ptr2_cpu[i], i);
>> +	prime_sync_end(dma_buf2_fd, true);
>> +
>> +	drm_intel_bo_unreference(bo_1);
>> +	drm_intel_bo_unreference(bo_2);
>> +	munmap(ptr_cpu, width * height);
>> +	munmap(ptr2_cpu, width * height);
>
> Do we have anything that verifies that dmabuf maps persist beyond
> gem_close() on the original bo?

that's test_refcounting in prime_mmap.c


> Yes, that test should hit all interruptible paths we have in dmabuf and
> would be a great addition to igt.

cool, thanks. I'm resending now v2.

Tiago