Re: [PATCH 02/18] drm/i915/gt: Mark up the nested engine-pm timeline lock as irqsafe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:

> We use a fake timeline->mutex lock to reassure lockdep that the timeline
> is always locked when emitting requests. However, the use inside
> __engine_park() may be inside hardirq and so lockdep now complains about
> the mixed irq-state of the nested locked. Disable irqs around the
> lockdep tracking to keep it happy.
>
> Fixes: 6c69a45445af ("drm/i915/gt: Mark context->active_count as protected by timeline->mutex")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index 5f03f7dcad72..5d003751968b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -37,9 +37,15 @@ static int __engine_unpark(struct intel_wakeref *wf)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_LOCKDEP)
> +
>  static inline void __timeline_mark_lock(struct intel_context *ce)
>  {
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
>  	mutex_acquire(&ce->timeline->mutex.dep_map, 2, 0, _THIS_IP_);
> +	local_irq_restore(flags);

I am starting to have second thoughts. One could argue that the
net effect is on positive side.

But we diverge on behaviour now.

Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>

>  }
>  
>  static inline void __timeline_mark_unlock(struct intel_context *ce)
> @@ -47,6 +53,18 @@ static inline void __timeline_mark_unlock(struct intel_context *ce)
>  	mutex_release(&ce->timeline->mutex.dep_map, 0, _THIS_IP_);
>  }
>  
> +#else
> +
> +static inline void __timeline_mark_lock(struct intel_context *ce)
> +{
> +}
> +
> +static inline void __timeline_mark_unlock(struct intel_context *ce)
> +{
> +}
> +
> +#endif
> +
>  static bool switch_to_kernel_context(struct intel_engine_cs *engine)
>  {
>  	struct i915_request *rq;
> -- 
> 2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux