Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: Arvind Yadav <arvind.yadav@intel.com>, intel-xe@lists.freedesktop.org
Cc: matthew.brost@intel.com, himal.prasad.ghimiray@intel.com,
	 pallavi.mishra@intel.com
Subject: Re: [PATCH v5 6/9] drm/xe/madvise: Implement per-VMA purgeable state tracking
Date: Tue, 24 Feb 2026 13:48:40 +0100	[thread overview]
Message-ID: <823a16af4733d5b82470b6ed6da203de09644caa.camel@linux.intel.com> (raw)
In-Reply-To: <20260211152644.1661165-7-arvind.yadav@intel.com>

On Wed, 2026-02-11 at 20:56 +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.
> 
> One thing to note: when the last VMA goes away, we default back to
> WILLNEED. DONTNEED is a per-mapping hint, and without any mappings
> there is no remaining madvise state to justify purging. This prevents
> BOs from becoming purgeable solely due to being temporarily unmapped.
> 
> 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)
> 
> v5:
>   - Call xe_bo_recheck_purgeable_on_vma_unbind() from
> xe_vma_destroy()
>     right after drm_gpuva_unlink() where we already hold the BO lock,
>     drop the trylock-based late destroy path (Matt)
>   - Move purgeable_state into xe_vma_mem_attr with the other madvise
>     attributes (Matt)
>   - Drop READ_ONCE since the BO lock already protects us (Matt)
>   - Keep returning false when there are no VMAs - otherwise we'd mark
>     BOs purgeable without any user hint (Matt)
>   - Use xe_bo_set_purgeable_state() instead of direct
> initialization(Matt)
>   - use xe_assert instead of drm_war (Thomas)

Typo. 

There were also a couple of review issues in my reply here:

https://patchwork.freedesktop.org/patch/699451/?series=156651&rev=5

that were never addressed or at least commented upon.

The comment there on retaining purgeable state after the last vma is
unmapped could be discussed, though.

Let's say we unmap a vma marking a bo purgeable. It then becomes either
purged or non-purgeable. 

Then an app tries to access it either using a new vma or CPU map. Then
it will typically succeed, or might occasionally fail if the bo
happened to be purged in between.

How do we handle new vma map requests and cpu-faults to a bo in
purgeable state? Do we block those?

Thanks,
Thomas



> 
> 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_svm.c        |  1 +
>  drivers/gpu/drm/xe/xe_vm.c         |  9 ++-
>  drivers/gpu/drm/xe/xe_vm_madvise.c | 98
> ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_vm_madvise.h |  3 +
>  drivers/gpu/drm/xe/xe_vm_types.h   | 11 ++++
>  5 files changed, 116 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_svm.c
> b/drivers/gpu/drm/xe/xe_svm.c
> index cda3bf7e2418..329c77aa5c20 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -318,6 +318,7 @@ static void xe_vma_set_default_attributes(struct
> xe_vma *vma)
>  		.preferred_loc.migration_policy =
> DRM_XE_MIGRATE_ALL_PAGES,
>  		.pat_index = vma->attr.default_pat_index,
>  		.atomic_access = DRM_XE_ATOMIC_UNDEFINED,
> +		.purgeable_state = XE_MADV_PURGEABLE_WILLNEED,
>  	};
>  
>  	xe_vma_mem_attr_copy(&vma->attr, &default_attr);
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 71cf3ce6c62b..e84b9e7cb5eb 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -39,6 +39,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)
> @@ -1085,6 +1086,7 @@ 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 = xe_vma_bo(vma);
>  
>  	if (vma->ufence) {
>  		xe_sync_ufence_put(vma->ufence);
> @@ -1099,7 +1101,7 @@ 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));
> +		xe_bo_put(bo);
>  	}
>  
>  	xe_vma_free(vma);
> @@ -1125,6 +1127,7 @@ static void vma_destroy_cb(struct dma_fence
> *fence,
>  static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence
> *fence)
>  {
>  	struct xe_vm *vm = xe_vma_vm(vma);
> +	struct xe_bo *bo = xe_vma_bo(vma);
>  
>  	lockdep_assert_held_write(&vm->lock);
>  	xe_assert(vm->xe, list_empty(&vma->combined_links.destroy));
> @@ -1133,9 +1136,10 @@ static void xe_vma_destroy(struct xe_vma *vma,
> struct dma_fence *fence)
>  		xe_assert(vm->xe, vma->gpuva.flags &
> XE_VMA_DESTROYED);
>  		xe_userptr_destroy(to_userptr_vma(vma));
>  	} else if (!xe_vma_is_null(vma) &&
> !xe_vma_is_cpu_addr_mirror(vma)) {
> -		xe_bo_assert_held(xe_vma_bo(vma));
> +		xe_bo_assert_held(bo);
>  
>  		drm_gpuva_unlink(&vma->gpuva);
> +		xe_bo_recompute_purgeable_state(bo);
>  	}
>  
>  	xe_vm_assert_held(vm);
> @@ -2681,6 +2685,7 @@ static int vm_bind_ioctl_ops_parse(struct xe_vm
> *vm, struct drm_gpuva_ops *ops,
>  				.atomic_access =
> DRM_XE_ATOMIC_UNDEFINED,
>  				.default_pat_index = op-
> >map.pat_index,
>  				.pat_index = op->map.pat_index,
> +				.purgeable_state =
> XE_MADV_PURGEABLE_WILLNEED,
>  			};
>  
>  			flags |= op->map.vma_flags &
> XE_VMA_CREATE_MASK;
> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c
> b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index d9cfba7bfe0b..c184426546a2 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;
> @@ -183,6 +184,89 @@ 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;
> +
> +	xe_bo_assert_held(bo);
> +
> +	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 (vma->attr.purgeable_state !=
> XE_MADV_PURGEABLE_DONTNEED)
> +				return false;
> +		}
> +	}
> +
> +	/*
> +	 * No VMAs => no mapping-level DONTNEED hint.
> +	 * Default to WILLNEED to avoid making BOs purgeable without
> +	 * explicit user intent.
> +	 */
> +	if (!has_vmas)
> +		return false;
> +
> +	return true;
> +}
> +
> +/**
> + * xe_bo_recompute_purgeable_state() - Recompute BO purgeable state
> from VMAs
> + * @bo: Buffer object
> + *
> + * Walk all VMAs to determine if BO should be purgeable or not.
> + * Shared BOs require unanimous DONTNEED state from all mappings.
> + *
> + * Locking: Caller must hold BO dma-resv lock. When iterating GPUVM
> lists,
> + * VM lock must also be held (write) to prevent concurrent VMA
> modifications.
> + * This is satisfied at both call sites:
> + * - xe_vma_destroy(): holds vm->lock write
> + * - madvise_purgeable(): holds vm->lock write (from madvise ioctl
> path)
> + *
> + * Return: nothing
> + */
> +void xe_bo_recompute_purgeable_state(struct xe_bo *bo)
> +{
> +	if (!bo)
> +		return;
> +
> +	xe_bo_assert_held(bo);
> +
> +	/*
> +	 * 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)
> +			xe_bo_set_purgeable_state(bo,
> XE_MADV_PURGEABLE_DONTNEED);
> +	} else {
> +		/* At least one VMA is WILLNEED - BO must not be
> purgeable */
> +		if (bo->madv_purgeable !=
> XE_MADV_PURGEABLE_WILLNEED)
> +			xe_bo_set_purgeable_state(bo,
> XE_MADV_PURGEABLE_WILLNEED);
> +	}
> +}
> +
>  /**
>   * madvise_purgeable - Handle purgeable buffer object advice
>   * @xe: XE device
> @@ -231,14 +315,20 @@ static void __maybe_unused
> madvise_purgeable(struct xe_device *xe,
>  
>  		switch (op->purge_state_val.val) {
>  		case DRM_XE_VMA_PURGEABLE_STATE_WILLNEED:
> -			xe_bo_set_purgeable_state(bo,
> XE_MADV_PURGEABLE_WILLNEED);
> +			vmas[i]->attr.purgeable_state =
> XE_MADV_PURGEABLE_WILLNEED;
> +
> +			/* Update BO purgeable state */
> +			xe_bo_recompute_purgeable_state(bo);
>  			break;
>  		case DRM_XE_VMA_PURGEABLE_STATE_DONTNEED:
> -			xe_bo_set_purgeable_state(bo,
> XE_MADV_PURGEABLE_DONTNEED);
> +			vmas[i]->attr.purgeable_state =
> XE_MADV_PURGEABLE_DONTNEED;
> +
> +			/* Update BO purgeable state */
> +			xe_bo_recompute_purgeable_state(bo);
>  			break;
>  		default:
> -			drm_warn(&vm->xe->drm, "Invalid madvice
> value = %d\n",
> -				 op->purge_state_val.val);
> +			/* Should never hit - values validated in
> madvise_args_are_sane() */
> +			xe_assert(vm->xe, 0);
>  			return;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.h
> b/drivers/gpu/drm/xe/xe_vm_madvise.h
> index b0e1fc445f23..39acd2689ca0 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_recompute_purgeable_state(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 43203e90ee3e..fd563039e8f4 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -94,6 +94,17 @@ struct xe_vma_mem_attr {
>  	 * same as default_pat_index unless overwritten by madvise.
>  	 */
>  	u16 pat_index;
> +
> +	/**
> +	 * @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;
>  };
>  
>  struct xe_vma {

  reply	other threads:[~2026-02-24 12:48 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-11 15:26 [PATCH v5 0/9] drm/xe/madvise: Add support for purgeable buffer objects Arvind Yadav
2026-02-11 15:26 ` [PATCH v5 1/9] drm/xe/uapi: Add UAPI " Arvind Yadav
2026-02-24 10:50   ` Thomas Hellström
2026-02-26 17:58   ` Souza, Jose
2026-02-27  9:32     ` Yadav, Arvind
2026-02-11 15:26 ` [PATCH v5 2/9] drm/xe/bo: Add purgeable bo state tracking and field madv to xe_bo Arvind Yadav
2026-02-11 16:00   ` Matthew Brost
2026-02-11 15:26 ` [PATCH v5 3/9] drm/xe/madvise: Implement purgeable buffer object support Arvind Yadav
2026-02-24 12:21   ` Thomas Hellström
2026-02-24 14:56     ` Yadav, Arvind
2026-02-11 15:26 ` [PATCH v5 4/9] drm/xe/bo: Handle CPU faults on purged buffer objects Arvind Yadav
2026-02-11 15:26 ` [PATCH v5 5/9] drm/xe/vm: Prevent binding of " Arvind Yadav
2026-02-11 16:17   ` Matthew Brost
2026-02-11 15:26 ` [PATCH v5 6/9] drm/xe/madvise: Implement per-VMA purgeable state tracking Arvind Yadav
2026-02-24 12:48   ` Thomas Hellström [this message]
2026-02-24 15:07     ` Yadav, Arvind
2026-02-24 16:36       ` Matthew Brost
2026-02-25  5:35         ` Yadav, Arvind
2026-02-25  8:21           ` Thomas Hellström
2026-02-25  9:04             ` Matthew Brost
2026-02-25  9:18               ` Thomas Hellström
2026-02-25  9:40                 ` Yadav, Arvind
2026-02-25 18:32                   ` Matthew Brost
2026-02-11 15:26 ` [PATCH v5 7/9] drm/xe/madvise: Block imported and exported dma-bufs Arvind Yadav
2026-02-24 14:15   ` Thomas Hellström
2026-02-11 15:26 ` [PATCH v5 8/9] drm/xe/bo: Add purgeable shrinker state helpers Arvind Yadav
2026-02-24 14:21   ` Thomas Hellström
2026-02-24 15:09     ` Yadav, Arvind
2026-02-11 15:26 ` [PATCH v5 9/9] drm/xe/madvise: Enable purgeable buffer object IOCTL support Arvind Yadav
2026-02-11 15:40   ` Matthew Brost
2026-02-11 15:46 ` [PATCH v5 0/9] drm/xe/madvise: Add support for purgeable buffer objects Matthew Brost
2026-02-25 10:10   ` Yadav, Arvind
2026-02-11 16:21 ` ✗ CI.checkpatch: warning for drm/xe/madvise: Add support for purgeable buffer objects (rev6) Patchwork
2026-02-11 16:22 ` ✓ CI.KUnit: success " Patchwork
2026-02-11 17:11 ` ✗ Xe.CI.BAT: failure " Patchwork
2026-02-13  1:15 ` ✗ 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=823a16af4733d5b82470b6ed6da203de09644caa.camel@linux.intel.com \
    --to=thomas.hellstrom@linux.intel.com \
    --cc=arvind.yadav@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    --cc=pallavi.mishra@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