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

[Zeng, Oak]

> -----Original Message-----
> From: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Sent: November 29, 2024 3:37 AM
> To: Zeng, Oak <oak.zeng at intel.com>; intel-xe at lists.freedesktop.org
> Subject: Re: [PATCH] drm/xe: unlock object in an error condition
> 
> 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 at 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.

I see. Thanks for the explanation. 

Oak

> 
> 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)