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

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

[Tiago Vignatti]

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@chris-wilson.co.uk>
Signed-off-by: Tiago Vignatti <tiago.vignatti@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);
+	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);
+}
+
+/*
+ * Constantly interrupt concurrent blits to stress out prime_sync_* and make
+ * 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.
+ */
+static void test_ioctl_errors(void)
+{
+	int i;
+	int num_children = 8*sysconf(_SC_NPROCESSORS_ONLN);
+
+	igt_fork_signal_helper();
+	igt_fork(child, num_children) {
+		for (i = 0; i < ROUNDS; i++)
+			blit_and_cmp();
+	}
+	igt_waitchildren();
+	igt_stop_signal_helper();
+}
+
 int main(int argc, char **argv)
 {
 	int i;
@@ -235,6 +317,11 @@ int main(int argc, char **argv)
 		igt_fail_on_f(!stale, "couldn't find any stale cache lines\n");
 	}
 
+	igt_subtest("ioctl-errors") {
+		igt_info("exercising concurrent blit to get ioctl errors\n");
+		test_ioctl_errors();
+	}
+
 	igt_fixture {
 		intel_batchbuffer_free(batch);
 		drm_intel_bufmgr_destroy(bufmgr);
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Chris Wilson]

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@chris-wilson.co.uk>
> Signed-off-by: Tiago Vignatti <tiago.vignatti@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.

> +	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?

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@chris-wilson.co.uk>
>> Signed-off-by: Tiago Vignatti <tiago.vignatti@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

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

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

[Tiago Vignatti]

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.

v2: fix prime sync direction when reading mmap'ed file.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 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..80d1c1f 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, false);
+	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, false);
+
+	drm_intel_bo_unreference(bo_1);
+	drm_intel_bo_unreference(bo_2);
+	munmap(ptr_cpu, width * height);
+	munmap(ptr2_cpu, width * height);
+}
+
+/*
+ * Constantly interrupt concurrent blits to stress out prime_sync_* and make
+ * 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.
+ */
+static void test_ioctl_errors(void)
+{
+	int i;
+	int num_children = 8*sysconf(_SC_NPROCESSORS_ONLN);
+
+	igt_fork_signal_helper();
+	igt_fork(child, num_children) {
+		for (i = 0; i < ROUNDS; i++)
+			blit_and_cmp();
+	}
+	igt_waitchildren();
+	igt_stop_signal_helper();
+}
+
 int main(int argc, char **argv)
 {
 	int i;
@@ -235,6 +317,11 @@ int main(int argc, char **argv)
 		igt_fail_on_f(!stale, "couldn't find any stale cache lines\n");
 	}
 
+	igt_subtest("ioctl-errors") {
+		igt_info("exercising concurrent blit to get ioctl errors\n");
+		test_ioctl_errors();
+	}
+
 	igt_fixture {
 		intel_batchbuffer_free(batch);
 		drm_intel_bufmgr_destroy(bufmgr);
-- 
2.1.4

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

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

[Chris Wilson]

On Thu, Mar 17, 2016 at 06:18:06PM -0300, Tiago Vignatti wrote:
> +static void test_ioctl_errors(void)
> +{
> +	int i;
> +	int num_children = 8*sysconf(_SC_NPROCESSORS_ONLN);
> +
> +	igt_fork_signal_helper();
> +	igt_fork(child, num_children) {

Actually there is a danger here in only using a fork-bomb, we may loose
out on signal interruptions due to busyspins on the mutex (i.e. the
contention hiding critical paths, since we crucially rely on timing).

For fun, I would use:

int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
int num_children;

igt_fork_signal_helper();
for (int num_children = 1; num_children <= 8 *ncpus; num_children <<= 1) {
	igt_fork(child, num_children) {
		struct timespec start = {};
		while (igt_seconds_elapsed(&start) <= num_children)
			blit_and_cmp();
	}
}
igt_waitchildren();
igt_stop_signal_helper();
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] prime_mmap_coherency: Add return error tests for prime sync ioctl

[Chris Wilson]

On Fri, Mar 18, 2016 at 09:44:40AM +0000, Chris Wilson wrote:
> On Thu, Mar 17, 2016 at 06:18:06PM -0300, Tiago Vignatti wrote:
> > +static void test_ioctl_errors(void)
> > +{
> > +	int i;
> > +	int num_children = 8*sysconf(_SC_NPROCESSORS_ONLN);
> > +
> > +	igt_fork_signal_helper();
> > +	igt_fork(child, num_children) {
> 
> Actually there is a danger here in only using a fork-bomb, we may loose
> out on signal interruptions due to busyspins on the mutex (i.e. the
> contention hiding critical paths, since we crucially rely on timing).
> 
> For fun, I would use:
> 
> int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> int num_children;
> 
> igt_fork_signal_helper();
> for (int num_children = 1; num_children <= 8 *ncpus; num_children <<= 1) {
> 	igt_fork(child, num_children) {
> 		struct timespec start = {};
> 		while (igt_seconds_elapsed(&start) <= num_children)
> 			blit_and_cmp();
> 	}
> }
> igt_waitchildren();

With the bug ofc!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tiago Vignatti]

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.

v2: fix prime sync direction when reading mmap'ed file.
v3: change the upper bound using time rather than loops
 
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
---
 tests/prime_mmap_coherency.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c
index 180d8a4..d2b2a4f 100644
--- a/tests/prime_mmap_coherency.c
+++ b/tests/prime_mmap_coherency.c
@@ -180,6 +180,90 @@ 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, false);
+	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, false);
+
+	drm_intel_bo_unreference(bo_1);
+	drm_intel_bo_unreference(bo_2);
+	munmap(ptr_cpu, width * height);
+	munmap(ptr2_cpu, width * height);
+}
+
+/*
+ * Constantly interrupt concurrent blits to stress out prime_sync_* and make
+ * 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.
+ */
+static void test_ioctl_errors(void)
+{
+	int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+
+	igt_fork_signal_helper();
+	for (int num_children = 1; num_children <= 8 *ncpus; num_children <<= 1) {
+		igt_fork(child, num_children) {
+			struct timespec start = {};
+			while (igt_nsec_elapsed(&start) <= num_children)
+				blit_and_cmp();
+		}
+		igt_waitchildren();
+	}
+	igt_stop_signal_helper();
+}
+
 int main(int argc, char **argv)
 {
 	int i;
@@ -235,6 +319,11 @@ int main(int argc, char **argv)
 		igt_fail_on_f(!stale, "couldn't find any stale cache lines\n");
 	}
 
+	igt_subtest("ioctl-errors") {
+		igt_info("exercising concurrent blit to get ioctl errors\n");
+		test_ioctl_errors();
+	}
+
 	igt_fixture {
 		intel_batchbuffer_free(batch);
 		drm_intel_bufmgr_destroy(bufmgr);
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Fri, Mar 18, 2016 at 03:08:56PM -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.
> 
> v2: fix prime sync direction when reading mmap'ed file.
> v3: change the upper bound using time rather than loops
>  
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>

I'm probably blind, but where is the reviewed kernel patch for this? If
it's somewhere hidden, please resubmit with all the whizzbang stuff needed
for merging added ;-)

Thanks, Daniel

> ---
>  tests/prime_mmap_coherency.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
> 
> diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c
> index 180d8a4..d2b2a4f 100644
> --- a/tests/prime_mmap_coherency.c
> +++ b/tests/prime_mmap_coherency.c
> @@ -180,6 +180,90 @@ 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, false);
> +	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, false);
> +
> +	drm_intel_bo_unreference(bo_1);
> +	drm_intel_bo_unreference(bo_2);
> +	munmap(ptr_cpu, width * height);
> +	munmap(ptr2_cpu, width * height);
> +}
> +
> +/*
> + * Constantly interrupt concurrent blits to stress out prime_sync_* and make
> + * 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.
> + */
> +static void test_ioctl_errors(void)
> +{
> +	int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> +
> +	igt_fork_signal_helper();
> +	for (int num_children = 1; num_children <= 8 *ncpus; num_children <<= 1) {
> +		igt_fork(child, num_children) {
> +			struct timespec start = {};
> +			while (igt_nsec_elapsed(&start) <= num_children)
> +				blit_and_cmp();
> +		}
> +		igt_waitchildren();
> +	}
> +	igt_stop_signal_helper();
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	int i;
> @@ -235,6 +319,11 @@ int main(int argc, char **argv)
>  		igt_fail_on_f(!stale, "couldn't find any stale cache lines\n");
>  	}
>  
> +	igt_subtest("ioctl-errors") {
> +		igt_info("exercising concurrent blit to get ioctl errors\n");
> +		test_ioctl_errors();
> +	}
> +
>  	igt_fixture {
>  		intel_batchbuffer_free(batch);
>  		drm_intel_bufmgr_destroy(bufmgr);
> -- 
> 2.1.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tiago Vignatti]

On 03/18/2016 03:11 PM, Daniel Vetter wrote:
> On Fri, Mar 18, 2016 at 03:08:56PM -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.
>>
>> v2: fix prime sync direction when reading mmap'ed file.
>> v3: change the upper bound using time rather than loops
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
>
> I'm probably blind, but where is the reviewed kernel patch for this? If
> it's somewhere hidden, please resubmit with all the whizzbang stuff needed
> for merging added ;-)
>
> Thanks, Daniel

You're not blind Daniel :) Chris will be sending the kernel side but 
regardless this igt test should be good to go even without the kernel patch.

Tiago
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Chris Wilson]

On Fri, Mar 18, 2016 at 03:08:56PM -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.
> 
> v2: fix prime sync direction when reading mmap'ed file.
> v3: change the upper bound using time rather than loops
>  
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>
> ---
> +static void test_ioctl_errors(void)
> +{
> +	int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> +
> +	igt_fork_signal_helper();
> +	for (int num_children = 1; num_children <= 8 *ncpus; num_children <<= 1) {

Hmm, that's a lot of buffers....

I'm going to stick a couple of intel_require_memmory and
intel_check_memory() here.

Wait that's no moon. Oops, give me back my swap!

> +		igt_fork(child, num_children) {
> +			struct timespec start = {};
> +			while (igt_nsec_elapsed(&start) <= num_children)

igt_nsec_elapsed().... Barely any time at all!

Presumed you wanted seconds, fixed the memleak and pushed.
Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel