Re: [PATCH] drm/xe: unlock object in an error condition

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi, Oak,


On Thu, 2024-11-28 at 16:34 -0500, Oak Zeng wrote:
> In xe_gem_fault, we reserved a buffer object but didn't unreserve
> it under an error condition. Unreserve it.
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index f51d86511cb9e..634a1b54ca60d 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1286,7 +1286,7 @@ static vm_fault_t xe_gem_fault(struct vm_fault
> *vmf)
>  	}
>  
>  	if (ret == VM_FAULT_RETRY && !(vmf->flags &
> FAULT_FLAG_RETRY_NOWAIT))
> -		goto out;
> +		goto unlock;

This change is not correct. On fault retries, the dma_resv lock and the
mmap lock is already dropped by the ttm helpers. This weird construct
is to allow wait for io outside wide locks, and then restart the fault.

So this patch can be dropped.

Thanks,
Thomas




>  	/*
>  	 * ttm_bo_vm_reserve() already has dma_resv_lock.
>  	 */
> @@ -1297,6 +1297,7 @@ static vm_fault_t xe_gem_fault(struct vm_fault
> *vmf)
>  		mutex_unlock(&xe->mem_access.vram_userfault.lock);
>  	}
>  
> +unlock:
>  	dma_resv_unlock(tbo->base.resv);
>  out:
>  	if (needs_rpm)