All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] tests: Fix "-thrashing" and "-thrash-inactive" distinction
@ 2013-11-04 16:30 oscar.mateo
  2013-11-04 16:30 ` [PATCH 2/4] lib: Add igt_drop_caches_set() oscar.mateo
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: oscar.mateo @ 2013-11-04 16:30 UTC (permalink / raw)
  To: intel-gfx

From: Oscar Mateo <oscar.mateo@intel.com>

A typo in the relocation tests made both sub-tests perform the
same action: drop *all* caches.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 tests/gem_persistent_relocs.c |    2 +-
 tests/gem_reloc_vs_gpu.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/gem_persistent_relocs.c b/tests/gem_persistent_relocs.c
index 29b19ed..34d8492 100644
--- a/tests/gem_persistent_relocs.c
+++ b/tests/gem_persistent_relocs.c
@@ -289,7 +289,7 @@ static void do_forked_test(int fd, unsigned flags)
 	if (flags & (THRASH | THRASH_INACTIVE)) {
 		char fname[FILENAME_MAX];
 		int drop_caches_fd;
-		const char *data = THRASH_INACTIVE ? "0xf" : "0x7";
+		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
 
 		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
 			 "/sys/kernel/debug/dri", drm_get_card(),
diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c
index ae7b446..bbfdc3a 100644
--- a/tests/gem_reloc_vs_gpu.c
+++ b/tests/gem_reloc_vs_gpu.c
@@ -284,7 +284,7 @@ static void do_forked_test(int fd, unsigned flags)
 	if (flags & (THRASH | THRASH_INACTIVE)) {
 		char fname[FILENAME_MAX];
 		int drop_caches_fd;
-		const char *data = THRASH_INACTIVE ? "0xf" : "0x7";
+		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
 
 		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
 			 "/sys/kernel/debug/dri", drm_get_card(),
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] lib: Add igt_drop_caches_set()
  2013-11-04 16:30 [PATCH 1/4] tests: Fix "-thrashing" and "-thrash-inactive" distinction oscar.mateo
@ 2013-11-04 16:30 ` oscar.mateo
  2013-11-04 17:06   ` Daniel Vetter
  2013-11-04 16:30 ` [PATCH 3/4] prime_self_import: Assure no pending requests before object counting oscar.mateo
  2013-11-04 16:30 ` [PATCH 4/4] gem_flink_race: " oscar.mateo
  2 siblings, 1 reply; 11+ messages in thread
From: oscar.mateo @ 2013-11-04 16:30 UTC (permalink / raw)
  To: intel-gfx

From: Oscar Mateo <oscar.mateo@intel.com>

This is basically a "drop cache" interface to the igt_debugfs
facilities. Also, update existing users.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_debugfs.c             |   28 ++++++++++++++++++++++++++++
 lib/igt_debugfs.h             |   15 +++++++++++++++
 tests/gem_persistent_relocs.c |   15 ++++-----------
 tests/gem_reloc_vs_gpu.c      |   15 ++++-----------
 4 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 0319eff..fc274fd 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -316,3 +316,31 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
 
 	*out_crcs = crcs;
 }
+
+/*
+ * Drop caches
+ */
+
+int igt_drop_caches_set(uint64_t val)
+{
+	igt_debugfs_t debugfs;
+	int fd;
+	char data[19];
+	size_t nbytes;
+	int ret = -1;
+
+	sprintf(data, "0x%" PRIx64, val);
+
+	igt_debugfs_init(&debugfs);
+	fd = igt_debugfs_open(&debugfs, "i915_gem_drop_caches", O_WRONLY);
+
+	if (fd >= 0)
+	{
+		nbytes = write(fd, data, strlen(data) + 1);
+		if (nbytes == strlen(data) + 1)
+			ret = 0;
+		close(fd);
+	}
+
+	return ret;
+}
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index c2810ee..02f4afa 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -79,4 +79,19 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
 void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
 			   igt_crc_t **out_crcs);
 
+/*
+ * Drop caches
+ */
+
+#define DROP_UNBOUND 0x1
+#define DROP_BOUND 0x2
+#define DROP_RETIRE 0x4
+#define DROP_ACTIVE 0x8
+#define DROP_ALL (DROP_UNBOUND | \
+		  DROP_BOUND | \
+		  DROP_RETIRE | \
+		  DROP_ACTIVE)
+
+int igt_drop_caches_set(uint64_t val);
+
 #endif /* __IGT_DEBUGFS_H__ */
diff --git a/tests/gem_persistent_relocs.c b/tests/gem_persistent_relocs.c
index 34d8492..863f464 100644
--- a/tests/gem_persistent_relocs.c
+++ b/tests/gem_persistent_relocs.c
@@ -42,6 +42,7 @@
 #include "intel_bufmgr.h"
 #include "intel_batchbuffer.h"
 #include "intel_gpu_tools.h"
+#include "igt_debugfs.h"
 
 /*
  * Testcase: Persistent relocations as used by uxa/libva
@@ -287,21 +288,13 @@ static void do_forked_test(int fd, unsigned flags)
 	struct igt_helper_process thrasher = {};
 
 	if (flags & (THRASH | THRASH_INACTIVE)) {
-		char fname[FILENAME_MAX];
-		int drop_caches_fd;
-		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
-
-		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
-			 "/sys/kernel/debug/dri", drm_get_card(),
-			 "i915_gem_drop_caches");
-
-		drop_caches_fd = open(fname, O_WRONLY);
-		igt_require(drop_caches_fd >= 0);
+		uint64_t val = (flags & THRASH_INACTIVE) ?
+				(DROP_RETIRE | DROP_BOUND | DROP_UNBOUND) : DROP_ALL;
 
 		igt_fork_helper(&thrasher) {
 			while (1) {
 				usleep(1000);
-				igt_assert(write(drop_caches_fd, data, strlen(data) + 1) == strlen(data) + 1);
+				do_or_die(igt_drop_caches_set(val));
 			}
 		}
 	}
diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c
index bbfdc3a..9508b1c 100644
--- a/tests/gem_reloc_vs_gpu.c
+++ b/tests/gem_reloc_vs_gpu.c
@@ -42,6 +42,7 @@
 #include "intel_bufmgr.h"
 #include "intel_batchbuffer.h"
 #include "intel_gpu_tools.h"
+#include "igt_debugfs.h"
 
 /*
  * Testcase: Kernel relocations vs. gpu races
@@ -282,21 +283,13 @@ static void do_forked_test(int fd, unsigned flags)
 	struct igt_helper_process thrasher = {};
 
 	if (flags & (THRASH | THRASH_INACTIVE)) {
-		char fname[FILENAME_MAX];
-		int drop_caches_fd;
-		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
-
-		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
-			 "/sys/kernel/debug/dri", drm_get_card(),
-			 "i915_gem_drop_caches");
-
-		drop_caches_fd = open(fname, O_WRONLY);
-		igt_require(drop_caches_fd >= 0);
+		uint64_t val = (flags & THRASH_INACTIVE) ?
+				(DROP_RETIRE | DROP_BOUND | DROP_UNBOUND) : DROP_ALL;
 
 		igt_fork_helper(&thrasher) {
 			while (1) {
 				usleep(1000);
-				igt_assert(write(drop_caches_fd, data, strlen(data) + 1) == strlen(data) + 1);
+				do_or_die(igt_drop_caches_set(val));
 			}
 		}
 	}
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] prime_self_import: Assure no pending requests before object counting
  2013-11-04 16:30 [PATCH 1/4] tests: Fix "-thrashing" and "-thrash-inactive" distinction oscar.mateo
  2013-11-04 16:30 ` [PATCH 2/4] lib: Add igt_drop_caches_set() oscar.mateo
@ 2013-11-04 16:30 ` oscar.mateo
  2013-11-04 17:09   ` Daniel Vetter
  2013-11-05 10:56   ` [PATCH 3/4 v2] " oscar.mateo
  2013-11-04 16:30 ` [PATCH 4/4] gem_flink_race: " oscar.mateo
  2 siblings, 2 replies; 11+ messages in thread
From: oscar.mateo @ 2013-11-04 16:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

From: Oscar Mateo <oscar.mateo@intel.com>

We don't want a previously used object to be freed in the middle of a
before/after object counting operation (or we would get a "-1 objects
leaked" message). We have seen this happening, e.g., when a context
from a previous run dies, but its backing object is alive waiting for
a retire_work to kick in.

v2: Use igt_debugfs facilities for drop cache

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
---
 tests/prime_self_import.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
index 2edc1f8..fb007bc 100644
--- a/tests/prime_self_import.c
+++ b/tests/prime_self_import.c
@@ -46,6 +46,7 @@
 #include "drm.h"
 #include "i915_drm.h"
 #include "drmtest.h"
+#include "igt_debugfs.h"
 
 #define BO_SIZE (16*1024)
 
@@ -252,10 +253,13 @@ static void test_reimport_close_race(void)
 	pthread_t *threads;
 	int r, i, num_threads;
 	int fds[2];
-	int obj_count = get_object_count();
+	int obj_count;
 	void *status;
 	uint32_t handle;
 
+	igt_drop_caches_set(DROP_RETIRE);
+	obj_count = get_object_count();
+
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
 	threads = calloc(num_threads, sizeof(pthread_t));
@@ -330,9 +334,12 @@ static void test_export_close_race(void)
 	pthread_t *threads;
 	int r, i, num_threads;
 	int fd;
-	int obj_count = get_object_count();
+	int obj_count;
 	void *status;
 
+	igt_drop_caches_set(DROP_RETIRE);
+	obj_count = get_object_count();
+
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
 	threads = calloc(num_threads, sizeof(pthread_t));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] gem_flink_race: Assure no pending requests before object counting
  2013-11-04 16:30 [PATCH 1/4] tests: Fix "-thrashing" and "-thrash-inactive" distinction oscar.mateo
  2013-11-04 16:30 ` [PATCH 2/4] lib: Add igt_drop_caches_set() oscar.mateo
  2013-11-04 16:30 ` [PATCH 3/4] prime_self_import: Assure no pending requests before object counting oscar.mateo
@ 2013-11-04 16:30 ` oscar.mateo
  2013-11-05 10:57   ` [PATCH 4/4 v2] " oscar.mateo
  2 siblings, 1 reply; 11+ messages in thread
From: oscar.mateo @ 2013-11-04 16:30 UTC (permalink / raw)
  To: intel-gfx

From: Oscar Mateo <oscar.mateo@intel.com>

Same thing that was done for prime_self_import.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 tests/gem_flink_race.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/gem_flink_race.c b/tests/gem_flink_race.c
index b81007e..9b25d9a 100644
--- a/tests/gem_flink_race.c
+++ b/tests/gem_flink_race.c
@@ -35,6 +35,7 @@
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_bufmgr.h"
+#include "igt_debugfs.h"
 
 /* Testcase: check for flink/open vs. gem close races
  *
@@ -157,9 +158,12 @@ static void test_flink_close(void)
 {
 	pthread_t *threads;
 	int r, i, num_threads;
-	int obj_count = get_object_count();
+	int obj_count;
 	void *status;
 
+	igt_drop_caches_set(DROP_RETIRE);
+	obj_count = get_object_count();
+
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
 	threads = calloc(num_threads, sizeof(pthread_t));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] lib: Add igt_drop_caches_set()
  2013-11-04 16:30 ` [PATCH 2/4] lib: Add igt_drop_caches_set() oscar.mateo
@ 2013-11-04 17:06   ` Daniel Vetter
  2013-11-04 17:22     ` Mateo Lozano, Oscar
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2013-11-04 17:06 UTC (permalink / raw)
  To: oscar.mateo; +Cc: intel-gfx

On Mon, Nov 04, 2013 at 04:30:47PM +0000, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> This is basically a "drop cache" interface to the igt_debugfs
> facilities. Also, update existing users.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_debugfs.c             |   28 ++++++++++++++++++++++++++++
>  lib/igt_debugfs.h             |   15 +++++++++++++++
>  tests/gem_persistent_relocs.c |   15 ++++-----------
>  tests/gem_reloc_vs_gpu.c      |   15 ++++-----------
>  4 files changed, 51 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 0319eff..fc274fd 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -316,3 +316,31 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>  
>  	*out_crcs = crcs;
>  }
> +
> +/*
> + * Drop caches
> + */
> +
> +int igt_drop_caches_set(uint64_t val)
> +{
> +	igt_debugfs_t debugfs;
> +	int fd;
> +	char data[19];
> +	size_t nbytes;
> +	int ret = -1;
> +
> +	sprintf(data, "0x%" PRIx64, val);
> +
> +	igt_debugfs_init(&debugfs);
> +	fd = igt_debugfs_open(&debugfs, "i915_gem_drop_caches", O_WRONLY);
> +
> +	if (fd >= 0)
> +	{
> +		nbytes = write(fd, data, strlen(data) + 1);
> +		if (nbytes == strlen(data) + 1)
> +			ret = 0;
> +		close(fd);
> +	}
> +
> +	return ret;
> +}

Just a quick style nitpick on igt helpers: If no one cares about the
return value then it's simpler to just sprinkle igt_asserts into the
helper and make the function never fail. I'll bikeshed that in a patch on
top of your series.
-Daniel

> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index c2810ee..02f4afa 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -79,4 +79,19 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
>  void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>  			   igt_crc_t **out_crcs);
>  
> +/*
> + * Drop caches
> + */
> +
> +#define DROP_UNBOUND 0x1
> +#define DROP_BOUND 0x2
> +#define DROP_RETIRE 0x4
> +#define DROP_ACTIVE 0x8
> +#define DROP_ALL (DROP_UNBOUND | \
> +		  DROP_BOUND | \
> +		  DROP_RETIRE | \
> +		  DROP_ACTIVE)
> +
> +int igt_drop_caches_set(uint64_t val);
> +
>  #endif /* __IGT_DEBUGFS_H__ */
> diff --git a/tests/gem_persistent_relocs.c b/tests/gem_persistent_relocs.c
> index 34d8492..863f464 100644
> --- a/tests/gem_persistent_relocs.c
> +++ b/tests/gem_persistent_relocs.c
> @@ -42,6 +42,7 @@
>  #include "intel_bufmgr.h"
>  #include "intel_batchbuffer.h"
>  #include "intel_gpu_tools.h"
> +#include "igt_debugfs.h"
>  
>  /*
>   * Testcase: Persistent relocations as used by uxa/libva
> @@ -287,21 +288,13 @@ static void do_forked_test(int fd, unsigned flags)
>  	struct igt_helper_process thrasher = {};
>  
>  	if (flags & (THRASH | THRASH_INACTIVE)) {
> -		char fname[FILENAME_MAX];
> -		int drop_caches_fd;
> -		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
> -
> -		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
> -			 "/sys/kernel/debug/dri", drm_get_card(),
> -			 "i915_gem_drop_caches");
> -
> -		drop_caches_fd = open(fname, O_WRONLY);
> -		igt_require(drop_caches_fd >= 0);
> +		uint64_t val = (flags & THRASH_INACTIVE) ?
> +				(DROP_RETIRE | DROP_BOUND | DROP_UNBOUND) : DROP_ALL;
>  
>  		igt_fork_helper(&thrasher) {
>  			while (1) {
>  				usleep(1000);
> -				igt_assert(write(drop_caches_fd, data, strlen(data) + 1) == strlen(data) + 1);
> +				do_or_die(igt_drop_caches_set(val));
>  			}
>  		}
>  	}
> diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c
> index bbfdc3a..9508b1c 100644
> --- a/tests/gem_reloc_vs_gpu.c
> +++ b/tests/gem_reloc_vs_gpu.c
> @@ -42,6 +42,7 @@
>  #include "intel_bufmgr.h"
>  #include "intel_batchbuffer.h"
>  #include "intel_gpu_tools.h"
> +#include "igt_debugfs.h"
>  
>  /*
>   * Testcase: Kernel relocations vs. gpu races
> @@ -282,21 +283,13 @@ static void do_forked_test(int fd, unsigned flags)
>  	struct igt_helper_process thrasher = {};
>  
>  	if (flags & (THRASH | THRASH_INACTIVE)) {
> -		char fname[FILENAME_MAX];
> -		int drop_caches_fd;
> -		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
> -
> -		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
> -			 "/sys/kernel/debug/dri", drm_get_card(),
> -			 "i915_gem_drop_caches");
> -
> -		drop_caches_fd = open(fname, O_WRONLY);
> -		igt_require(drop_caches_fd >= 0);
> +		uint64_t val = (flags & THRASH_INACTIVE) ?
> +				(DROP_RETIRE | DROP_BOUND | DROP_UNBOUND) : DROP_ALL;
>  
>  		igt_fork_helper(&thrasher) {
>  			while (1) {
>  				usleep(1000);
> -				igt_assert(write(drop_caches_fd, data, strlen(data) + 1) == strlen(data) + 1);
> +				do_or_die(igt_drop_caches_set(val));
>  			}
>  		}
>  	}
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] prime_self_import: Assure no pending requests before object counting
  2013-11-04 16:30 ` [PATCH 3/4] prime_self_import: Assure no pending requests before object counting oscar.mateo
@ 2013-11-04 17:09   ` Daniel Vetter
  2013-11-05 10:56   ` [PATCH 3/4 v2] " oscar.mateo
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2013-11-04 17:09 UTC (permalink / raw)
  To: oscar.mateo; +Cc: intel-gfx, Ben Widawsky

On Mon, Nov 04, 2013 at 04:30:48PM +0000, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> We don't want a previously used object to be freed in the middle of a
> before/after object counting operation (or we would get a "-1 objects
> leaked" message). We have seen this happening, e.g., when a context
> from a previous run dies, but its backing object is alive waiting for
> a retire_work to kick in.
> 
> v2: Use igt_debugfs facilities for drop cache
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>

Imo better to move the call to drop_caches into get_object_count - it
makes it clearer why we want this. I've applied the first two patches.

Thanks, Daniel
> ---
>  tests/prime_self_import.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
> index 2edc1f8..fb007bc 100644
> --- a/tests/prime_self_import.c
> +++ b/tests/prime_self_import.c
> @@ -46,6 +46,7 @@
>  #include "drm.h"
>  #include "i915_drm.h"
>  #include "drmtest.h"
> +#include "igt_debugfs.h"
>  
>  #define BO_SIZE (16*1024)
>  
> @@ -252,10 +253,13 @@ static void test_reimport_close_race(void)
>  	pthread_t *threads;
>  	int r, i, num_threads;
>  	int fds[2];
> -	int obj_count = get_object_count();
> +	int obj_count;
>  	void *status;
>  	uint32_t handle;
>  
> +	igt_drop_caches_set(DROP_RETIRE);
> +	obj_count = get_object_count();
> +
>  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
>  
>  	threads = calloc(num_threads, sizeof(pthread_t));
> @@ -330,9 +334,12 @@ static void test_export_close_race(void)
>  	pthread_t *threads;
>  	int r, i, num_threads;
>  	int fd;
> -	int obj_count = get_object_count();
> +	int obj_count;
>  	void *status;
>  
> +	igt_drop_caches_set(DROP_RETIRE);
> +	obj_count = get_object_count();
> +
>  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
>  
>  	threads = calloc(num_threads, sizeof(pthread_t));
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] lib: Add igt_drop_caches_set()
  2013-11-04 17:06   ` Daniel Vetter
@ 2013-11-04 17:22     ` Mateo Lozano, Oscar
  2013-11-04 17:41       ` Daniel Vetter
  0 siblings, 1 reply; 11+ messages in thread
From: Mateo Lozano, Oscar @ 2013-11-04 17:22 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

> > This is basically a "drop cache" interface to the igt_debugfs
> > facilities. Also, update existing users.
> >
> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  lib/igt_debugfs.c             |   28 ++++++++++++++++++++++++++++
> >  lib/igt_debugfs.h             |   15 +++++++++++++++
> >  tests/gem_persistent_relocs.c |   15 ++++-----------
> >  tests/gem_reloc_vs_gpu.c      |   15 ++++-----------
> >  4 files changed, 51 insertions(+), 22 deletions(-)
> >
> > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index
> > 0319eff..fc274fd 100644
> > --- a/lib/igt_debugfs.c
> > +++ b/lib/igt_debugfs.c
> > @@ -316,3 +316,31 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc,
> > int n_crcs,
> >
> >  	*out_crcs = crcs;
> >  }
> > +
> > +/*
> > + * Drop caches
> > + */
> > +
> > +int igt_drop_caches_set(uint64_t val) {
> > +	igt_debugfs_t debugfs;
> > +	int fd;
> > +	char data[19];
> > +	size_t nbytes;
> > +	int ret = -1;
> > +
> > +	sprintf(data, "0x%" PRIx64, val);
> > +
> > +	igt_debugfs_init(&debugfs);
> > +	fd = igt_debugfs_open(&debugfs, "i915_gem_drop_caches",
> O_WRONLY);
> > +
> > +	if (fd >= 0)
> > +	{
> > +		nbytes = write(fd, data, strlen(data) + 1);
> > +		if (nbytes == strlen(data) + 1)
> > +			ret = 0;
> > +		close(fd);
> > +	}
> > +
> > +	return ret;
> > +}
> 
> Just a quick style nitpick on igt helpers: If no one cares about the return value
> then it's simpler to just sprinkle igt_asserts into the helper and make the
> function never fail. I'll bikeshed that in a patch on top of your series.
> -Daniel

