[Intel-gfx] [PATCH 12/21] drm/i915: Replace wait-on-mutex with wait-on-bit in reset worker

[Mika Kuoppala]

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

> Since we have a cooperative mode now with a direct reset, we can avoid
> the contention on struct_mutex and instead try then sleep on the
> I915_RESET_IN_PROGRESS bit. If the mutex is held and that bit is
> cleared, all is fine. Otherwise, we sleep for a bit and try again. In
> the worst case we sleep for an extra second waiting for the mutex to be
> released (no one touching the GPU is allowed the struct_mutex whilst the
> I915_RESET_IN_PROGRESS bit is set). But when we have a direct reset,
> this allows us to clean up the reset worker faster.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

(sorry for the possible echo, i had to resend due to mail
problems)

Need to wait v2 for this one. Contemplating on waiting
pattern in irc, it became clear that the signaling side is
missing. Will just make the wakeup for the waiters
on reset queue faster.

-Mika

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2c7cb5041511..699ee2c7a3e4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2497,7 +2497,7 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv)
>  	char *error_event[] = { I915_ERROR_UEVENT "=1", NULL };
>  	char *reset_event[] = { I915_RESET_UEVENT "=1", NULL };
>  	char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL };
> -	int ret;
> +	int ret = -EAGAIN;
>  
>  	kobject_uevent_env(kobj, KOBJ_CHANGE, error_event);
>  
> @@ -2512,21 +2512,27 @@ static void i915_reset_and_wakeup(struct drm_i915_private *dev_priv)
>  	 * simulated reset via debugs, so get an RPM reference.
>  	 */
>  	intel_runtime_pm_get(dev_priv);
> -
>  	intel_prepare_reset(dev_priv);
>  
> -	/*
> -	 * All state reset _must_ be completed before we update the
> -	 * reset counter, for otherwise waiters might miss the reset
> -	 * pending state and not properly drop locks, resulting in
> -	 * deadlocks with the reset work.
> -	 */
> -	mutex_lock(&dev_priv->drm.struct_mutex);
> -	ret = i915_reset(dev_priv);
> -	mutex_unlock(&dev_priv->drm.struct_mutex);
> +	do {
> +		/*
> +		 * All state reset _must_ be completed before we update the
> +		 * reset counter, for otherwise waiters might miss the reset
> +		 * pending state and not properly drop locks, resulting in
> +		 * deadlocks with the reset work.
> +		 */
> +		if (mutex_trylock(&dev_priv->drm.struct_mutex)) {
> +			ret = i915_reset(dev_priv);
> +			mutex_unlock(&dev_priv->drm.struct_mutex);
> +		}
>  
> -	intel_finish_reset(dev_priv);
> +		/* We need to wait for anyone holding the lock to wakeup */
> +	} while (wait_on_bit_timeout(&dev_priv->gpu_error.flags,
> +				     I915_RESET_IN_PROGRESS,
> +				     TASK_UNINTERRUPTIBLE,
> +				     HZ));
>  
> +	intel_finish_reset(dev_priv);
>  	intel_runtime_pm_put(dev_priv);
>  
>  	if (ret == 0)
> -- 
> 2.9.3