From: Matthew Brost <matthew.brost@intel.com>
To: "Yadav, Arvind" <arvind.yadav@intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
intel-xe@lists.freedesktop.org, 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 08:36:54 -0800 [thread overview]
Message-ID: <aZ3TpuSFKzY4y9sg@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <5aaab739-2291-441e-937b-746495ce7d58@intel.com>
On Tue, Feb 24, 2026 at 08:37:44PM +0530, Yadav, Arvind wrote:
>
> On 24-02-2026 18:18, Thomas Hellström wrote:
> > 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.
>
>
> Noted,
>
> >
> > 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?
>
>
> @Thomas,
>
> The implementation already blocks new access to purged BOs:
> 1. New VMA mappings (Patch 0005): vma_lock_and_validate() rejects MAP
> operations to purged BOs with -EINVAL via the check_purged flag.
> 2. CPU faults (Patch 0004): Both xe_bo_cpu_prep() and xe_gem_mmap_offset()
> return errors (-EFAULT / VM_FAULT_SIGBUS) when accessing purged BOs.
> 3 . "Once purged, always purged": Even when the last VMA is unmapped,
> xe_bo_recompute_purgeable_state() preserves the PURGED state - it never
> transitions back to WILLNEED or DONTNEED (see early return at the top of the
> function).
>
> The only way forward for the application is to destroy the purged BO and
> create a new one.
>
> Regarding the 'no VMAs → WILLNEED' logic: this only applies to non-purged
> BOs that happen to be temporarily unmapped. Purged BOs remain permanently
> invalid.
So I think xe_bo_all_vmas_dontneed() isn't 100% correct...
I think should return an enum...
enum xe_bo_vmas_purge_state { /* Maybe a better name? */
XE_BO_VMAS_STATE_DONTNEED = 0,
XE_BO_VMAS_STATE_WILLNEED = 1,
XE_BO_VMAS_STATE_NO_VMAS = 2,
};
Then in xe_bo_recompute_purgeable_state() something like this:
void xe_bo_recompute_purgeable_state(struct xe_bo *bo)
{
enum xe_bo_vma_purge_state state;
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;
state = xe_bo_all_vmas_dontneed(bo);
if (state == XE_BO_VMAS_STATE_DONTNEED) {
/* 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 if (state == XE_BO_VMAS_STATE_WILLNEED) {
/* 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);
}
}
I think would avoid the last unbind unintentionally flipping from
DONTNEED -> WILLNEED.
What do you both of you (Thomas, Arvind) think?
Matt
>
> Thanks,
> Arvind
> >
> > 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 {
next prev parent reply other threads:[~2026-02-24 16:37 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
2026-02-24 15:07 ` Yadav, Arvind
2026-02-24 16:36 ` Matthew Brost [this message]
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=aZ3TpuSFKzY4y9sg@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