[PATCH i-g-t v2] gem_flink_race/prime_self_import: Improve test reliability

[PATCH i-g-t v2] gem_flink_race/prime_self_import: Improve test reliability

[Derek Morton]

gem_flink_race and prime_self_import have subtests which read the
number of open gem objects from debugfs to determine if objects have
leaked during the test. However the test can fail sporadically if
the number of gem objects changes due to other process activity.
This patch introduces a change to check the number of gem objects
several times to filter out any fluctuations.

v2: Moved the common code to a library and made the loop android
specific (Daniel Vetter)

Signed-off-by: Derek Morton <derek.j.morton@intel.com>
---
 lib/igt_debugfs.c         | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_debugfs.h         |  6 +++++
 tests/gem_flink_race.c    | 25 +++------------------
 tests/prime_self_import.c | 31 +++++---------------------
 4 files changed, 72 insertions(+), 47 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 2c3b1cf..b467b09 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -667,3 +667,60 @@ void igt_enable_prefault(void)
 {
 	igt_prefault_control(true);
 }
+
+static int get_object_count(void)
+{
+	FILE *file;
+	int ret, scanned;
+
+	igt_drop_caches_set(DROP_RETIRE | DROP_ACTIVE);
+
+	file = igt_debugfs_fopen("i915_gem_objects", "r");
+
+	scanned = fscanf(file, "%i objects", &ret);
+	igt_assert_eq(scanned, 1);
+
+	return ret;
+}
+
+/**
+ * get_stable_obj_count:
+ * @driver: fd to drm/i915 GEM driver
+ *
+ * This puts the driver into a stable (quiescent) state and then returns the
+ * current number of gem buffer objects as reported in the i915_gem_objects
+ * debugFS interface.
+ */
+int get_stable_obj_count(int driver)
+{
+	int obj_count;
+	gem_quiescent_gpu(driver);
+	obj_count = get_object_count();
+	/* The test relies on the system being in the same state before and
+	   after the test so any difference in the object count is a result of
+	   leaks during the test. gem_quiescent_gpu() mostly achieves this but
+	   on android occasionally obj_count can still change briefly.
+	   The loop ensures obj_count has remained stable over several checks */
+#ifdef ANDROID
+	{
+		int loop_count = 0;
+		int prev_obj_count = obj_count;
+		while (loop_count < 4) {
+			usleep(200000);
+			gem_quiescent_gpu(driver);
+			obj_count = get_object_count();
+			if (obj_count == prev_obj_count) {
+				loop_count++;
+			} else {
+				igt_debug("loop_count=%d, obj_count=%d, prev_obj_count=%d\n",
+					loop_count, obj_count, prev_obj_count);
+				loop_count = 0;
+				prev_obj_count = obj_count;
+			}
+
+		}
+	}
+#endif
+	return obj_count;
+}
+
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index bbf7f69..09e73a8 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -165,4 +165,10 @@ void igt_drop_caches_set(uint64_t val);
 void igt_disable_prefault(void);
 void igt_enable_prefault(void);
 
+/*
+ * Put the driver into a stable (quiescent) state and get the current number of
+ * gem buffer objects
+ */
+int get_stable_obj_count(int driver);
+
 #endif /* __IGT_DEBUGFS_H__ */
diff --git a/tests/gem_flink_race.c b/tests/gem_flink_race.c
index b17ef85..757cbca 100644
--- a/tests/gem_flink_race.c
+++ b/tests/gem_flink_race.c
@@ -44,28 +44,11 @@ IGT_TEST_DESCRIPTION("Check for flink/open vs. gem close races.");
  * in the flink name and corresponding reference getting leaked.
  */
 
-/* We want lockless and I'm to lazy to dig out an atomic libarary. On x86 this
+/* We want lockless and I'm to lazy to dig out an atomic library. On x86 this
  * works, too. */
 volatile int pls_die = 0;
 int fd;
 
-static int get_object_count(void)
-{
-	FILE *file;
-	int scanned, ret;
-
-	igt_drop_caches_set(DROP_RETIRE | DROP_ACTIVE);
-
-	file = igt_debugfs_fopen("i915_gem_objects", "r");
-
-	scanned = fscanf(file, "%i objects", &ret);
-	igt_assert_eq(scanned, 1);
-	igt_debug("Reports %d objects\n", ret);
-
-	return ret;
-}
-
-
 static void *thread_fn_flink_name(void *p)
 {
 	struct drm_gem_open open_struct;
@@ -164,8 +147,7 @@ static void test_flink_close(void)
 	 * up the counts */
 	fake = drm_open_driver(DRIVER_INTEL);
 
-	gem_quiescent_gpu(fake);
-	obj_count = get_object_count();
+	obj_count = get_stable_obj_count(fake);
 
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
@@ -190,8 +172,7 @@ static void test_flink_close(void)
 
 	close(fd);
 
-	gem_quiescent_gpu(fake);
-	obj_count = get_object_count() - obj_count;
+	obj_count = get_stable_obj_count(fake) - obj_count;
 
 	igt_info("leaked %i objects\n", obj_count);
 
diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
index 91fe231..931e91e 100644
--- a/tests/prime_self_import.c
+++ b/tests/prime_self_import.c
@@ -47,7 +47,7 @@
 #include "drm.h"
 
 IGT_TEST_DESCRIPTION("Check whether prime import/export works on the same"
-		     " device.");
+		     " device... but with different fds.");
 
 #define BO_SIZE (16*1024)
 
@@ -157,7 +157,7 @@ static void test_with_one_bo_two_files(void)
 	dma_buf_fd2 = prime_handle_to_fd(fd2, handle_open);
 	handle_import = prime_fd_to_handle(fd2, dma_buf_fd2);
 
-	/* dma-buf selfimporting an flink bo should give the same handle */
+	/* dma-buf self importing an flink bo should give the same handle */
 	igt_assert_eq_u32(handle_import, handle_open);
 
 	close(fd1);
@@ -211,21 +211,6 @@ static void test_with_one_bo(void)
 	check_bo(fd2, handle_import1, fd2, handle_import1);
 }
 
-static int get_object_count(void)
-{
-	FILE *file;
-	int ret, scanned;
-
-	igt_drop_caches_set(DROP_RETIRE | DROP_ACTIVE);
-
-	file = igt_debugfs_fopen("i915_gem_objects", "r");
-
-	scanned = fscanf(file, "%i objects", &ret);
-	igt_assert_eq(scanned, 1);
-
-	return ret;
-}
-
 static void *thread_fn_reimport_vs_close(void *p)
 {
 	struct drm_gem_close close_bo;
@@ -258,8 +243,7 @@ static void test_reimport_close_race(void)
 	 * up the counts */
 	fake = drm_open_driver(DRIVER_INTEL);
 
-	gem_quiescent_gpu(fake);
-	obj_count = get_object_count();
+	obj_count = get_stable_obj_count(fake);
 
 	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
 
@@ -290,8 +274,7 @@ static void test_reimport_close_race(void)
 	close(fds[0]);
 	close(fds[1]);
 
-	gem_quiescent_gpu(fake);
-	obj_count = get_object_count() - obj_count;
+	obj_count = get_stable_obj_count(fake) - obj_count;
 
 	igt_info("leaked %i objects\n", obj_count);
 
@@ -350,8 +333,7 @@ static void test_export_close_race(void)
 	 * up the counts */
 	fake = drm_open_driver(DRIVER_INTEL);
 
-	gem_quiescent_gpu(fake);
-	obj_count = get_object_count();
+	obj_count = get_stable_obj_count(fake);
 
 	fd = drm_open_driver(DRIVER_INTEL);
 
@@ -373,8 +355,7 @@ static void test_export_close_race(void)
 
 	close(fd);
 
-	gem_quiescent_gpu(fake);
-	obj_count = get_object_count() - obj_count;
+	obj_count = get_stable_obj_count(fake) - obj_count;
 
 	igt_info("leaked %i objects\n", obj_count);
 
-- 
1.9.1

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

Re: [PATCH i-g-t v2] gem_flink_race/prime_self_import: Improve test reliability

[Daniel Vetter]

On Fri, Dec 11, 2015 at 03:18:30PM +0000, Derek Morton wrote:
> gem_flink_race and prime_self_import have subtests which read the
> number of open gem objects from debugfs to determine if objects have
> leaked during the test. However the test can fail sporadically if
> the number of gem objects changes due to other process activity.
> This patch introduces a change to check the number of gem objects
> several times to filter out any fluctuations.
> 
> v2: Moved the common code to a library and made the loop android
> specific (Daniel Vetter)
> 
> Signed-off-by: Derek Morton <derek.j.morton@intel.com>
> ---
>  lib/igt_debugfs.c         | 57 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_debugfs.h         |  6 +++++
>  tests/gem_flink_race.c    | 25 +++------------------
>  tests/prime_self_import.c | 31 +++++---------------------
>  4 files changed, 72 insertions(+), 47 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 2c3b1cf..b467b09 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -667,3 +667,60 @@ void igt_enable_prefault(void)
>  {
>  	igt_prefault_control(true);
>  }
> +
> +static int get_object_count(void)
> +{
> +	FILE *file;
> +	int ret, scanned;
> +
> +	igt_drop_caches_set(DROP_RETIRE | DROP_ACTIVE);
> +
> +	file = igt_debugfs_fopen("i915_gem_objects", "r");
> +
> +	scanned = fscanf(file, "%i objects", &ret);
> +	igt_assert_eq(scanned, 1);
> +
> +	return ret;
> +}
> +
> +/**
> + * get_stable_obj_count:

Please give this an igt_debugfs_ or whatever prefix for namespacing.

> + * @driver: fd to drm/i915 GEM driver
> + *
> + * This puts the driver into a stable (quiescent) state and then returns the
> + * current number of gem buffer objects as reported in the i915_gem_objects
> + * debugFS interface.
> + */
> +int get_stable_obj_count(int driver)
> +{
> +	int obj_count;
> +	gem_quiescent_gpu(driver);
> +	obj_count = get_object_count();
> +	/* The test relies on the system being in the same state before and
> +	   after the test so any difference in the object count is a result of
> +	   leaks during the test. gem_quiescent_gpu() mostly achieves this but
> +	   on android occasionally obj_count can still change briefly.
> +	   The loop ensures obj_count has remained stable over several checks */

Please use kernel comment style like

	/* bla
	 * more bla
	 */

lgtm otherwise.
-Daniel

> +#ifdef ANDROID
> +	{
> +		int loop_count = 0;
> +		int prev_obj_count = obj_count;
> +		while (loop_count < 4) {
> +			usleep(200000);
> +			gem_quiescent_gpu(driver);
> +			obj_count = get_object_count();
> +			if (obj_count == prev_obj_count) {
> +				loop_count++;
> +			} else {
> +				igt_debug("loop_count=%d, obj_count=%d, prev_obj_count=%d\n",
> +					loop_count, obj_count, prev_obj_count);
> +				loop_count = 0;
> +				prev_obj_count = obj_count;
> +			}
> +
> +		}
> +	}
> +#endif
> +	return obj_count;
> +}
> +
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index bbf7f69..09e73a8 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -165,4 +165,10 @@ void igt_drop_caches_set(uint64_t val);
>  void igt_disable_prefault(void);
>  void igt_enable_prefault(void);
>  
> +/*
> + * Put the driver into a stable (quiescent) state and get the current number of
> + * gem buffer objects
> + */
> +int get_stable_obj_count(int driver);
> +
>  #endif /* __IGT_DEBUGFS_H__ */
> diff --git a/tests/gem_flink_race.c b/tests/gem_flink_race.c
> index b17ef85..757cbca 100644
> --- a/tests/gem_flink_race.c
> +++ b/tests/gem_flink_race.c
> @@ -44,28 +44,11 @@ IGT_TEST_DESCRIPTION("Check for flink/open vs. gem close races.");
>   * in the flink name and corresponding reference getting leaked.
>   */
>  
> -/* We want lockless and I'm to lazy to dig out an atomic libarary. On x86 this
> +/* We want lockless and I'm to lazy to dig out an atomic library. On x86 this
>   * works, too. */
>  volatile int pls_die = 0;
>  int fd;
>  
> -static int get_object_count(void)
> -{
> -	FILE *file;
> -	int scanned, ret;
> -
> -	igt_drop_caches_set(DROP_RETIRE | DROP_ACTIVE);
> -
> -	file = igt_debugfs_fopen("i915_gem_objects", "r");
> -
> -	scanned = fscanf(file, "%i objects", &ret);
> -	igt_assert_eq(scanned, 1);
> -	igt_debug("Reports %d objects\n", ret);
> -
> -	return ret;
> -}
> -
> -
>  static void *thread_fn_flink_name(void *p)
>  {
>  	struct drm_gem_open open_struct;
> @@ -164,8 +147,7 @@ static void test_flink_close(void)
>  	 * up the counts */
>  	fake = drm_open_driver(DRIVER_INTEL);
>  
> -	gem_quiescent_gpu(fake);
> -	obj_count = get_object_count();
> +	obj_count = get_stable_obj_count(fake);
>  
>  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
>  
> @@ -190,8 +172,7 @@ static void test_flink_close(void)
>  
>  	close(fd);
>  
> -	gem_quiescent_gpu(fake);
> -	obj_count = get_object_count() - obj_count;
> +	obj_count = get_stable_obj_count(fake) - obj_count;
>  
>  	igt_info("leaked %i objects\n", obj_count);
>  
> diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
> index 91fe231..931e91e 100644
> --- a/tests/prime_self_import.c
> +++ b/tests/prime_self_import.c
> @@ -47,7 +47,7 @@
>  #include "drm.h"
>  
>  IGT_TEST_DESCRIPTION("Check whether prime import/export works on the same"
> -		     " device.");
> +		     " device... but with different fds.");
>  
>  #define BO_SIZE (16*1024)
>  
> @@ -157,7 +157,7 @@ static void test_with_one_bo_two_files(void)
>  	dma_buf_fd2 = prime_handle_to_fd(fd2, handle_open);
>  	handle_import = prime_fd_to_handle(fd2, dma_buf_fd2);
>  
> -	/* dma-buf selfimporting an flink bo should give the same handle */
> +	/* dma-buf self importing an flink bo should give the same handle */
>  	igt_assert_eq_u32(handle_import, handle_open);
>  
>  	close(fd1);
> @@ -211,21 +211,6 @@ static void test_with_one_bo(void)
>  	check_bo(fd2, handle_import1, fd2, handle_import1);
>  }
>  
> -static int get_object_count(void)
> -{
> -	FILE *file;
> -	int ret, scanned;
> -
> -	igt_drop_caches_set(DROP_RETIRE | DROP_ACTIVE);
> -
> -	file = igt_debugfs_fopen("i915_gem_objects", "r");
> -
> -	scanned = fscanf(file, "%i objects", &ret);
> -	igt_assert_eq(scanned, 1);
> -
> -	return ret;
> -}
> -
>  static void *thread_fn_reimport_vs_close(void *p)
>  {
>  	struct drm_gem_close close_bo;
> @@ -258,8 +243,7 @@ static void test_reimport_close_race(void)
>  	 * up the counts */
>  	fake = drm_open_driver(DRIVER_INTEL);
>  
> -	gem_quiescent_gpu(fake);
> -	obj_count = get_object_count();
> +	obj_count = get_stable_obj_count(fake);
>  
>  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
>  
> @@ -290,8 +274,7 @@ static void test_reimport_close_race(void)
>  	close(fds[0]);
>  	close(fds[1]);
>  
> -	gem_quiescent_gpu(fake);
> -	obj_count = get_object_count() - obj_count;
> +	obj_count = get_stable_obj_count(fake) - obj_count;
>  
>  	igt_info("leaked %i objects\n", obj_count);
>  
> @@ -350,8 +333,7 @@ static void test_export_close_race(void)
>  	 * up the counts */
>  	fake = drm_open_driver(DRIVER_INTEL);
>  
> -	gem_quiescent_gpu(fake);
> -	obj_count = get_object_count();
> +	obj_count = get_stable_obj_count(fake);
>  
>  	fd = drm_open_driver(DRIVER_INTEL);
>  
> @@ -373,8 +355,7 @@ static void test_export_close_race(void)
>  
>  	close(fd);
>  
> -	gem_quiescent_gpu(fake);
> -	obj_count = get_object_count() - obj_count;
> +	obj_count = get_stable_obj_count(fake) - obj_count;
>  
>  	igt_info("leaked %i objects\n", obj_count);
>  
> -- 
> 1.9.1
> 

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