public inbox for intel-xe@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Arvind Yadav <arvind.yadav@intel.com>
Cc: <intel-xe@lists.freedesktop.org>,
	<himal.prasad.ghimiray@intel.com>,
	<thomas.hellstrom@linux.intel.com>, <pallavi.mishra@intel.com>
Subject: Re: [PATCH v4 6/8] drm/xe/madvise: Implement per-VMA purgeable state tracking
Date: Tue, 20 Jan 2026 09:41:22 -0800	[thread overview]
Message-ID: <aW++QuG51rGP1Ru4@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <20260120060900.3137984-7-arvind.yadav@intel.com>

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?

> +	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;
> +
> +	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",
> 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.

Matt

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

  reply	other threads:[~2026-01-20 17:41 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20  6:08 [PATCH v4 0/8] drm/xe/madvise: Add support for purgeable buffer objects Arvind Yadav
2026-01-20  6:08 ` [PATCH v4 1/8] drm/xe/uapi: Add UAPI " Arvind Yadav
2026-01-20 17:20   ` Matthew Brost
2026-01-21 18:42     ` Vivi, Rodrigo
2026-01-20  6:08 ` [PATCH v4 2/8] drm/xe/bo: Add purgeable bo state tracking and field madv to xe_bo Arvind Yadav
2026-01-20 17:45   ` Matthew Brost
2026-01-21  5:30     ` Yadav, Arvind
2026-01-22 15:05     ` Thomas Hellström
2026-01-20  6:08 ` [PATCH v4 3/8] drm/xe/madvise: Implement purgeable buffer object support Arvind Yadav
2026-01-20 16:58   ` Matthew Brost
2026-01-20 17:15     ` Matthew Brost
2026-01-21  8:24       ` Yadav, Arvind
2026-01-22 15:30     ` Thomas Hellström
2026-01-30  8:13       ` Yadav, Arvind
2026-01-20 17:44   ` Matthew Brost
2026-01-20  6:08 ` [PATCH v4 4/8] drm/xe/bo: Handle CPU faults on purged buffer objects Arvind Yadav
2026-01-20 17:23   ` Matthew Brost
2026-01-22 15:54   ` Thomas Hellström
2026-01-20  6:08 ` [PATCH v4 5/8] drm/xe/vm: Prevent binding of " Arvind Yadav
2026-01-20 17:27   ` Matthew Brost
2026-01-23  5:41     ` Yadav, Arvind
2026-01-23 12:37       ` Thomas Hellström
2026-01-30  8:17         ` Yadav, Arvind
2026-01-20  6:08 ` [PATCH v4 6/8] drm/xe/madvise: Implement per-VMA purgeable state tracking Arvind Yadav
2026-01-20 17:41   ` Matthew Brost [this message]
2026-01-21  5:11     ` Yadav, Arvind
2026-01-23 13:07     ` Thomas Hellström
2026-01-20  6:08 ` [PATCH v4 7/8] drm/xe/madvise: Block imported and exported dma-bufs Arvind Yadav
2026-01-20 17:51   ` Matthew Brost
2026-01-23 13:31     ` Thomas Hellström
2026-01-30  8:22       ` Yadav, Arvind
2026-01-30  8:59         ` Thomas Hellström
2026-01-20  6:08 ` [PATCH v4 8/8] drm/xe/bo: Add purgeable shrinker state helpers Arvind Yadav
2026-01-20 17:58   ` Matthew Brost
2026-01-23 13:42     ` Thomas Hellström
2026-01-20  6:14 ` ✗ CI.checkpatch: warning for drm/xe/madvise: Add support for purgeable buffer objects (rev5) Patchwork
2026-01-20  6:16 ` ✗ CI.KUnit: failure " 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=aW++QuG51rGP1Ru4@lstrano-desk.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=arvind.yadav@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=pallavi.mishra@intel.com \
    --cc=thomas.hellstrom@linux.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