Re: [PATCH v2 2/2] drm/xe: Update shrinker batch size based on average BO size

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

On Fri, Aug 14, 2026 at 04:24:05PM -0700, Matthew Brost wrote:
> On Fri, Aug 14, 2026 at 04:37:37PM +0200, Thomas Hellström wrote:
> > Update our preferred vmscan batch size on each count pass to avoid
> > invoking scan_objects for requests too small to free even a single
> > average-sized GEM object. Our rough estimate for an effective batch
> > is twice the average number of pages per populated ttm_tt across all
> > shrinkable and purgeable objects. The factor of two provides headroom
> > so that most scan invocations can free at least one GEM object despite
> > variability in object sizes.
> > 
> > The batch value is updated as an exponential moving average,
> > (old_batch + avg) / 2, to smooth out sudden changes in the
> > object population. It is floored at 128 pages, the kernel default
> > SHRINK_BATCH, to ensure the shrinker remains responsive when there
> > are very few objects.
> > 
> > The populated_tts counter introduced in the previous commit provides
> > the object count needed for the average. We inherit the same
> > justification as the analogous mechanism in i915: shrinking a GEM
> > object has non-trivial locking overhead, so firing the shrinker for
> > requests smaller than a single object is wasteful.
> > 
> > v2:
> > - Fix the average object size estimate to account for the full
> >   shrinkable and purgeable population.
> > 
> > Assisted-by: GitHub_Copilot:claude-sonnet-4.6
> > Assisted-by: GitHub_Copilot:claude-sonnet-5
> 
> This is probably the right direction given what we currently have in
> terms of shrinker control, but the core heuristic is still a pretty poor
> one. My understanding is that it combines batch and seek values using
> some odd math to determine whether a scan is worthwhile at a given
> priority level. We probably want to avoid shrinking at the initial scan
> priorities, and I believe this change accomplishes that.
> 
> That said, I think we really want two shrinkers instead: one with the
> default settings (or perhaps even a reduced seek value) for purgeable
> BOs, and another for BOs that we legitimately need to back up. The
> purgeable one should be favored to run eariler, likewise the TTM pool
> shrinker should be favored run before our shrinker too.
> 
> Also we really should look at getting priority based shrinking in too, I
> have follow up there too which disconnects purgable / not in working set
> from shared VM dma-resv also, further prioritizing though shrinks.
> 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_shrinker.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> > index cded230f5459..284fce207705 100644
> > --- a/drivers/gpu/drm/xe/xe_shrinker.c
> > +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> > @@ -146,6 +146,8 @@ xe_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
> >  {
> >  	struct xe_shrinker *shrinker = to_xe_shrinker(shrink);
> >  	unsigned long num_pages;
> > +	unsigned long total_pages;
> > +	unsigned long populated_tts;
> >  	bool can_backup = !!(sc->gfp_mask & __GFP_FS);
> >  
> >  	num_pages = ttm_backup_bytes_avail() >> PAGE_SHIFT;
> > @@ -157,8 +159,32 @@ xe_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
> >  		num_pages = 0;
> >  
> >  	num_pages += shrinker->purgeable_pages;
> > +	total_pages = shrinker->shrinkable_pages + shrinker->purgeable_pages;
> > +	populated_tts = shrinker->populated_tts;
> >  	read_unlock(&shrinker->lock);
> >  
> > +	/*
> > +	 * Update our preferred vmscan batch size for the next pass.
> > +	 * Our rough guess for an effective batch size is twice the average
> > +	 * number of pages per GEM object. That is, we don't want the
> > +	 * shrinker to fire until the request is large enough to justify
> > +	 * the overhead of freeing at least one GEM object.
> > +	 *
> > +	 * Base the average on the full shrinkable + purgeable population
> > +	 * (total_pages), not on num_pages, which is reduced to just the
> > +	 * purgeable pages whenever the gfp mask disallows backup (can_backup
> > +	 * false). Otherwise the estimate would systematically undershoot in
> > +	 * exactly the GFP_NOFS / GFP_NOIO reclaim paths where avoiding
> > +	 * excessive scan_objects() calls matters most.
> > +	 */
> > +	if (populated_tts) {
> > +		unsigned long avg = 2 * total_pages / populated_tts;
> > +
> > +		shrinker->shrink->batch =
> > +			max((shrinker->shrink->batch + avg) >> 1,
> > +			    128UL /* default SHRINK_BATCH */);
> 
> I think 128UL should be at least whatever TTM pool batch is?
> 

If my math is correct the TTM pook shrinker sets this to 320 when
MAX_PAGE_ORDER == 10.

Matt

> Matt
> 
> > +	}
> > +
> >  	return num_pages ? num_pages : SHRINK_EMPTY;
> >  }
> >  
> > -- 
> > 2.55.0
> >