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

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

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.

/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?