All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.