Re: [PATCH v4 1/5] drm/xe: Bump the LRU of every BO an exec touches

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Oct 27, 2026 at 06:59:16PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/xe: xe_vm_bump_lru() never visits private BOs in drm_exec loop
> --
> 
> commit 035844b594106f3b21b07d586a5574fd0b1e0af5
> Author: Matthew Brost <matthew.brost@intel.com>
> 
> drm/xe: Bump the LRU of every BO an exec touches
> 
> This commit updates xe_vm_bump_lru() to replace the drm_gpuvm_is_extobj()
> check in its drm_exec_for_each_locked_object() loop with explicit checks
> skipping xe_vm_obj(vm) and BOs whose bulk_move matches vm->lru_bulk_move,
> intending to bump non-bulk-moved private BOs such as those in fault-mode VMs.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 1ee72721b2ce3..2b84721804a77 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -426,10 +426,8 @@ int xe_vm_validate_rebind(struct xe_vm *vm, struct drm_exec *exec,
> >   * @vm: The vm for which we are bumping LRUs.
> >   * @exec: The struct drm_exec with the locked GEM objects.
> >   *
> > - * Move the VM's private BOs, and all external objects locked by @exec,
> > - * to the LRU tail. Private BOs are moved in bulk using @vm's
> > - * &xe_vm.lru_bulk_move. External objects, which may be shared with other
> > - * VMs, are not part of that bulk move and are bumped individually.
> > + * Move all BOs locked by @exec to the LRU tail. Those covered by @vm's
> > + * &xe_vm.lru_bulk_move are moved in bulk, the rest individually.
> >   */
> >  void xe_vm_bump_lru(struct xe_vm *vm, struct drm_exec *exec)
> >  {
> > @@ -439,10 +437,17 @@ void xe_vm_bump_lru(struct xe_vm *vm, struct drm_exec *exec)
> >  	spin_lock(lru_lock);
> >  	ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
> >  	drm_exec_for_each_locked_object(exec, obj) {
> > -		if (!drm_gpuvm_is_extobj(&vm->gpuvm, obj))
> > +		struct xe_bo *bo;
> > +
> > +		/* The VM's dummy resv object, not a BO. */
> > +		if (obj == xe_vm_obj(vm))
> > +			continue;
> > +
> > +		bo = gem_to_xe_bo(obj);
> > +		if (bo->ttm.bulk_move == &vm->lru_bulk_move)
> >  			continue;
> >  
> > -		ttm_bo_move_to_lru_tail(&gem_to_xe_bo(obj)->ttm);
> > +		ttm_bo_move_to_lru_tail(&bo->ttm);
> 
> [Severity: Medium]
> Can drm_exec_for_each_locked_object() in xe_vm_bump_lru() ever visit a
> private BO of the VM?
> 
> In drivers/gpu/drm/xe/xe_bo.c:__xe_bo_create_locked(), private BOs (where
> vm is non-null) are initialized with xe_vm_resv(vm) as their reservation
> object:
> 
> 	bo = xe_bo_init_locked(xe, bo, tile, vm ? xe_vm_resv(vm) : NULL,
> 			       vm && !xe_vm_in_fault_mode(vm) &&
> 			       flags & XE_BO_FLAG_USER ?
> 			       &vm->lru_bulk_move : NULL, size,
> 			       cpu_caching, type, flags, NULL, exec);
> 
> Because private BOs share the VM's dma_resv, drm_gpuvm_is_extobj() is false
> and they are never added to gpuvm->extobj.list.
> 
> When callers prepare the drm_exec context before calling xe_vm_bump_lru()
> (via xe_validation_exec_lock() -> drm_gpuvm_exec_lock() in xe_exec_ioctl()
> or xe_preempt_work_begin() in preempt_rebind_work_func()),
> drm_gpuvm_prepare_vm() only locks xe_vm_obj(vm) and
> drm_gpuvm_prepare_objects_locked() only locks external objects from
> gpuvm->extobj.list:
> 
> 	drm_gpuvm_resv_assert_held(gpuvm);
> 	list_for_each_entry(vm_bo, &gpuvm->extobj.list, list.entry.extobj) {
> 		if (drm_gpuvm_bo_is_zombie(vm_bo))
> 			continue;
> 
> 		ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
> 
> In addition, even when the follow-up commit a6d00986d9f1 ("drm/xe: Bump the
> LRU on a successful page fault") calls xe_vm_bump_lru() from
> xe_pagefault_handle_vma(), xe_pagefault_begin() -> xe_vm_lock_vma() only
> locks xe_vm_obj(vm) and external BOs:
> 
> 	err = drm_exec_lock_obj(exec, xe_vm_obj(vm));
> 	if (!err && bo && !bo->vm)
> 		err = drm_exec_lock_obj(exec, &bo->ttm.base);
> 
> Since private BOs are never recorded in exec->objects, the loop in
> xe_vm_bump_lru() only ever visits xe_vm_obj(vm) and external BOs (where
> bo->vm and bo->ttm.bulk_move are always null). Does this mean the
> bo->ttm.bulk_move == &vm->lru_bulk_move check is unreachable and private BOs
> without a bulk_move (such as fault-mode private BOs or private BOs detached
> from bulk_move in xe_gem_object_close()) still never get their LRU bumped?
>

I think this is right. Let me drop this patch + adjust page fault patch
to just direcly call ttm_bo_move_to_lru_tail.

Matt

> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261027183638.1031299-1-matthew.brost@intel.com?part=1