All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: <sashiko-reviews@lists.linux.dev>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>,
	<intel-xe@lists.freedesktop.org>
Subject: Re: [RFC 4/4] drm/xe: Bump prefetch BO LRU for already-valid ranges
Date: Fri, 18 Sep 2026 14:28:19 -0700	[thread overview]
Message-ID: <aq2s87pHUQarNmFa@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260916113301.6B98D1F000FF@smtp.kernel.org>

On Wed, Sep 16, 2026 at 11:33:01AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/xe: memory leak of svm_range on prefetch_lru_bump failure
> --
> 
> commit 09ec19c469b99c340c07a04780a85f997af72901
> Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> drm/xe: Bump prefetch BO LRU for already-valid ranges
> 
> This commit introduces xe_svm_range_prefetch_lru_bump to re-fault CPU pages
> without touching DMA mappings and move backing BOs to the LRU tail for
> already-valid prefetch ranges.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index fe9ccaa129122..948060a702470 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> 
> [ ... ]
> 
> > @@ -2583,6 +2583,15 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops,
> >  						  dpagemap, &valid_pages)) {
> >  				xe_svm_range_debug(svm_range, "PREFETCH - RANGE IS VALID");
> >  				xe_assert(vm->xe, valid_pages);
> > +
> > +				if (dpagemap) {
> > +					err = xe_svm_range_prefetch_lru_bump(vm, vma,
> > +									     svm_range,

How about we just make xe_svm_range_prefetch_lru_bump non-failable? All
prefetching is fundamentally an eager optimization: we hope it succeeds,
but if it races, oh well, a fault will fix it. I think the LRU bump fits
that mindset as well.

Matt

> > +									     dpagemap);
> > +					if (err)
> > +						goto unwind_prefetch_ops;
> 
> [Severity: Medium]
> Does this code leak the svm_range?
> 
> During a VM_BIND ioctl with prefetch, xe_svm_range_find_or_insert() returns
> an svm_range with an incremented reference count. If 
> xe_svm_range_prefetch_lru_bump() fails here, we jump to unwind_prefetch_ops.
> 
> Because this error path is taken before the svm_range is tracked in the
> op->prefetch_range.range xarray, the cleanup routine
> xe_svm_prefetch_gpuva_ops_fini() will not know about it and will not free it.
> 
> Should this error path call xe_svm_range_put(svm_range) before jumping to
> unwind_prefetch_ops to avoid leaking the reference?
> 
> > +				}
> > +
> >  				need_put = true;
> >  				goto check_next_range;
> >  			}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=4

  reply	other threads:[~2026-09-18 21:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16 11:28 [RFC 0/4] Refresh TTM LRU on SVM prefetch Himal Prasad Ghimiray
2026-09-16 11:28 ` [RFC 1/4] drm/pagemap: Add helper to access backing devmem allocation Himal Prasad Ghimiray
2026-09-18 21:04   ` Matthew Brost
2026-09-16 11:28 ` [RFC 2/4] drm/gpusvm: Add devmem callback to get_pages Himal Prasad Ghimiray
2026-09-16 11:36   ` sashiko-bot
2026-09-18 21:24     ` Matthew Brost
2026-09-21  8:34       ` Ghimiray, Himal Prasad
2026-09-16 11:28 ` [RFC 3/4] drm/xe: Bump prefetch BO LRU via GPUSVM devmem callback Himal Prasad Ghimiray
2026-09-16 11:36   ` sashiko-bot
2026-09-18 21:02     ` Matthew Brost
2026-09-21  8:35       ` Ghimiray, Himal Prasad
2026-09-16 11:28 ` [RFC 4/4] drm/xe: Bump prefetch BO LRU for already-valid ranges Himal Prasad Ghimiray
2026-09-16 11:33   ` sashiko-bot
2026-09-18 21:28     ` Matthew Brost [this message]
2026-09-21  8:35       ` Ghimiray, Himal Prasad
2026-09-16 11:32 ` ✗ CI.KUnit: failure for Refresh TTM LRU on SVM prefetch 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=aq2s87pHUQarNmFa@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.