Re: [PATCH v4 4/4] drm/xe: Update non-purgeable shrinker batch size based on average BO size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: shrinker count callback serializes direct reclaim via write_lock
--

commit a78bd8842e67f957a2ced6d827e1c4a6430ff29b
Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>
drm/xe: Update non-purgeable shrinker batch size based on average BO size

This commit updates the preferred vmscan batch size for the non-purgeable 
shrinker based on the average buffer object size. It introduces an 
exponential moving average that is updated during the shrinker count phase.

> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index 8e3fe675e0f34..955908b3bdf3e 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
[ ... ]
> @@ -210,15 +235,26 @@ xe_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
>  	unsigned long num_pages;
>  	bool can_backup = !!(sc->gfp_mask & __GFP_FS);
>  
> +	if (!can_backup)
> +		return SHRINK_EMPTY;
> +
>  	num_pages = ttm_backup_bytes_avail() >> PAGE_SHIFT;
> -	read_lock(&shrinker->lock);
> +	/* Consistently use write_lock() for simplicity. */
> +	write_lock(&shrinker->lock);

[Severity: Medium]
Will this exclusive write lock create a system-wide performance bottleneck
during memory reclaim?

The core mm subsystem frequently calls the shrinker's count_objects callback
(xe_shrinker_count) from do_shrink_slab() during direct reclaim. This occurs
concurrently across multiple allocating tasks and CPUs when under memory
pressure.

By changing read_lock() to write_lock() to update the batch_avg heuristic,
this forces all concurrent direct reclaimers system-wide to serialize on this
single lock, which could lead to severe lock contention and performance
degradation.

>  
> -	if (can_backup)
> -		num_pages = min_t(unsigned long, num_pages, shrinker->shrinkable_pages);
> -	else
> -		num_pages = 0;
> +	num_pages = min_t(unsigned long, num_pages, shrinker->shrinkable_pages);
>  
> -	read_unlock(&shrinker->lock);
> +	if (shrinker->populated_tts) {
> +		unsigned long avg = 2 * (shrinker->shrinkable_pages +
> +					 shrinker->purgeable_pages) /
> +				     shrinker->populated_tts;
> +
> +		shrinker->batch_avg = max((shrinker->batch_avg + (long)avg) >> 1,
> +					  128L /* default SHRINK_BATCH */);
> +		WRITE_ONCE(shrink->batch, shrinker->batch_avg);
> +	}
> +
> +	write_unlock(&shrinker->lock);
>  
>  	return num_pages ? num_pages : SHRINK_EMPTY;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016133523.3622-1-thomas.hellstrom@linux.intel.com?part=4