[PATCH] drm/xe/xe_sync: Add debug printing to check_ufence

[Cavitt, Jonathan]

-----Original Message-----
From: Vivi, Rodrigo <rodrigo.vivi at intel.com> 
Sent: Friday, December 6, 2024 7:26 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
Cc: intel-xe at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Zuo, Alex <alex.zuo at intel.com>
Subject: Re: [PATCH] drm/xe/xe_sync: Add debug printing to check_ufence
> 
> On Mon, Dec 02, 2024 at 07:46:31PM +0000, Jonathan Cavitt wrote:
> > The xe_sync helper function check_ufence can occasionally report EBUSY
> > if the ufence has not been signalled yet.  EBUSY is a non-fatal error
> > value for the function, so it is not desireable to warn in cases where
> > EBUSY is reported because it is up to the user to decide if EBUSY is a
> > fatal error in their use cases.  However, we can and should report EBUSY
> > to the debug logs for diagnostic purposes.
> > 
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_vm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 610226c7c1ce..09ab7111e091 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -2400,7 +2400,7 @@ static int op_lock_and_prep(struct drm_exec *exec, struct xe_vm *vm,
> >  		break;
> >  	case DRM_GPUVA_OP_REMAP:
> >  		err = check_ufence(gpuva_to_vma(op->base.remap.unmap->va));
> > -		if (err)
> > +		if (XE_IOCTL_DBG(vm->xe, err))
> 
> this macro is good for ioctl arguments check, not for errors dbg during ioctl
> path...
> 
> If really needed and not in a very critical latency path 

It's not "strictly" necessary, but it would help with some diagnostics and shouldn't
incur any noticeable overhead, especially in the non-error cases.

> then we should add the dbg call directly I believe...

Understood.  I'll use vm_dbg instead.
-Jonathan Cavitt

> 
> >  			break;
> >  
> >  		err = vma_lock_and_validate(exec,
> > @@ -2413,7 +2413,7 @@ static int op_lock_and_prep(struct drm_exec *exec, struct xe_vm *vm,
> >  		break;
> >  	case DRM_GPUVA_OP_UNMAP:
> >  		err = check_ufence(gpuva_to_vma(op->base.unmap.va));
> > -		if (err)
> > +		if (XE_IOCTL_DBG(vm->xe, err))
> >  			break;
> >  
> >  		err = vma_lock_and_validate(exec,
> > -- 
> > 2.43.0
> > 
>