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

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

On 10/10/2025 2:06 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.
> 
> 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>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  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 aca7ae5489b91..33b09737ccba8 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -199,6 +199,7 @@ static void ggtt_fini_early(struct drm_device *drm, void *arg)
>  {
>  	struct xe_ggtt *ggtt = arg;
>  
> +	drain_workqueue(ggtt->wq);

as said earlier, drain() will be implicitly called by the below destroy() so moving it here is a nop 

and in some cases (at least in the past) the draining here, in "drmm" action, was just too late,
and that's why drain() was moved to separate "devm" action that was called while required devm data was still available

if this is broken now, please attach dmesg/crash log to better understand the case 

>  	destroy_workqueue(ggtt->wq);
>  	drm_mm_takedown(&ggtt->mm);
>  }
> @@ -246,13 +247,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
> @@ -305,10 +299,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)