Re: [PATCH v4 6/8] drm/xe/madvise: Implement per-VMA purgeable state tracking

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Tue, 2026-01-20 at 09:41 -0800, Matthew Brost wrote:
> On Tue, Jan 20, 2026 at 11:38:52AM +0530, Arvind Yadav wrote:
> > Track purgeable state per-VMA instead of using a coarse shared
> > BO check. This prevents purging shared BOs until all VMAs across
> > all VMs are marked DONTNEED.
> > 
> > Add xe_bo_all_vmas_dontneed() to check all VMAs before marking
> > a BO purgeable. Add xe_bo_recheck_purgeable_on_vma_unbind() to
> > handle state transitions when VMAs are destroyed - if all
> > remaining VMAs are DONTNEED the BO can become purgeable, or if
> > no VMAs remain it transitions to WILLNEED.
> > 
> > The per-VMA purgeable_state field stores the madvise hint for
> > each mapping. Shared BOs can only be purged when all VMAs
> > unanimously indicate DONTNEED.
> > 
> > v3:
> >   - This addresses Thomas Hellström's feedback: "loop over all vmas
> >     attached to the bo and check that they all say WONTNEED. This
> > will
> >     also need a check at VMA unbinding"
> > 
> > v4:
> >   - @madv_purgeable atomic_t → u32 change across all relevant
> > patches. (Matt)
> > 
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_vm.c         | 15 +++++-
> >  drivers/gpu/drm/xe/xe_vm_madvise.c | 84
> > +++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/xe/xe_vm_madvise.h |  3 ++
> >  drivers/gpu/drm/xe/xe_vm_types.h   | 11 ++++
> >  4 files changed, 111 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c
> > b/drivers/gpu/drm/xe/xe_vm.c
> > index f250daae3012..9543960b5613 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -40,6 +40,7 @@
> >  #include "xe_tile.h"
> >  #include "xe_tlb_inval.h"
> >  #include "xe_trace_bo.h"
> > +#include "xe_vm_madvise.h"
> >  #include "xe_wa.h"
> >  
> >  static struct drm_gem_object *xe_vm_obj(struct xe_vm *vm)
> > @@ -1079,12 +1080,18 @@ static struct xe_vma *xe_vma_create(struct
> > xe_vm *vm,
> >  static void xe_vma_destroy_late(struct xe_vma *vma)
> >  {
> >  	struct xe_vm *vm = xe_vma_vm(vma);
> > +	struct xe_bo *bo = NULL;
> >  
> >  	if (vma->ufence) {
> >  		xe_sync_ufence_put(vma->ufence);
> >  		vma->ufence = NULL;
> >  	}
> >  
> > +	/* Get BO reference for purgeable state re-check */
> > +	if (!xe_vma_is_userptr(vma) && !xe_vma_is_null(vma) &&
> > +	    !xe_vma_is_cpu_addr_mirror(vma))
> > +		bo = xe_vma_bo(vma);
> 
> I think xe_vma_bo just returns NULL if any of the above conditions
> are
> met, so I believe is ok to just blindly call xe_vma_bo as you have a
> NULL check on the BO below.
> 
> > +
> >  	if (xe_vma_is_userptr(vma)) {
> >  		struct xe_userptr_vma *uvma = to_userptr_vma(vma);
> >  
> > @@ -1093,7 +1100,13 @@ static void xe_vma_destroy_late(struct
> > xe_vma *vma)
> >  	} else if (xe_vma_is_null(vma) ||
> > xe_vma_is_cpu_addr_mirror(vma)) {
> >  		xe_vm_put(vm);
> >  	} else {
> > -		xe_bo_put(xe_vma_bo(vma));
> > +		/* Trylock safe for async context; madvise
> > corrects failures */
> > +		if (bo && dma_resv_trylock(bo->ttm.base.resv)) {
> > +			xe_bo_recheck_purgeable_on_vma_unbind(bo);
> 
> Also I don't think the correct place to call this.
> 
> I believe you can call this function in xe_vma_destroy after
> drm_gpuva_unlink. You also have BO lock there too so no need from
> this
> trylock path.
> 
> > +			dma_resv_unlock(bo->ttm.base.resv);
> > +		}
> > +
> > +		xe_bo_put(bo);
> >  	}
> >  
> >  	xe_vma_free(vma);
> > diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c
> > b/drivers/gpu/drm/xe/xe_vm_madvise.c
> > index dfeab9e24a09..27b6ad65b314 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> > +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> > @@ -12,6 +12,7 @@
> >  #include "xe_pat.h"
> >  #include "xe_pt.h"
> >  #include "xe_svm.h"
> > +#include "xe_vm.h"
> >  
> >  struct xe_vmas_in_madvise_range {
> >  	u64 addr;
> > @@ -179,6 +180,80 @@ static void madvise_pat_index(struct xe_device
> > *xe, struct xe_vm *vm,
> >  	}
> >  }
> >  
> > +/**
> > + * xe_bo_all_vmas_dontneed() - Check if all VMAs of a BO are
> > marked DONTNEED
> > + * @bo: Buffer object
> > + *
> > + * Check all VMAs across all VMs to determine if BO can be purged.
> > + * Shared BOs require unanimous DONTNEED state from all mappings.
> > + *
> > + * Caller must hold BO dma-resv lock.
> > + *
> > + * Return: true if all VMAs are DONTNEED, false otherwise
> > + */
> > +static bool xe_bo_all_vmas_dontneed(struct xe_bo *bo)
> > +{
> > +	struct drm_gpuvm_bo *vm_bo;
> > +	struct drm_gpuva *gpuva;
> > +	struct drm_gem_object *obj = &bo->ttm.base;
> > +	bool has_vmas = false;
> > +
> > +	dma_resv_assert_held(bo->ttm.base.resv);
> > +
> > +	drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> > +		drm_gpuvm_bo_for_each_va(gpuva, vm_bo) {
> > +			struct xe_vma *vma = gpuva_to_vma(gpuva);
> > +
> > +			has_vmas = true;
> > +
> > +			/* Any non-DONTNEED VMA prevents purging
> > */
> > +			if (READ_ONCE(vma->purgeable_state) !=
> > XE_MADV_PURGEABLE_DONTNEED)
> 
> You don't need the READ_ONCE as purgeable_state is only accessed
> under
> the dma-resv lock, also there isn't a WRITE_ONCE anywhere.
> 
> > +				return false;
> > +		}
> > +	}
> > +
> > +	/* No VMAs means not purgeable */
> 
> No VMAs means it is purgeable, right?

Then it would be purgeable just after creation, even if CPU is
accessing. However I think we should consider the case where a bo is
purgeable and it's single VMA goes away. Then I'd say it's still
purgeable.

I guess this is our last chance to, after discussing for years,
revert to a bo madvise UAPI before the UAPI is set in stone, given the
above corner-cases. I must admit I'm not 100% sure myself, given CPU
madvises are always based on the virtual address map, and we try to
mimic that as much as possible. 

> 
> > +	if (!has_vmas)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * xe_bo_recheck_purgeable_on_vma_unbind() - Re-evaluate BO
> > purgeable state after VMA unbind
> > + * @bo: Buffer object
> > + *
> > + * When a VMA is unbound, re-check if the BO's purgeable state
> > should change.
> > + * Destroyed VMAs may allow the BO to become purgeable if all
> > remaining VMAs
> > + * are DONTNEED, or require transition to WILLNEED if no VMAs
> > remain.
> > + *
> > + * Called from VMA destruction path with BO dma-resv lock held.
> > + */
> > +void xe_bo_recheck_purgeable_on_vma_unbind(struct xe_bo *bo)
> > +{
> > +	if (!bo)
> > +		return;
> > +
> > +	dma_resv_assert_held(bo->ttm.base.resv);
> > +
> > +	/*
> > +	 * Once purged, always purged. Cannot transition back to
> > WILLNEED.
> > +	 * This matches i915 semantics where purged BOs are
> > permanently invalid.
> > +	 */
> > +	if (bo->madv_purgeable == XE_MADV_PURGEABLE_PURGED)
> > +		return;

	Perhaps replace the below with

	bo->madv_purgeable = xe_bo_all_vmas_dontneed(bo) ?
		XE_MADV_PURGEABLE_DONTNEED : 		XE_MADV_PURGEABLE_WILLNEED; 
> > +
> > +	if (xe_bo_all_vmas_dontneed(bo)) {
> > +		/* All VMAs are DONTNEED - mark BO purgeable */
> > +		if (bo->madv_purgeable !=
> > XE_MADV_PURGEABLE_DONTNEED)
> > +			bo->madv_purgeable =
> > XE_MADV_PURGEABLE_DONTNEED;
> > +	} else {
> > +		/* At least one VMA is WILLNEED - BO must not be
> > purgeable */
> > +		if (bo->madv_purgeable !=
> > XE_MADV_PURGEABLE_WILLNEED)
> > +			bo->madv_purgeable =
> > XE_MADV_PURGEABLE_WILLNEED;
> > +	}
> > +}
> > +
> >  /*
> >   * Handle purgeable buffer object advice for
> > DONTNEED/WILLNEED/PURGED.
> >   * Returns true if any BO was purged, false otherwise.
> > @@ -213,10 +288,17 @@ static bool xe_vm_madvise_purgeable_bo(struct
> > xe_device *xe, struct xe_vm *vm,
> >  
> >  		switch (op->purge_state_val.val) {
> >  		case DRM_XE_VMA_PURGEABLE_STATE_WILLNEED:
> > +			vmas[i]->purgeable_state =
> > XE_MADV_PURGEABLE_WILLNEED;
> > +
> > +			/* Mark VMA WILLNEED - BO becomes non-
> > purgeable immediately */
> >  			bo->madv_purgeable =
> > XE_MADV_PURGEABLE_WILLNEED;
> >  			break;
> >  		case DRM_XE_VMA_PURGEABLE_STATE_DONTNEED:
> > -			bo->madv_purgeable =
> > XE_MADV_PURGEABLE_DONTNEED;
> > +			vmas[i]->purgeable_state =
> > XE_MADV_PURGEABLE_DONTNEED;
> > +
> > +			/* Mark BO purgeable only if all VMAs are
> > DONTNEED */
> > +			if (xe_bo_all_vmas_dontneed(bo))
> > +				bo->madv_purgeable =
> > XE_MADV_PURGEABLE_DONTNEED;
> >  			break;
> >  		default:
> >  			drm_warn(&vm->xe->drm, "Invalid madvice
> > value = %d\n",

This is part of the IOCTL processing, right? Then we should use
XE_IOCTL_DBG() on invalid values, or xe_assert() if input values are
already checked elsewhere.


> > diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.h
> > b/drivers/gpu/drm/xe/xe_vm_madvise.h
> > index b0e1fc445f23..61868f851949 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_madvise.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_madvise.h
> > @@ -8,8 +8,11 @@
> >  
> >  struct drm_device;
> >  struct drm_file;
> > +struct xe_bo;
> >  
> >  int xe_vm_madvise_ioctl(struct drm_device *dev, void *data,
> >  			struct drm_file *file);
> >  
> > +void xe_bo_recheck_purgeable_on_vma_unbind(struct xe_bo *bo);
> > +
> >  #endif
> > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h
> > b/drivers/gpu/drm/xe/xe_vm_types.h
> > index 437f64202f3b..94ca9d033b06 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > @@ -150,6 +150,17 @@ struct xe_vma {
> >  	 */
> >  	bool skip_invalidation;
> >  
> > +	/**
> > +	 * @purgeable_state: Purgeable hint for this VMA mapping
> > +	 *
> > +	 * Per-VMA purgeable state from madvise. Valid states are
> > WILLNEED (0)
> > +	 * or DONTNEED (1). Shared BOs require all VMAs to be
> > DONTNEED before
> > +	 * the BO can be purged. PURGED state exists only at BO
> > level.
> > +	 *
> > +	 * Protected by BO dma-resv lock. Set via
> > DRM_IOCTL_XE_MADVISE.
> > +	 */
> > +	u32 purgeable_state;
> > +
> 
> I think xe_vma_mem_attr is a better place for this field.

+1.

/Thomas


> 
> Matt
> 
> >  	/**
> >  	 * @ufence: The user fence that was provided with MAP.
> >  	 * Needs to be signalled before UNMAP can be processed.
> > -- 
> > 2.43.0
> >