Re: [PATCH 2/7] drm/i915: Only sync tasklets once for recursive reset preparation

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

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

> When setting up reset, we may need to recursively prepare an engine. In
> which case we should only synchronously flush the tasklets on the outer
> most call, the inner calls will then be inside an atomic section where
> the tasklet will never be run (and so the sync will never complete).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
>  drivers/gpu/drm/i915/i915_gem.h | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0dc369a9ec4d..aab1f5e77ae9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3036,7 +3036,7 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine)
>  	 * Turning off the execlists->tasklet until the reset is over
>  	 * prevents the race.
>  	 */
> -	tasklet_disable(&engine->execlists.tasklet);
> +	__tasklet_disable_once(&engine->execlists.tasklet);

It is debatable that could the naming be improved as
the 'once' is tied to the disable now.

__tasklet_disable_wait_[once|first].

I was going to insist on adding a comment tho,
but there it is already above, explaining
the recursion from set wedged.

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

>  
>  	/*
>  	 * We're using worker to queue preemption requests from the tasklet in
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 525920404ede..af033e7c1c56 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -26,6 +26,7 @@
>  #define __I915_GEM_H__
>  
>  #include <linux/bug.h>
> +#include <linux/interrupt.h>
>  
>  struct drm_i915_private;
>  
> @@ -72,4 +73,10 @@ struct drm_i915_private;
>  void i915_gem_park(struct drm_i915_private *i915);
>  void i915_gem_unpark(struct drm_i915_private *i915);
>  
> +static inline void __tasklet_disable_once(struct tasklet_struct *t)
> +{
> +	if (atomic_inc_return(&t->count) == 1)
> +		tasklet_unlock_wait(t);
> +}
> +
>  #endif /* __I915_GEM_H__ */
> -- 
> 2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx