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

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Dec 14, 2015 at 09:59:17AM +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)
> v3: Renamed get_stable_obj_count -> igt_get_stable_obj_count
> 
> Signed-off-by: Derek Morton <derek.j.morton@intel.com>

Merged, thanks for the patch.
-Daniel

> ---
>  lib/igt_debugfs.c         | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_debugfs.h         |  6 +++++
>  tests/gem_flink_race.c    | 25 +++-----------------
>  tests/prime_self_import.c | 31 +++++--------------------
>  4 files changed, 73 insertions(+), 47 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 2c3b1cf..4322e8e 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -667,3 +667,61 @@ 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;
> +}
> +
> +/**
> + * igt_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 igt_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..24018eb 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 igt_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..30e33f6 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 = igt_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 = igt_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..992334d 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 = igt_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 = igt_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 = igt_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 = igt_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