[PATCH 01/15] drm/xe/vm: Don't use a pin the vm_resv during validation

[Matthew Brost]

On Wed, Aug 13, 2025 at 04:33:01PM +0200, Thomas Hellström wrote:
> On Wed, 2025-08-13 at 07:28 -0700, Matthew Brost wrote:
> > On Wed, Aug 13, 2025 at 12:51:07PM +0200, Thomas Hellström wrote:
> > > The pinning has the odd side-effect that unlocking *any* resv
> > > during validation triggers an "unlocking pinned lock" warning.
> > > 
> > 
> > So this is a cross process thing then - right? e.g., Process A pins a
> > dma-resv lock, Process B unlock a dma-resv lock and boom lockdep
> > warning? Just want to make sure I am understandinf the problem
> > correctly.
> 
> No, my understanding is that this is a single process thing.
> We lock the vm, pin it, lock a bo for eviction, perhaps using the same
> ww_mutex_ctx, unlock the evicted bo => bang.
> 

Ah, ok. Got it, makes sense how this can occuring within a single
process if eviction is triggered.

> It might be that the locks need to be locked using the same ww_mutex
> context, but not sure.
>

I'd guess lockdep_pin_lock operates on a lockdep class, all dma-resv use
the same class which creates this issue.

At any rate, this extra assert isn't really needed so I don't see an
issue removing this.

Reviewed-by: Matthew Brost <matthew.brost at intel.com>

> /Thomas
> 
> 
> > 
> > Matt 
> > 
> > > Cc: Matthew Brost <matthew.brost at intel.com>
> > > Fixes: 9d5558649f68 ("drm/xe: Rework eviction rejection of bound
> > > external bos")
> > > Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_bo.c |  5 ++---
> > >  drivers/gpu/drm/xe/xe_vm.h | 15 ++-------------
> > >  2 files changed, 4 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_bo.c
> > > b/drivers/gpu/drm/xe/xe_bo.c
> > > index 6fea39842e1e..11eaf3b06766 100644
> > > --- a/drivers/gpu/drm/xe/xe_bo.c
> > > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > > @@ -2468,7 +2468,6 @@ int xe_bo_validate(struct xe_bo *bo, struct
> > > xe_vm *vm, bool allow_res_evict)
> > >  		.no_wait_gpu = false,
> > >  		.gfp_retry_mayfail = true,
> > >  	};
> > > -	struct pin_cookie cookie;
> > >  	int ret;
> > >  
> > >  	if (vm) {
> > > @@ -2479,10 +2478,10 @@ int xe_bo_validate(struct xe_bo *bo, struct
> > > xe_vm *vm, bool allow_res_evict)
> > >  		ctx.resv = xe_vm_resv(vm);
> > >  	}
> > >  
> > > -	cookie = xe_vm_set_validating(vm, allow_res_evict);
> > > +	xe_vm_set_validating(vm, allow_res_evict);
> > >  	trace_xe_bo_validate(bo);
> > >  	ret = ttm_bo_validate(&bo->ttm, &bo->placement, &ctx);
> > > -	xe_vm_clear_validating(vm, allow_res_evict, cookie);
> > > +	xe_vm_clear_validating(vm, allow_res_evict);
> > >  
> > >  	return ret;
> > >  }
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.h
> > > b/drivers/gpu/drm/xe/xe_vm.h
> > > index 2f213737c7e5..2ecb417c19a2 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.h
> > > +++ b/drivers/gpu/drm/xe/xe_vm.h
> > > @@ -315,22 +315,14 @@ void xe_vm_snapshot_free(struct
> > > xe_vm_snapshot *snap);
> > >   * Register this task as currently making bos resident for the vm.
> > > Intended
> > >   * to avoid eviction by the same task of shared bos bound to the
> > > vm.
> > >   * Call with the vm's resv lock held.
> > > - *
> > > - * Return: A pin cookie that should be used for
> > > xe_vm_clear_validating().
> > >   */
> > > -static inline struct pin_cookie xe_vm_set_validating(struct xe_vm
> > > *vm,
> > > -						     bool
> > > allow_res_evict)
> > > +static inline void xe_vm_set_validating(struct xe_vm *vm, bool
> > > allow_res_evict)
> > >  {
> > > -	struct pin_cookie cookie = {};
> > > -
> > >  	if (vm && !allow_res_evict) {
> > >  		xe_vm_assert_held(vm);
> > > -		cookie = lockdep_pin_lock(&xe_vm_resv(vm)-
> > > >lock.base);
> > >  		/* Pairs with READ_ONCE in xe_vm_is_validating()
> > > */
> > >  		WRITE_ONCE(vm->validating, current);
> > >  	}
> > > -
> > > -	return cookie;
> > >  }
> > >  
> > >  /**
> > > @@ -338,17 +330,14 @@ static inline struct pin_cookie
> > > xe_vm_set_validating(struct xe_vm *vm,
> > >   * @vm: Pointer to the vm or NULL
> > >   * @allow_res_evict: Eviction from @vm was allowed. Must be set to
> > > the same
> > >   * value as for xe_vm_set_validation().
> > > - * @cookie: Cookie obtained from xe_vm_set_validating().
> > >   *
> > >   * Register this task as currently making bos resident for the vm.
> > > Intended
> > >   * to avoid eviction by the same task of shared bos bound to the
> > > vm.
> > >   * Call with the vm's resv lock held.
> > >   */
> > > -static inline void xe_vm_clear_validating(struct xe_vm *vm, bool
> > > allow_res_evict,
> > > -					  struct pin_cookie
> > > cookie)
> > > +static inline void xe_vm_clear_validating(struct xe_vm *vm, bool
> > > allow_res_evict)
> > >  {
> > >  	if (vm && !allow_res_evict) {
> > > -		lockdep_unpin_lock(&xe_vm_resv(vm)->lock.base,
> > > cookie);
> > >  		/* Pairs with READ_ONCE in xe_vm_is_validating()
> > > */
> > >  		WRITE_ONCE(vm->validating, NULL);
> > >  	}
> > > -- 
> > > 2.50.1
> > > 
>