Re: [PATCH v4 3/4] drm/xe: Split the bo shrinker into purgeable and non-purgeable shrinkers

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Oct 16, 2026 at 04:09:44PM +0200, Thomas Hellström wrote:
> On Fri, 2026-10-16 at 13:53 +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential
> > issue(s) to consider:
> > - [High] drm/xe: Use-After-Free of shrinker during DRM device
> > teardown
> > - [High] drm/xe: O(N^2) CPU lockup in purgeable shrinker due to
> > uncounted LRU skips
> > --
> > 
> > commit 1e9348c2fe4768de2cf24e4dd9b6188400c84043
> > Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > 
> > drm/xe: Split the bo shrinker into purgeable and non-purgeable
> > shrinkers
> > 
> > This commit splits the single xe_shrinker into two independent
> > shrinker
> > instances: one for purgeable objects and another for non-purgeable
> > (shrinkable) objects. This allows the shrinker core to apply
> > different
> > reclaim pressures based on the varying costs of reclaiming these
> > object
> > types.
> > 
> > > diff --git a/drivers/gpu/drm/xe/xe_shrinker.c
> > > b/drivers/gpu/drm/xe/xe_shrinker.c
> > > index 378022c37a57c..8e3fe675e0f34 100644
> > > --- a/drivers/gpu/drm/xe/xe_shrinker.c
> > > +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> > 
> > [ ... ]
> > 
> > > @@ -216,63 +238,52 @@ static bool xe_shrinker_runtime_pm_get(struct
> > > xe_shrinker *shrinker, bool force,
> > 
> > [ ... ]
> > 
> > > -static unsigned long xe_shrinker_scan(struct shrinker *shrink,
> > > struct shrink_control *sc)
> > > +static unsigned long xe_shrinker_scan_purgeable(struct shrinker
> > > *shrink,
> > > +						struct
> > > shrink_control *sc)
> > >  {
> > >  	struct xe_shrinker *shrinker = to_xe_shrinker(shrink);
> > >  	struct ttm_operation_ctx ctx = {
> > >  		.interruptible = false,
> > >  		.no_wait_gpu = ttm_bo_shrink_avoid_wait(),
> > >  	};
> > > -	unsigned long nr_to_scan, nr_scanned = 0, freed = 0;
> > > +	unsigned long nr_to_scan = sc->nr_to_scan, nr_scanned = 0,
> > > freed = 0;
> > >  	struct xe_bo_shrink_flags shrink_flags = {
> > >  		.purge = true,
> > > -		/* Don't request writeback without __GFP_IO. */
> > > -		.writeback = !ctx.no_wait_gpu && (sc->gfp_mask &
> > > __GFP_IO),
> > >  	};
> > > -	bool runtime_pm;
> > > -	bool purgeable;
> > > -	bool can_backup = !!(sc->gfp_mask & __GFP_FS);
> > > -
> > > -	nr_to_scan = sc->nr_to_scan;
> > > -
> > > -	read_lock(&shrinker->lock);
> > > -	purgeable = !!shrinker->purgeable_pages;
> > > -	read_unlock(&shrinker->lock);
> > >  
> > > -	/* Might need runtime PM. Try to wake early if it looks
> > > like it. */
> > > -	runtime_pm = xe_shrinker_runtime_pm_get(shrinker, false,
> > > nr_to_scan, can_backup);
> > > -
> > > -	if (purgeable && nr_scanned < nr_to_scan)
> > > -		xe_shrinker_walk(shrinker, &ctx, shrink_flags,
> > > -				 nr_to_scan, &nr_scanned, &freed);
> > > +	xe_shrinker_walk(shrinker, &ctx, shrink_flags, nr_to_scan,
> > > &nr_scanned, &freed);
> > >  
> > >  	sc->nr_scanned = nr_scanned;
> > 
> > [Severity: High]
> > Can this cause an O(N^2) CPU lockup in the purgeable shrinker?
> > 
> > When the memory reclaim core (do_shrink_slab) calls this purgeable
> > scan
> > routine under memory pressure, xe_shrinker_walk() iterates the LRU
> > list.
> > 
> > However, non-purgeable BOs return -EBUSY and are skipped without
> > advancing
> > the scanned counter:
> > 
> > drivers/gpu/drm/xe/xe_bo.c:xe_bo_shrink() {
> >     ...
> >     if (!(tt->page_flags & TTM_TT_FLAG_EXTERNAL_MAPPABLE) ||
> >         (flags.purge && !xe_tt->purgeable))
> >         return -EBUSY;
> >     ...
> > }
> > 
> > drivers/gpu/drm/xe/xe_shrinker.c:__xe_shrinker_walk() {
> >     ...
> >             lret = xe_bo_shrink(ctx, ttm_bo, flags, scanned);
> >             /* Like the TTM LRU walk, skip bos we can't shrink now.
> > */
> >             if (lret == -EBUSY)
> >                 continue;
> >     ...
> > }
> > 
> > Since the LRU cursor is initialized locally on each call and non-
> > purgeable
> > BOs do not increment nr_scanned, every batch rescans all leading
> > non-purgeable BOs without significantly advancing sc->nr_scanned.
> > 
> > Will this force the shrinker core to repeatedly call this function,
> > processing the same leading non-purgeable BOs over and over again?
> 
> This can't happen in practice since purgeable bos are always ordered
> before non-purgeable bos on the list.
> 

Is this true, though? If a BO is marked as purged on a hot VM and shares
the VM's bulk-move structure, it would remain hot in the list. Also,
within an individual bulk-move list, I don't think a purgable object is
guaranteed to be at the front of the LRU.

So, at a minimum, BOs marked as purged should be moved to a lower
priority band, as this patch does [1], and reclaim should only walk the
purged-priority band [2].

We can do this in an immediate follow-up after this series merges.

Matt

[1] https://patchwork.freedesktop.org/patch/743454/?series=170454&rev=5
[2] https://patchwork.freedesktop.org/patch/743454/?series=170454&rev=5

> /Thomas
> 
> 
> 
> > 
> > > -	if (nr_scanned >= nr_to_scan || !can_backup)
> > > -		goto out;
> > >  
> > > -	/* If we didn't wake before, try to do it now if needed.
> > > */
> > > -	if (!runtime_pm)
> > > -		runtime_pm = xe_shrinker_runtime_pm_get(shrinker,
> > > true, 0, can_backup);
> > > +	return nr_scanned ? freed : SHRINK_STOP;
> > > +}
> > 
> > [ ... ]
> > 
> > > @@ -294,6 +305,7 @@ static void xe_shrinker_fini(struct drm_device
> > > *drm, void *arg)
> > >  	xe_assert(shrinker->xe, !shrinker->purgeable_pages);
> > >  	xe_assert(shrinker->xe, !shrinker->populated_tts);
> > >  	shrinker_free(shrinker->shrink);
> > > +	shrinker_free(shrinker->purge_shrink);
> > >  	flush_work(&shrinker->pm_worker);
> > >  	kfree(shrinker);
> > >  }
> > 
> > [Severity: High]
> > Does this kfree(shrinker) leave xe->mem.shrinker as a dangling
> > pointer?
> > 
> > Because drmm actions execute in LIFO order upon device release,
> > xe_shrinker_fini() is called before xe_device_destroy(). 
> > 
> > Later, xe_device_destroy() runs and calls xe_bo_dev_fini() which
> > flushes
> > the async_free workqueue. Pending BOs in the workqueue will have
> > their
> > final references dropped, leading to xe_ttm_tt_account_subtract():
> > 
> > drivers/gpu/drm/xe/xe_bo.c:xe_ttm_tt_account_subtract() {
> >     ...
> >     if (xe_tt->purgeable)
> >         xe_shrinker_mod_pages(xe->mem.shrinker, 0, -(long)tt-
> > >num_pages, -1);
> >     else
> >         xe_shrinker_mod_pages(xe->mem.shrinker, -(long)tt->num_pages,
> > 0, -1);
> >     ...
> > }
> > 
> > Can this lead to a use-after-free and memory corruption when driver
> > unloads?