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

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

[Chris Wilson]

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@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);
 
 		if (engine->irq_seqno_barrier)
 			engine->irq_seqno_barrier(engine);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Kill the tasklet then disable

[Patchwork]

== Series Details ==

Series: drm/i915: Kill the tasklet then disable
URL   : https://patchwork.freedesktop.org/series/19512/
State : success

== Summary ==

Series 19512v1 drm/i915: Kill the tasklet then disable
https://patchwork.freedesktop.org/api/1.0/series/19512/revisions/1/mbox/

fi-bdw-5557u     total:252  pass:241  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:202  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:235  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:224  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:223  dwarn:0   dfail:0   fail:0   skip:29 

4dbd7c0fbb78579ff491ef1184f78087055c5aa5 drm-tip: 2017y-02m-10d-21h-45m-14s UTC integration manifest
d194a1f drm/i915: Kill the tasklet then disable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3779/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Kill the tasklet then disable

[Chris Wilson]

On Sun, Feb 12, 2017 at 10:31:48AM +0000, Chris Wilson wrote:
> 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.

Ok, the loop inside softirq is something else. I think there is still
some merit in using kill to remove the a scheduled tasklet, disable will
leave it on the queue for the next enable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Kill the tasklet then disable

[Mika Kuoppala]

Chris Wilson <chris@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@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?
-Mika

>  		if (engine->irq_seqno_barrier)
>  			engine->irq_seqno_barrier(engine);
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx