On Fri, Aug 11, 2023 at 11:20:11AM -0700, Zhanjun Dong wrote:
> This attempts to avoid circular locking dependency between flush delayed
> work and intel_gt_reset.
> When intel_gt_reset was called, task will hold a lock.
> To cacel delayed work here, the _sync version will also acquire a lock,
> which might trigger the possible cirular locking dependency warning.
> When intel_gt_reset called, reset_in_progress flag will be set, add code
> to check the flag, call async verion if reset is in progress.
> 
> Signed-off-by: Zhanjun Dong <zhanjun.d...@intel.com>
> Cc: John Harrison <john.c.harri...@intel.com>
> Cc: Andi Shyti <andi.sh...@linux.intel.com>
> Cc: Daniel Vetter <dan...@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index a0e3ef1c65d2..600388c849f7 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1359,7 +1359,16 @@ static void guc_enable_busyness_worker(struct 
> intel_guc *guc)
>  
>  static void guc_cancel_busyness_worker(struct intel_guc *guc)
>  {
> -     cancel_delayed_work_sync(&guc->timestamp.work);
> +     /*
> +      * When intel_gt_reset was called, task will hold a lock.
> +      * To cacel delayed work here, the _sync version will also acquire a 
> lock, which might
> +      * trigger the possible cirular locking dependency warning.

This is not even close to a locking bugfix. Consider this a formal nack,
because the issue here is not even close to "needs more comments to
explain what's going on".
-Daniel

> +      * Check the reset_in_progress flag, call async verion if reset is in 
> progress.
> +      */
> +     if (guc_to_gt(guc)->uc.reset_in_progress)
> +             cancel_delayed_work(&guc->timestamp.work);
> +     else
> +             cancel_delayed_work_sync(&guc->timestamp.work);
>  }
>  
>  static void __reset_guc_busyness_stats(struct intel_guc *guc)
> -- 
> 2.34.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Reply via email to