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

Daniel Vetter daniel at ffwll.ch
Fri Dec 11 11:01:17 PST 2015


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 at 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


More information about the Intel-gfx mailing list