[Intel-xe] [PATCH] drm/xe: Fix error paths of __xe_bo_create_locked

[Matthew Brost]

On Tue, Jul 25, 2023 at 05:12:39PM +0200, Maarten Lankhorst wrote:
> ttm_bo_init_reserved() calls the destroy() callback if it fails.
> 
> Because of this, __xe_bo_create_locked is required to be responsible
> for freeing the bo even when it's passed in as argument.
> 
> Additionally, if the placement check fails, the bo was kept alive.
> Fix it too.
> 
> Reported-by: Oded Gabbay <ogabbay at kernel.org>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index e240df6c7ae64..b63290540dd9b 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1158,8 +1158,10 @@ struct xe_bo *__xe_bo_create_locked(struct xe_device *xe, struct xe_bo *bo,
>  	/* Only kernel objects should set GT */
>  	XE_BUG_ON(tile && type != ttm_bo_type_kernel);
>  
> -	if (XE_WARN_ON(!size))
> +	if (XE_WARN_ON(!size)) {
> +		xe_bo_free(bo);
>  		return ERR_PTR(-EINVAL);
> +	}
>  
>  	if (!bo) {
>  		bo = xe_bo_alloc();
> @@ -1197,8 +1199,10 @@ struct xe_bo *__xe_bo_create_locked(struct xe_device *xe, struct xe_bo *bo,
>  
>  	if (!(flags & XE_BO_FIXED_PLACEMENT_BIT)) {
>  		err = __xe_bo_placement_for_flags(xe, bo, bo->flags);
> -		if (WARN_ON(err))
> +		if (WARN_ON(err)) {
> +			xe_ttm_bo_destroy(&bo->ttm);

This doesn't look right as we haven't fully initialized the BO. An
example of this is the snippet from xe_ttm_bo_destroy:

1012         if (bo->vm && xe_bo_is_user(bo))
1013                 xe_vm_put(bo->vm);

This shouldn't be executed in this error path.

Also do we need to do a cleanup if ttm_bo_init_reserved fails?

Matt

>  			return ERR_PTR(err);
> +		}
>  	}
>  
>  	/* Defer populating type_sg bos */
> -- 
> 2.39.2
>