[Intel-gfx] [PATCH v4 1/3] drm/i915: Avoid early GPU idling due to race with new request

[Chris Wilson]

On Sat, Nov 05, 2016 at 10:35:59AM +0200, Imre Deak wrote:
> There is a small race where a new request can be submitted and retired
> after the idle worker started to run which leads to idling the GPU too
> early. Fix this by deferring the idling to the pending instance of the
> worker.
> 
> This scenario was pointed out by Chris.
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0dbf38c..36a16df 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2766,6 +2766,13 @@ i915_gem_idle_work_handler(struct work_struct *work)
>  		goto out_rearm;
>  	}
>  
> +	/*
> +	 * New request retired after this work handler started, extend active
> +	 * period until next instance of the work.
> +	 */
> +	if (work_pending(work))
> +		goto out_unlock;

Ok, it took some digging around inside kernel/workqueue.c to come to
agreement that this works.

WORK_STRUCT_PENDING_BIT is set when we queue the work and released just
prior to execution of the work->func. A race on active_requests before
we take the struct_mutex will leave this set.

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

For bonus points

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index cfda095..5d349e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1146,7 +1146,7 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
                engine_retire_requests(engine);
 
        if (!dev_priv->gt.active_requests)
-               queue_delayed_work(dev_priv->wq,
-                                  &dev_priv->gt.idle_work,
-                                  msecs_to_jiffies(100));
+               mod_delayed_work(dev_priv->wq,
+                               &dev_priv->gt.idle_work,
+                               msecs_to_jiffies(100));
 }

-- 
Chris Wilson, Intel Open Source Technology Centre