[RFC,i-g-t,1/6] tests/gem_pread: drop stolen memory related subtests
diff mbox

Message ID 1506985218-28880-2-git-send-email-daniele.ceraolospurio@intel.com
State New
Headers show

Commit Message

Daniele Ceraolo Spurio Oct. 2, 2017, 11 p.m. UTC
The feature was never merged and there has been no progress in the
last year. The subtests are currently skipping on all platforms by
checking a field in the get_aperture ioctl structure that doesn't
exist in the kernel version of the struct.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 tests/gem_pread.c | 95 -------------------------------------------------------
 1 file changed, 95 deletions(-)

Comments

Chris Wilson Oct. 3, 2017, 11:08 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2017-10-03 00:00:13)
> The feature was never merged and there has been no progress in the
> last year. The subtests are currently skipping on all platforms by
> checking a field in the get_aperture ioctl structure that doesn't
> exist in the kernel version of the struct.

The interface for this is upstream, fix the test to exercise the kernel
code (hint dmabuf).
-Chris
Daniele Ceraolo Spurio Oct. 3, 2017, 3:36 p.m. UTC | #2
On 03/10/17 04:08, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2017-10-03 00:00:13)
>> The feature was never merged and there has been no progress in the
>> last year. The subtests are currently skipping on all platforms by
>> checking a field in the get_aperture ioctl structure that doesn't
>> exist in the kernel version of the struct.
> 
> The interface for this is upstream, fix the test to exercise the kernel
> code (hint dmabuf).
> -Chris
> 

I'm not very familiar with the dmabuf interface, but looking at it I 
couldn't find any way of allocating and/or accessing an object in stolen 
memory from userspace. A grep for "create_stolen" also didn't show 
anything. Can you point me in the right direction?

Thanks,
Daniele
Chris Wilson Oct. 3, 2017, 3:49 p.m. UTC | #3
Quoting Daniele Ceraolo Spurio (2017-10-03 16:36:37)
> 
> 
> On 03/10/17 04:08, Chris Wilson wrote:
> > Quoting Daniele Ceraolo Spurio (2017-10-03 00:00:13)
> >> The feature was never merged and there has been no progress in the
> >> last year. The subtests are currently skipping on all platforms by
> >> checking a field in the get_aperture ioctl structure that doesn't
> >> exist in the kernel version of the struct.
> > 
> > The interface for this is upstream, fix the test to exercise the kernel
> > code (hint dmabuf).
> > -Chris
> > 
> 
> I'm not very familiar with the dmabuf interface, but looking at it I 
> couldn't find any way of allocating and/or accessing an object in stolen 
> memory from userspace. A grep for "create_stolen" also didn't show 
> anything. Can you point me in the right direction?

It's just that a dmabuf of a e.g. vgem bo will use the same pread paths
that we originally wrote for stolen support. (We need a bo that is not
backed by struct page.)

Something like 

static uint32_t create_foreign_bo(int device, uint32_t sz, void *data)
{
	struct vgem_bo scratch;
	int vgem, dmabuf;
	uint32_t handle;

	vgem = drm_driver_open(DRIVER_VGEM);

	scratch.width = 1024;
	scratch.height = sz / 4096;
	scratch.bpp = 32;
	vgem_create(vgem, &scratch);

	igt_assert_eq(sz, scratch.size);

	if (data) {
		void *ptr;

		ptr = vgem_mmap(vgem, &scratch, PROT_WRITE);
		memcpy(ptr, data, sz);
		munmap(ptr, scratch.size);
	}	

	dmabuf = prime_handle_to_fd(vgem, scratch.handle);
	handle = prime_fd_to_handle(device, dmabuf);
	close(dmabuf);

	close(vgem);

	return handle;
}

will create a handle that we can pass to pread to exercise the same
paths. For pwrite, we need to keep the vgem bo around so we can read
back from its mmapping.

We do have coverage in prime_vgem.c, but it's not the first place you
would go to exercise i915_gem_pread_ioctl().
-Chris
Daniele Ceraolo Spurio Oct. 3, 2017, 4:02 p.m. UTC | #4
On 03/10/17 08:49, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2017-10-03 16:36:37)
>>
>>
>> On 03/10/17 04:08, Chris Wilson wrote:
>>> Quoting Daniele Ceraolo Spurio (2017-10-03 00:00:13)
>>>> The feature was never merged and there has been no progress in the
>>>> last year. The subtests are currently skipping on all platforms by
>>>> checking a field in the get_aperture ioctl structure that doesn't
>>>> exist in the kernel version of the struct.
>>>
>>> The interface for this is upstream, fix the test to exercise the kernel
>>> code (hint dmabuf).
>>> -Chris
>>>
>>
>> I'm not very familiar with the dmabuf interface, but looking at it I
>> couldn't find any way of allocating and/or accessing an object in stolen
>> memory from userspace. A grep for "create_stolen" also didn't show
>> anything. Can you point me in the right direction?
> 
> It's just that a dmabuf of a e.g. vgem bo will use the same pread paths
> that we originally wrote for stolen support. (We need a bo that is not
> backed by struct page.)
> 
> Something like
> 
> static uint32_t create_foreign_bo(int device, uint32_t sz, void *data)
> {
> 	struct vgem_bo scratch;
> 	int vgem, dmabuf;
> 	uint32_t handle;
> 
> 	vgem = drm_driver_open(DRIVER_VGEM);
> 
> 	scratch.width = 1024;
> 	scratch.height = sz / 4096;
> 	scratch.bpp = 32;
> 	vgem_create(vgem, &scratch);
> 
> 	igt_assert_eq(sz, scratch.size);
> 
> 	if (data) {
> 		void *ptr;
> 
> 		ptr = vgem_mmap(vgem, &scratch, PROT_WRITE);
> 		memcpy(ptr, data, sz);
> 		munmap(ptr, scratch.size);
> 	}	
> 
> 	dmabuf = prime_handle_to_fd(vgem, scratch.handle);
> 	handle = prime_fd_to_handle(device, dmabuf);
> 	close(dmabuf);
> 
> 	close(vgem);
> 
> 	return handle;
> }
> 
> will create a handle that we can pass to pread to exercise the same
> paths. For pwrite, we need to keep the vgem bo around so we can read
> back from its mmapping.
> 
> We do have coverage in prime_vgem.c, but it's not the first place you
> would go to exercise i915_gem_pread_ioctl().
> -Chris
> 

Thanks for the clarification. I'll update the patch to replace the 
stolen tests with the vgem ones.

Daniele

Patch
diff mbox

diff --git a/tests/gem_pread.c b/tests/gem_pread.c
index f4cf472..39a46ed 100644
--- a/tests/gem_pread.c
+++ b/tests/gem_pread.c
@@ -40,10 +40,6 @@ 
 #include "drm.h"
 
 #define OBJECT_SIZE 16384
-#define LARGE_OBJECT_SIZE 1024 * 1024
-#define KGRN "\x1B[32m"
-#define KRED "\x1B[31m"
-#define KNRM "\x1B[0m"
 
 static void do_gem_read(int fd, uint32_t handle, void *buf, int len, int loops)
 {
@@ -79,8 +75,6 @@  static const char *bytes_per_sec(char *buf, double v)
 
 
 uint32_t *src, dst;
-uint32_t *dst_user, src_stolen, large_stolen;
-uint32_t *stolen_pf_user, *stolen_nopf_user;
 int fd, count;
 
 int main(int argc, char **argv)
@@ -113,8 +107,6 @@  int main(int argc, char **argv)
 
 		dst = gem_create(fd, object_size);
 		src = malloc(object_size);
-		src_stolen = gem_create_stolen(fd, object_size);
-		dst_user = malloc(object_size);
 	}
 
 	igt_subtest("basic") {
@@ -151,96 +143,9 @@  int main(int argc, char **argv)
 		}
 	}
 
-	igt_subtest("stolen-normal") {
-		gem_require_stolen_support(fd);
-		for (count = 1; count <= 1<<17; count <<= 1) {
-			struct timeval start, end;
-
-			gettimeofday(&start, NULL);
-			do_gem_read(fd, src_stolen, dst_user, object_size, count);
-			gettimeofday(&end, NULL);
-			usecs = elapsed(&start, &end, count);
-			bps = bytes_per_sec(buf, object_size/usecs*1e6);
-			igt_info("Time to pread %d bytes x %6d:	%7.3fµs, %s\n",
-				 object_size, count, usecs, bps);
-			fflush(stdout);
-		}
-	}
-	for (c = cache; c->level != -1; c++) {
-		igt_subtest_f("stolen-%s", c->name) {
-			gem_require_stolen_support(fd);
-			gem_set_caching(fd, src_stolen, c->level);
-
-			for (count = 1; count <= 1<<17; count <<= 1) {
-				struct timeval start, end;
-
-				gettimeofday(&start, NULL);
-				do_gem_read(fd, src_stolen, dst_user,
-					    object_size, count);
-				gettimeofday(&end, NULL);
-				usecs = elapsed(&start, &end, count);
-				bps = bytes_per_sec(buf, object_size/usecs*1e6);
-				igt_info("Time to stolen-%s pread %d bytes x %6d:      %7.3fµs, %s\n",
-					 c->name, object_size, count, usecs, bps);
-				fflush(stdout);
-			}
-		}
-	}
-
-	/* List the time taken in pread operation for stolen objects, with
-	 * and without the overhead of page fault handling on accessing the
-	 * user space buffer
-	 */
-	igt_subtest("pagefault-pread") {
-		gem_require_stolen_support(fd);
-		large_stolen = gem_create_stolen(fd, LARGE_OBJECT_SIZE);
-		stolen_nopf_user = (uint32_t *) mmap(NULL, LARGE_OBJECT_SIZE,
-						PROT_WRITE,
-						MAP_ANONYMOUS|MAP_PRIVATE,
-						-1, 0);
-		igt_assert(stolen_nopf_user);
-
-		for (count = 1; count <= 10; count ++) {
-			struct timeval start, end;
-			double t_elapsed = 0;
-
-			gettimeofday(&start, NULL);
-			do_gem_read(fd, large_stolen, stolen_nopf_user,
-				    LARGE_OBJECT_SIZE, 1);
-			gettimeofday(&end, NULL);
-			t_elapsed = elapsed(&start, &end, count);
-			bps = bytes_per_sec(buf, object_size/t_elapsed*1e6);
-			igt_info("Pagefault-N - Time to pread %d bytes: %7.3fµs, %s\n",
-				 LARGE_OBJECT_SIZE, t_elapsed, bps);
-
-			stolen_pf_user = (uint32_t *) mmap(NULL, LARGE_OBJECT_SIZE,
-						      PROT_WRITE,
-						      MAP_ANONYMOUS|MAP_PRIVATE,
-						      -1, 0);
-			igt_assert(stolen_pf_user);
-
-			gettimeofday(&start, NULL);
-			do_gem_read(fd, large_stolen, stolen_pf_user,
-				    LARGE_OBJECT_SIZE, 1);
-			gettimeofday(&end, NULL);
-			usecs = elapsed(&start, &end, count);
-			bps = bytes_per_sec(buf, object_size/usecs*1e6);
-			igt_info("Pagefault-Y - Time to pread %d bytes: %7.3fµs, %s%s%s\n",
-				 LARGE_OBJECT_SIZE, usecs,
-				 t_elapsed < usecs ? KGRN : KRED, bps, KNRM);
-			fflush(stdout);
-			munmap(stolen_pf_user, LARGE_OBJECT_SIZE);
-		}
-		munmap(stolen_nopf_user, LARGE_OBJECT_SIZE);
-		gem_close(fd, large_stolen);
-	}
-
-
 	igt_fixture {
 		free(src);
 		gem_close(fd, dst);
-		free(dst_user);
-		gem_close(fd, src_stolen);
 
 		close(fd);
 	}