Well, I left the return value so that the user could decide whether or not to assert (e.g. gem_reloc_vs_gpu probably wants to assert, but if we end up adding a call to igt_drop_caches_set() inside gem_quiescent_gpu() and we assert, then we are virtually making *all* tests depend on i915_gem_drop_caches being available...). You know better than anyone if what I just said makes sense, so I´ll leave the ultimate decision to you :)

> > diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h index
> > c2810ee..02f4afa 100644
> > --- a/lib/igt_debugfs.h
> > +++ b/lib/igt_debugfs.h
> > @@ -79,4 +79,19 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
> > void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> >  			   igt_crc_t **out_crcs);
> >
> > +/*
> > + * Drop caches
> > + */
> > +
> > +#define DROP_UNBOUND 0x1
> > +#define DROP_BOUND 0x2
> > +#define DROP_RETIRE 0x4
> > +#define DROP_ACTIVE 0x8
> > +#define DROP_ALL (DROP_UNBOUND | \
> > +		  DROP_BOUND | \
> > +		  DROP_RETIRE | \
> > +		  DROP_ACTIVE)
> > +
> > +int igt_drop_caches_set(uint64_t val);
> > +
> >  #endif /* __IGT_DEBUGFS_H__ */
> > diff --git a/tests/gem_persistent_relocs.c
> > b/tests/gem_persistent_relocs.c index 34d8492..863f464 100644
> > --- a/tests/gem_persistent_relocs.c
> > +++ b/tests/gem_persistent_relocs.c
> > @@ -42,6 +42,7 @@
> >  #include "intel_bufmgr.h"
> >  #include "intel_batchbuffer.h"
> >  #include "intel_gpu_tools.h"
> > +#include "igt_debugfs.h"
> >
> >  /*
> >   * Testcase: Persistent relocations as used by uxa/libva @@ -287,21
> > +288,13 @@ static void do_forked_test(int fd, unsigned flags)
> >  	struct igt_helper_process thrasher = {};
> >
> >  	if (flags & (THRASH | THRASH_INACTIVE)) {
> > -		char fname[FILENAME_MAX];
> > -		int drop_caches_fd;
> > -		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
> > -
> > -		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
> > -			 "/sys/kernel/debug/dri", drm_get_card(),
> > -			 "i915_gem_drop_caches");
> > -
> > -		drop_caches_fd = open(fname, O_WRONLY);
> > -		igt_require(drop_caches_fd >= 0);
> > +		uint64_t val = (flags & THRASH_INACTIVE) ?
> > +				(DROP_RETIRE | DROP_BOUND |
> DROP_UNBOUND) : DROP_ALL;
> >
> >  		igt_fork_helper(&thrasher) {
> >  			while (1) {
> >  				usleep(1000);
> > -				igt_assert(write(drop_caches_fd, data,
> strlen(data) + 1) == strlen(data) + 1);
> > +				do_or_die(igt_drop_caches_set(val));
> >  			}
> >  		}
> >  	}
> > diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c index
> > bbfdc3a..9508b1c 100644
> > --- a/tests/gem_reloc_vs_gpu.c
> > +++ b/tests/gem_reloc_vs_gpu.c
> > @@ -42,6 +42,7 @@
> >  #include "intel_bufmgr.h"
> >  #include "intel_batchbuffer.h"
> >  #include "intel_gpu_tools.h"
> > +#include "igt_debugfs.h"
> >
> >  /*
> >   * Testcase: Kernel relocations vs. gpu races @@ -282,21 +283,13 @@
> > static void do_forked_test(int fd, unsigned flags)
> >  	struct igt_helper_process thrasher = {};
> >
> >  	if (flags & (THRASH | THRASH_INACTIVE)) {
> > -		char fname[FILENAME_MAX];
> > -		int drop_caches_fd;
> > -		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
> > -
> > -		snprintf(fname, FILENAME_MAX, "%s/%i/%s",
> > -			 "/sys/kernel/debug/dri", drm_get_card(),
> > -			 "i915_gem_drop_caches");
> > -
> > -		drop_caches_fd = open(fname, O_WRONLY);
> > -		igt_require(drop_caches_fd >= 0);
> > +		uint64_t val = (flags & THRASH_INACTIVE) ?
> > +				(DROP_RETIRE | DROP_BOUND |
> DROP_UNBOUND) : DROP_ALL;
> >
> >  		igt_fork_helper(&thrasher) {
> >  			while (1) {
> >  				usleep(1000);
> > -				igt_assert(write(drop_caches_fd, data,
> strlen(data) + 1) == strlen(data) + 1);
> > +				do_or_die(igt_drop_caches_set(val));
> >  			}
> >  		}
> >  	}
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] lib: Add igt_drop_caches_set()
  2013-11-04 17:22     ` Mateo Lozano, Oscar
@ 2013-11-04 17:41       ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2013-11-04 17:41 UTC (permalink / raw)
  To: Mateo Lozano, Oscar; +Cc: intel-gfx

On Mon, Nov 4, 2013 at 6:22 PM, Mateo Lozano, Oscar
<oscar.mateo@intel.com> wrote:
> Well, I left the return value so that the user could decide whether or not to assert (e.g. gem_reloc_vs_gpu probably wants to assert, but if we end up adding a call to igt_drop_caches_set() inside gem_quiescent_gpu() and we assert, then we are virtually making *all* tests depend on i915_gem_drop_caches being available...). You know better than anyone if what I just said makes sense, so I´ll leave the ultimate decision to you :)

Hm, I've forgotten that there are kernels without the drop caches
interface. I've just checked and the first version with it is 3.9.
That's old enough for me to no longer care, at least wrt testing stuff
with i-g-t. If anyone wants to regression test such old kernels they
can pick an older version of igt.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/4 v2] prime_self_import: Assure no pending requests before object counting
  2013-11-04 16:30 ` [PATCH 3/4] prime_self_import: Assure no pending requests before object counting oscar.mateo
  2013-11-04 17:09   ` Daniel Vetter
@ 2013-11-05 10:56   ` oscar.mateo
  1 sibling, 0 replies; 11+ messages in thread
From: oscar.mateo @ 2013-11-05 10:56 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

From: Oscar Mateo <oscar.mateo@intel.com>

We don't want a previously used object to be freed in the middle of a
before/after object counting operation (or we would get a "-1 objects
leaked" message). We have seen this happening, e.g., when a context
from a previous run dies, but its backing object is alive waiting for
a retire_work to kick in.

v2: Use igt_debugfs facilities for drop cache.
v3: Move igt_drop_caches_set() call inside get_object_count() to make
it clearer why we want this.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
---
 tests/prime_self_import.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
index 2edc1f8..2566af9 100644
--- a/tests/prime_self_import.c
+++ b/tests/prime_self_import.c
@@ -46,6 +46,7 @@
 #include "drm.h"
 #include "i915_drm.h"
 #include "drmtest.h"
+#include "igt_debugfs.h"
 
 #define BO_SIZE (16*1024)
 
@@ -218,6 +219,8 @@ static int get_object_count(void)
 	int device = drm_get_card();
 	char *path;
 
+	igt_drop_caches_set(DROP_RETIRE);
+
 	ret = asprintf(&path, "/sys/kernel/debug/dri/%d/i915_gem_objects", device);
 	igt_assert(ret != -1);
 
@@ -252,10 +255,12 @@ static void test_reimport_close_race(void)
 	pthread_t *threads;
 	int r, i, num_threads;
 	int fds[2];
-	int obj_count = get_object_count();
+	int obj_count;
 	void *status;
 	uint32_t handle;
 
+	obj_count = get_object_count();
+
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
 	threads = calloc(num_threads, sizeof(pthread_t));
@@ -330,9 +335,11 @@ static void test_export_close_race(void)
 	pthread_t *threads;
 	int r, i, num_threads;
 	int fd;
-	int obj_count = get_object_count();
+	int obj_count;
 	void *status;
 
+	obj_count = get_object_count();
+
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
 	threads = calloc(num_threads, sizeof(pthread_t));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4 v2] gem_flink_race: Assure no pending requests before object counting
  2013-11-04 16:30 ` [PATCH 4/4] gem_flink_race: " oscar.mateo
@ 2013-11-05 10:57   ` oscar.mateo
  2013-11-05 11:47     ` Daniel Vetter
  0 siblings, 1 reply; 11+ messages in thread
From: oscar.mateo @ 2013-11-05 10:57 UTC (permalink / raw)
  To: intel-gfx

From: Oscar Mateo <oscar.mateo@intel.com>

Same thing that was done for prime_self_import.

v2: Move igt_drop_caches_set() call inside get_object_count() to make
it clearer why we want this.

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 tests/gem_flink_race.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/gem_flink_race.c b/tests/gem_flink_race.c
index b81007e..195ec15 100644
--- a/tests/gem_flink_race.c
+++ b/tests/gem_flink_race.c
@@ -35,6 +35,7 @@
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_bufmgr.h"
+#include "igt_debugfs.h"
 
 /* Testcase: check for flink/open vs. gem close races
  *
@@ -54,6 +55,8 @@ static int get_object_count(void)
 	int device = drm_get_card();
 	char *path;
 
+	igt_drop_caches_set(DROP_RETIRE);
+
 	ret = asprintf(&path, "/sys/kernel/debug/dri/%d/i915_gem_objects", device);
 	igt_assert(ret != -1);
 
@@ -157,9 +160,11 @@ static void test_flink_close(void)
 {
 	pthread_t *threads;
 	int r, i, num_threads;
-	int obj_count = get_object_count();
+	int obj_count;
 	void *status;
 
+	obj_count = get_object_count();
+
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
 	threads = calloc(num_threads, sizeof(pthread_t));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4 v2] gem_flink_race: Assure no pending requests before object counting
  2013-11-05 10:57   ` [PATCH 4/4 v2] " oscar.mateo
@ 2013-11-05 11:47     ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2013-11-05 11:47 UTC (permalink / raw)
  To: oscar.mateo; +Cc: intel-gfx

On Tue, Nov 05, 2013 at 10:57:31AM +0000, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> Same thing that was done for prime_self_import.
> 
> v2: Move igt_drop_caches_set() call inside get_object_count() to make
> it clearer why we want this.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>

Both merged, thanks for the patches.
-Daniel

> ---
>  tests/gem_flink_race.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_flink_race.c b/tests/gem_flink_race.c
> index b81007e..195ec15 100644
> --- a/tests/gem_flink_race.c
> +++ b/tests/gem_flink_race.c
> @@ -35,6 +35,7 @@
>  #include "drmtest.h"
>  #include "i915_drm.h"
>  #include "intel_bufmgr.h"
> +#include "igt_debugfs.h"
>  
>  /* Testcase: check for flink/open vs. gem close races
>   *
> @@ -54,6 +55,8 @@ static int get_object_count(void)
>  	int device = drm_get_card();
>  	char *path;
>  
> +	igt_drop_caches_set(DROP_RETIRE);
> +
>  	ret = asprintf(&path, "/sys/kernel/debug/dri/%d/i915_gem_objects", device);
>  	igt_assert(ret != -1);
>  
> @@ -157,9 +160,11 @@ static void test_flink_close(void)
>  {
>  	pthread_t *threads;
>  	int r, i, num_threads;
> -	int obj_count = get_object_count();
> +	int obj_count;
>  	void *status;
>  
> +	obj_count = get_object_count();
> +
>  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
>  
>  	threads = calloc(num_threads, sizeof(pthread_t));
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-11-05 11:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-04 16:30 [PATCH 1/4] tests: Fix "-thrashing" and "-thrash-inactive" distinction oscar.mateo
2013-11-04 16:30 ` [PATCH 2/4] lib: Add igt_drop_caches_set() oscar.mateo
2013-11-04 17:06   ` Daniel Vetter
2013-11-04 17:22     ` Mateo Lozano, Oscar
2013-11-04 17:41       ` Daniel Vetter
2013-11-04 16:30 ` [PATCH 3/4] prime_self_import: Assure no pending requests before object counting oscar.mateo
2013-11-04 17:09   ` Daniel Vetter
2013-11-05 10:56   ` [PATCH 3/4 v2] " oscar.mateo
2013-11-04 16:30 ` [PATCH 4/4] gem_flink_race: " oscar.mateo
2013-11-05 10:57   ` [PATCH 4/4 v2] " oscar.mateo
2013-11-05 11:47     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.