Re: [PATCH v3 1/6] drm/xe: Only have a single drmm release action.

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 8/19/2025 12:11 PM, Maarten Lankhorst wrote:
> The broken action happened after ggtt_early_fini, so
> it's safe to put the drain_workqueue in there instead of
> creating a new place.

can you share a log with that broken action?

the whole point of having separate devm action here was to
make sure that we will not have any pending work behind
left for the drmm unwind phase, as devm actions, where we
unmap GGTT, will be run before drmm unwind phase

> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> Fixes: 89d2835c3680 ("drm/xe: Process deferred GGTT node removals on device unwind")
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index e03222f5ac5a1..0c6aa3126b43a 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -178,6 +178,7 @@ static void ggtt_fini_early(struct drm_device *drm, void *arg)
>  {
>  	struct xe_ggtt *ggtt = arg;
>  
> +	drain_workqueue(ggtt->wq);

btw, that's redundant since drain_workqueue() is called
implicitly by the below destroy_workqueue()

>  	destroy_workqueue(ggtt->wq);
>  	mutex_destroy(&ggtt->lock);
>  	drm_mm_takedown(&ggtt->mm);
> @@ -238,13 +239,6 @@ int xe_ggtt_init_kunit(struct xe_ggtt *ggtt, u32 reserved, u32 size)
>  }
>  EXPORT_SYMBOL_IF_KUNIT(xe_ggtt_init_kunit);
>  
> -static void dev_fini_ggtt(void *arg)
> -{
> -	struct xe_ggtt *ggtt = arg;
> -
> -	drain_workqueue(ggtt->wq);
> -}
> -
>  /**
>   * xe_ggtt_init_early - Early GGTT initialization
>   * @ggtt: the &xe_ggtt to be initialized
> @@ -297,10 +291,6 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  	if (err)
>  		return err;
>  
> -	err = devm_add_action_or_reset(xe->drm.dev, dev_fini_ggtt, ggtt);
> -	if (err)
> -		return err;
> -
>  	if (IS_SRIOV_VF(xe)) {
>  		err = xe_tile_sriov_vf_prepare_ggtt(ggtt->tile);
>  		if (err)