Re: [PATCH v7 11/32] drm/xe: Move GGTT lock init to alloc

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

On 10/7/2025 1:26 PM, Matthew Brost wrote:
> The GGTT lock is needed very early during GT initialization for a VF;
> move the GGTT lock initialization to the allocation phase.
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 7fdd0a97a628..f7717057cd14 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -159,6 +159,16 @@ static void xe_ggtt_clear(struct xe_ggtt *ggtt, u64 start, u64 size)
>  	}
>  }
>  
> +static void primelockdep(struct xe_ggtt *ggtt)
> +{
> +	if (!IS_ENABLED(CONFIG_LOCKDEP))
> +		return;
> +
> +	fs_reclaim_acquire(GFP_KERNEL);
> +	might_lock(&ggtt->lock);
> +	fs_reclaim_release(GFP_KERNEL);
> +}
> +
>  /**
>   * xe_ggtt_alloc - Allocate a GGTT for a given &xe_tile
>   * @tile: &xe_tile
> @@ -170,8 +180,19 @@ static void xe_ggtt_clear(struct xe_ggtt *ggtt, u64 start, u64 size)
>  struct xe_ggtt *xe_ggtt_alloc(struct xe_tile *tile)
>  {
>  	struct xe_ggtt *ggtt = drmm_kzalloc(&tile_to_xe(tile)->drm, sizeof(*ggtt), GFP_KERNEL);
> -	if (ggtt)
> +
> +	if (ggtt) {
> +		int err;
> +
>  		ggtt->tile = tile;
> +
> +		err = drmm_mutex_init(&tile_to_xe(tile)->drm, &ggtt->lock);
> +		if (err)
> +			ggtt = NULL;
> +		else
> +			primelockdep(ggtt);
> +	}
> +

all this looks over-complicated and against the rule to exit early on error, so maybe:

	struct xe_device *xe = tile_to_xe(tile);
	struct xe_ggtt *ggtt;

	ggtt = drmm_kzalloc(&xe->drm, sizeof(*ggtt), GFP_KERNEL);
	if (!ggtt)
		return NULL;
	if (drmm_mutex_init(&xe->drm, &ggtt->lock))
		return NULL;
	if (IS_ENABLED(CONFIG_LOCKDEP))
		primelockdep(ggtt);

	ggtt->tile = tile;	

>  	return ggtt;
>  }
>  
> @@ -180,7 +201,6 @@ static void ggtt_fini_early(struct drm_device *drm, void *arg)
>  	struct xe_ggtt *ggtt = arg;
>  
>  	destroy_workqueue(ggtt->wq);
> -	mutex_destroy(&ggtt->lock);
>  	drm_mm_takedown(&ggtt->mm);
>  }
>  
> @@ -198,16 +218,6 @@ void xe_ggtt_might_lock(struct xe_ggtt *ggtt)
>  }
>  #endif
>  
> -static void primelockdep(struct xe_ggtt *ggtt)
> -{
> -	if (!IS_ENABLED(CONFIG_LOCKDEP))
> -		return;
> -
> -	fs_reclaim_acquire(GFP_KERNEL);
> -	might_lock(&ggtt->lock);
> -	fs_reclaim_release(GFP_KERNEL);
> -}
> -
>  static const struct xe_ggtt_pt_ops xelp_pt_ops = {
>  	.pte_encode_flags = xelp_ggtt_pte_flags,
>  	.ggtt_set_pte = xe_ggtt_set_pte,
> @@ -227,8 +237,6 @@ static void __xe_ggtt_init_early(struct xe_ggtt *ggtt, u32 reserved)
>  {
>  	drm_mm_init(&ggtt->mm, reserved,
>  		    ggtt->size - reserved);
> -	mutex_init(&ggtt->lock);
> -	primelockdep(ggtt);
>  }
>  
>  int xe_ggtt_init_kunit(struct xe_ggtt *ggtt, u32 reserved, u32 size)