Re: [PATCH 3/3] drm/i915/selftests: Replace wait-on-timeout with explicit timeout

From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/3] drm/i915/selftests: Replace wait-on-timeout with explicit timeout
Date: Mon, 09 Jul 2018 15:07:44 +0300	[thread overview]
Message-ID: <87efgctxwv.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20180709104237.21880-3-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In igt_flush_test() we install a background timer in order to ensure
> that the wait completes within a certain time. We can now tell the wait
> that it has to complete within a timeout, and so no longer need the
> background timer.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  .../gpu/drm/i915/selftests/igt_flush_test.c   | 55 +++----------------
>  1 file changed, 9 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c
> index 09ab037ce803..af66e3d4e23a 100644
> --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c
> +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c
> @@ -9,52 +9,8 @@
>  #include "../i915_selftest.h"
>  #include "igt_flush_test.h"
>  
> -struct wedge_me {
> -	struct delayed_work work;
> -	struct drm_i915_private *i915;
> -	const void *symbol;
> -};
> -
> -static void wedge_me(struct work_struct *work)
> -{
> -	struct wedge_me *w = container_of(work, typeof(*w), work.work);
> -
> -	pr_err("%pS timed out, cancelling all further testing.\n", w->symbol);
> -
> -	GEM_TRACE("%pS timed out.\n", w->symbol);
> -	GEM_TRACE_DUMP();
> -
> -	i915_gem_set_wedged(w->i915);
> -}
> -
> -static void __init_wedge(struct wedge_me *w,
> -			 struct drm_i915_private *i915,
> -			 long timeout,
> -			 const void *symbol)
> -{
> -	w->i915 = i915;
> -	w->symbol = symbol;
> -
> -	INIT_DELAYED_WORK_ONSTACK(&w->work, wedge_me);
> -	schedule_delayed_work(&w->work, timeout);
> -}
> -
> -static void __fini_wedge(struct wedge_me *w)
> -{
> -	cancel_delayed_work_sync(&w->work);
> -	destroy_delayed_work_on_stack(&w->work);
> -	w->i915 = NULL;
> -}
> -
> -#define wedge_on_timeout(W, DEV, TIMEOUT)				\
> -	for (__init_wedge((W), (DEV), (TIMEOUT), __builtin_return_address(0)); \
> -	     (W)->i915;							\
> -	     __fini_wedge((W)))
> -
>  int igt_flush_test(struct drm_i915_private *i915, unsigned int flags)
>  {
> -	struct wedge_me w;
> -
>  	cond_resched();
>  
>  	if (flags & I915_WAIT_LOCKED &&
> @@ -63,8 +19,15 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags)
>  		i915_gem_set_wedged(i915);
>  	}
>  
> -	wedge_on_timeout(&w, i915, HZ)
> -		i915_gem_wait_for_idle(i915, flags, MAX_SCHEDULE_TIMEOUT);
> +	if (i915_gem_wait_for_idle(i915, flags, HZ / 5) == -ETIME) {

For all the callers, changing from HZ to HZ/5 should be acceptable.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> +		pr_err("%pS timed out, cancelling all further testing.\n",
> +		       __builtin_return_address(0));
> +
> +		GEM_TRACE("%pS timed out.\n", __builtin_return_address(0));
> +		GEM_TRACE_DUMP();
> +
> +		i915_gem_set_wedged(i915);
> +	}
>  
>  	return i915_terminally_wedged(&i915->gpu_error) ? -EIO : 0;
>  }
> -- 
> 2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx