Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>,
	<intel-xe@lists.freedesktop.org>
Subject: Re: [V2] drm/xe: Skip clearing purged page-table BOs
Date: Mon, 7 Sep 2026 20:55:29 -0700	[thread overview]
Message-ID: <ap+HMbjPp68+Wb+d@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <ee9418ac-38fa-4891-8024-4d4752a406b7@intel.com>

On Tue, Sep 08, 2026 at 09:22:35AM +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> On 08-09-2026 00:58, Ghimiray, Himal Prasad wrote:
> > 
> > 
> > On 07-09-2026 21:07, Tejas Upadhyay wrote:
> > > VRAM page offlining can clear a page-table BO's vmap before VM teardown
> > > calls xe_pt_clear(). Check for a purged BO under its dma-resv lock before
> > > writing to the mapping.
> > > 
> > > v2(Sashiko):
> > > - Fix lock ordering
> > > 
> > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_pt.c | 6 ++++--
> > >   drivers/gpu/drm/xe/xe_vm.c | 8 +++++---
> > >   2 files changed, 9 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > > index 5d990c1c3740..a9b9cd7ac2c8 100644
> > > --- a/drivers/gpu/drm/xe/xe_pt.c
> > > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > > @@ -236,9 +236,11 @@ void xe_pt_destroy(struct xe_pt *pt, u32 flags,
> > > struct llist_head *deferred)
> > >    */
> > >   void xe_pt_clear(struct xe_device *xe, struct xe_pt *pt)
> > >   {
> > > -    struct iosys_map *map = &pt->bo->vmap;
> > > +    struct xe_bo *bo = pt->bo;
> > > -    xe_map_memset(xe, map, 0, 0, SZ_4K);
> > > +    xe_bo_assert_held(bo);
> > > +    if (!xe_bo_is_purged(bo))
> > > +        xe_map_memset(xe, &bo->vmap, 0, 0, SZ_4K);
> > >   }
> > >   /**
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > > index 9e71567b27cc..e77ff3cf3ca3 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > @@ -1876,10 +1876,13 @@ static void xe_vm_close(struct xe_vm *vm)
> > >       bound = drm_dev_enter(&xe->drm, &idx);
> > >       down_write(&vm->lock);
> > > +    xe_vm_lock(vm, false);
> > > +
> > >       if (xe_vm_in_fault_mode(vm))
> > >           xe_svm_notifier_lock(vm);
> > > -
> > >       vm->size = 0;
> > > +    if (xe_vm_in_fault_mode(vm))
> > > +        xe_svm_notifier_unlock(vm);
> > >       if (!((vm->flags & XE_VM_FLAG_MIGRATION))) {
> > >           struct xe_tile *tile;
> > > @@ -1901,8 +1904,7 @@ static void xe_vm_close(struct xe_vm *vm)
> > >           }
> > >       }
> > > -    if (xe_vm_in_fault_mode(vm))
> > > -        xe_svm_notifier_unlock(vm);
> > > +    xe_vm_unlock(vm);
> > >       up_write(&vm->lock);
> > 
> > LGTM
> > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> Sashiko pointed a valid scenario where the check breaks and could lead to
> NPD.  Please move to iosys_map_is_null check as suggested.
> 

Yes, indeed. Also if it isn't clear we still need the VM dma-resv lock
like in this series even though iosys_map_is_null won't have the needed
lockdep assertions.

Matt 

> > 
> > >       if (bound)
> > 
> 

  reply	other threads:[~2026-09-08  3:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 15:37 [V2] drm/xe: Skip clearing purged page-table BOs Tejas Upadhyay
2026-09-07 15:46 ` ✓ CI.KUnit: success for drm/xe: Skip clearing purged page-table BOs (rev2) Patchwork
2026-09-07 15:51 ` [V2] drm/xe: Skip clearing purged page-table BOs sashiko-bot
2026-09-07 16:45 ` ✓ Xe.CI.BAT: success for drm/xe: Skip clearing purged page-table BOs (rev2) Patchwork
2026-09-07 19:28 ` [V2] drm/xe: Skip clearing purged page-table BOs Ghimiray, Himal Prasad
2026-09-08  3:52   ` Ghimiray, Himal Prasad
2026-09-08  3:55     ` Matthew Brost [this message]
2026-09-08  3:56       ` Ghimiray, Himal Prasad
2026-09-07 19:41 ` ✓ Xe.CI.FULL: success for drm/xe: Skip clearing purged page-table BOs (rev2) 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=ap+HMbjPp68+Wb+d@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=tejas.upadhyay@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