Re: [PATCH v5 2/8] drm/xe/hw_error: Initialize hw_error_work per tile

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/xe/hw_error: check devm_add_action_or_reset() in
  xe_hw_error_init()
--

commit 507eb21a794f9cbaaf41a0422e6f54d0d752cfac
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/hw_error: Initialize hw_error_work per tile

This commit initializes tile->hw_error_work across all tiles instead of
only the root tile and registers a devm action to disable the work items on
teardown.

> diff --git a/drivers/gpu/drm/xe/xe_hw_error.c b/drivers/gpu/drm/xe/xe_hw_error.c
> index a934b649575c7..ef41bd24ece0c 100644
> --- a/drivers/gpu/drm/xe/xe_hw_error.c
> +++ b/drivers/gpu/drm/xe/xe_hw_error.c
> @@ -537,6 +537,16 @@ static void process_hw_errors(struct xe_device *xe)
>  	}
>  }
>  
> +static void hw_error_fini(void *arg)
> +{
> +	struct xe_device *xe = arg;
> +	struct xe_tile *tile;
> +	u8 id;
> +
> +	for_each_tile(tile, xe, id)
> +		disable_work_sync(&tile->hw_error_work);
> +}
> +
>  /**
>   * xe_hw_error_init - Initialize hw errors
>   * @xe: xe device instance
> @@ -546,12 +556,15 @@ static void process_hw_errors(struct xe_device *xe)
>   */
>  void xe_hw_error_init(struct xe_device *xe)
>  {
> -	struct xe_tile *tile = xe_device_get_root_tile(xe);
> +	struct xe_tile *tile;
> +	u8 id;
>  
>  	if (!IS_DGFX(xe) || IS_SRIOV_VF(xe))
>  		return;
>  
> -	INIT_WORK(&tile->hw_error_work, hw_error_work);
> +	for_each_tile(tile, xe, id)
> +		INIT_WORK(&tile->hw_error_work, hw_error_work);
>  
>  	process_hw_errors(xe);
> +	devm_add_action_or_reset(xe->drm.dev, hw_error_fini, xe);
>  }

[Severity: Medium]
As reported by Andi Shyti on v2 [1], should xe_hw_error_init() return int
and propagate the return value of devm_add_action_or_reset() to
xe_irq_install()?

If devm_add_action_or_reset() fails with -ENOMEM, it immediately invokes
hw_error_fini(), which calls disable_work_sync() on every
tile->hw_error_work.  Because the error is ignored, driver probe succeeds
with all per-tile hw_error_work items permanently disabled, so subsequent
hardware error interrupts calling schedule_work(&tile->hw_error_work) will
silently fail to queue hw_error_work().

[1] https://lore.kernel.org/r/auIW6DsRRyZ0GWnz@zenone.zhora.eu

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112060701.3327125-10-riana.tauro@intel.com?part=2