[Intel-gfx] [PATCH] drm/i915: Kill the tasklet then disable

[Chris Wilson]

On Mon, Feb 13, 2017 at 12:00:13PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Disabling the tasklet leaves it if scheduled on the ready to run list
> > until it is re-enabled. This will leave the ksoftird thread spinning
> > until satisfied. To prevent this situation on starting the GPU reset, we
> > want to kill the tasklet first and then disable. The same problem will
> > arise when a tasklet is scheduled from another device, so a better
> > solution is required for the general case.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 23a17f31aa06..a8805409a3e7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2689,8 +2689,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
> >  		 * Turning off the engine->irq_tasklet until the reset is over
> >  		 * prevents the race.
> >  		 */
> > -		tasklet_disable(&engine->irq_tasklet);
> >  		tasklet_kill(&engine->irq_tasklet);
> > +		tasklet_disable(&engine->irq_tasklet);
> >
> 
> With this order, do we need disable at all?

Yes. We still need the disable because a fence may be completed on
another engine that triggers the tasklet on this engine. Just killing
the existing tasklet invocation is not enough to prevent two threads
from concurrently writing to ELSP and port[] (i.e. the tasklet and
init_hw).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre