Re: [igt-dev] [PATCH igt 1/5] lib/dummyload: Avoid assertions in lowlevel spin constructor

[Abdiel Janulgue <abdiel.janulgue@linux.intel.com>]

On 02/28/2018 05:51 PM, Chris Wilson wrote:
> __igt_spin_batch_new() may be used inside a background helper which is
> competing against the GPU being reset. As such, we cannot even assert
> that the spin->handle is busy immediately after submission as it may
> have already been reset by another client writing to i915_wedged.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>

> ---
>  lib/igt_dummyload.c | 17 ++++++++++++++---
>  tests/pm_rps.c      |  4 +++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index ddd43451..4b20f23d 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -25,6 +25,7 @@
>  #include <time.h>
>  #include <signal.h>
>  #include <pthread.h>
> +#include <sys/poll.h>
>  
>  #include <i915_drm.h>
>  
> @@ -207,7 +208,6 @@ ___igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep,
>  
>  	spin->out_fence = emit_recursive_batch(spin, fd, ctx, engine, dep,
>  					       out_fence);
> -	igt_assert(gem_bo_busy(fd, spin->handle));
>  
>  	pthread_mutex_lock(&list_lock);
>  	igt_list_add(&spin->link, &spin_list);
> @@ -240,9 +240,14 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
>  igt_spin_t *
>  igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
>  {
> +	igt_spin_t *spin;
> +
>  	igt_require_gem(fd);
>  
> -	return __igt_spin_batch_new(fd, ctx, engine, dep);
> +	spin = __igt_spin_batch_new(fd, ctx, engine, dep);
> +	igt_assert(gem_bo_busy(fd, spin->handle));
> +
> +	return spin;
>  }
>  
>  igt_spin_t *
> @@ -269,10 +274,16 @@ __igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
>  igt_spin_t *
>  igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
>  {
> +	igt_spin_t *spin;
> +
>  	igt_require_gem(fd);
>  	igt_require(gem_has_exec_fence(fd));
>  
> -	return __igt_spin_batch_new_fence(fd, ctx, engine);
> +	spin = __igt_spin_batch_new_fence(fd, ctx, engine);
> +	igt_assert(gem_bo_busy(fd, spin->handle));
> +	igt_assert(poll(&(struct pollfd){spin->out_fence, POLLIN}, 1, 0) == 0);
> +
> +	return spin;
>  }
>  
>  static void notify(union sigval arg)
> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
> index f71976ed..006d084b 100644
> --- a/tests/pm_rps.c
> +++ b/tests/pm_rps.c
> @@ -221,6 +221,8 @@ static void load_helper_run(enum load load)
>  		return;
>  	}
>  
> +	igt_require_gem(drm_fd);
> +
>  	lh.exit = false;
>  	lh.load = load;
>  
> @@ -233,7 +235,7 @@ static void load_helper_run(enum load load)
>  
>  		igt_debug("Applying %s load...\n", lh.load ? "high" : "low");
>  
> -		spin[0] = igt_spin_batch_new(drm_fd, 0, 0, 0);
> +		spin[0] = __igt_spin_batch_new(drm_fd, 0, 0, 0);
>  		if (lh.load == HIGH)
>  			spin[1] = __igt_spin_batch_new(drm_fd, 0, 0, 0);
>  		while (!lh.exit) {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev