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.mor...@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

Reply via email to