Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: Matthew Auld <matthew.auld@intel.com>,
	Matthew Brost <matthew.brost@intel.com>
Cc: intel-xe@lists.freedesktop.org
Subject: Re: [PATCH] drm/xe: Bump LRU for external objects on exec and rebind
Date: Mon, 12 Oct 2026 16:31:29 +0200	[thread overview]
Message-ID: <de000967974d7f6180644fa27e2f6607994963a6.camel@linux.intel.com> (raw)
In-Reply-To: <dd0ad60b-c6a2-45c1-b85d-0368415526da@intel.com>

On Mon, 2026-10-12 at 15:21 +0100, Matthew Auld wrote:
> On 12/10/2026 14:45, Matthew Brost wrote:
> > On Mon, Oct 12, 2026 at 03:26:07PM +0200, Thomas Hellström wrote:
> > 
> > Yikes, not sure how we missed this one and this is certainly not
> > the
> > intent and will be large issue for compositor buffers.
> > 
> > I'd honestly lean towards this being a fixes.
> 
> +1

Thanks for reviewing, Matt & Matt. Yeah I agree totally. Just forgot
about it before submission. I wonder whether I broke this during the
GPUVM helper rewrite. Anyway, I'll dig up the history.

Thanks,
Thomas


> 
> > 
> > > The VM's private BOs get their LRU position refreshed in bulk via
> > > ttm_lru_bulk_move_tail() on vm->lru_bulk_move, both after a
> > > successful
> > > non-LR exec and after the LR preempt-fence rebind worker runs.
> > > External
> > > objects (BOs potentially shared across VMs) are never added to
> > > that
> > > per-VM bulk move list, so their LRU position is never refreshed
> > > by
> > > either path. This makes external objects in active use look more
> > > evictable than they are.
> > > 
> > > Add xe_vm_bump_lru(), which, under a single lru_lock critical
> > > section,
> > > performs the bulk move of the VM's private BOs and bumps each
> > > external
> > > object locked by the drm_exec transaction. Use it from both the
> > > non-LR
> > > exec path and the LR rebind worker, replacing the open-coded bulk
> > > move.
> > > 
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Matthew Brost <matthew.brost@intel.com>
> > 
> > Consider the above:
> > Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> 
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> 
> > 
> > > Assisted-by: LLM
> > > Signed-off-by: Thomas Hellström
> > > <thomas.hellstrom@linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_exec.c |  7 ++-----
> > >   drivers/gpu/drm/xe/xe_vm.c   | 31 ++++++++++++++++++++++++++++-
> > > --
> > >   drivers/gpu/drm/xe/xe_vm.h   |  2 ++
> > >   3 files changed, 32 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_exec.c
> > > b/drivers/gpu/drm/xe/xe_exec.c
> > > index d5293bc33a67..7dc9a0a1f569 100644
> > > --- a/drivers/gpu/drm/xe/xe_exec.c
> > > +++ b/drivers/gpu/drm/xe/xe_exec.c
> > > @@ -346,11 +346,8 @@ int xe_exec_ioctl(struct drm_device *dev,
> > > void *data, struct drm_file *file)
> > >   	xe_sched_job_push(job);
> > >   	xe_vm_reactivate_rebind(vm);
> > >   
> > > -	if (!err && !xe_vm_in_lr_mode(vm)) {
> > > -		spin_lock(&xe->ttm.lru_lock);
> > > -		ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
> > > -		spin_unlock(&xe->ttm.lru_lock);
> > > -	}
> > > +	if (!err && !xe_vm_in_lr_mode(vm))
> > > +		xe_vm_bump_lru(vm, exec);
> > >   
> > >   	if (mode == EXEC_MODE_LR)
> > >   		xe_hw_engine_group_resume_faulting_lr_jobs(group
> > > );
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.c
> > > b/drivers/gpu/drm/xe/xe_vm.c
> > > index ef369df4aa26..095f762bbb6f 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > @@ -413,6 +413,33 @@ int xe_vm_validate_rebind(struct xe_vm *vm,
> > > struct drm_exec *exec,
> > >   	return 0;
> > >   }
> > >   
> > > +/**
> > > + * xe_vm_bump_lru() - Bump the LRU position of BOs used by a
> > > submission
> > > + * @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.
> > > + */
> > > +void xe_vm_bump_lru(struct xe_vm *vm, struct drm_exec *exec)
> > > +{
> > > +	spinlock_t *lru_lock = &vm->xe->ttm.lru_lock;
> > > +	struct drm_gem_object *obj;
> > > +
> > > +	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))
> > > +			continue;
> > > +
> > > +		ttm_bo_move_to_lru_tail(&gem_to_xe_bo(obj)-
> > > >ttm);
> > > +		cond_resched_lock(lru_lock);
> > > +	}
> > > +	spin_unlock(lru_lock);
> > > +}
> > > +
> > >   static int xe_preempt_work_begin(struct drm_exec *exec, struct
> > > xe_vm *vm,
> > >   				 bool *done)
> > >   {
> > > @@ -563,9 +590,7 @@ static void preempt_rebind_work_func(struct
> > > work_struct *w)
> > >   
> > >   #undef retry_required
> > >   
> > > -	spin_lock(&vm->xe->ttm.lru_lock);
> > > -	ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
> > > -	spin_unlock(&vm->xe->ttm.lru_lock);
> > > +	xe_vm_bump_lru(vm, &exec);
> > >   
> > >   	/* Point of no return. */
> > >   	arm_preempt_fences(vm, &preempt_fences);
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.h
> > > b/drivers/gpu/drm/xe/xe_vm.h
> > > index 89c4f95984a2..06138c3c4315 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.h
> > > +++ b/drivers/gpu/drm/xe/xe_vm.h
> > > @@ -284,6 +284,8 @@ int xe_vm_lock_vma(struct drm_exec *exec,
> > > struct xe_vma *vma);
> > >   int xe_vm_validate_rebind(struct xe_vm *vm, struct drm_exec
> > > *exec,
> > >   			  unsigned int num_fences);
> > >   
> > > +void xe_vm_bump_lru(struct xe_vm *vm, struct drm_exec *exec);
> > > +
> > >   struct dma_fence *xe_vm_bind_kernel_bo(struct xe_vm *vm, struct
> > > xe_bo *bo,
> > >   				       struct xe_exec_queue *q,
> > > u64 addr,
> > >   				       enum xe_cache_level
> > > cache_lvl);
> > > -- 
> > > 2.56.0
> > > 

  reply	other threads:[~2026-10-12 14:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-12 13:26 [PATCH] drm/xe: Bump LRU for external objects on exec and rebind Thomas Hellström
2026-10-12 13:34 ` ✓ CI.KUnit: success for " Patchwork
2026-10-12 13:45 ` [PATCH] " Matthew Brost
2026-10-12 14:21   ` Matthew Auld
2026-10-12 14:31     ` Thomas Hellström [this message]
2026-10-12 15:26 ` ✓ Xe.CI.BAT: success for " Patchwork
2026-10-12 17:27 ` ✓ Xe.CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=de000967974d7f6180644fa27e2f6607994963a6.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox