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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Concurrent read-modify-write of the shared shrinker batch size without synchronization causes a data race.
--

--- Patch [2]: [PATCH v2 2/2] drm/xe: Update shrinker batch size based on average BO size ---
commit 93a100ad39134ef7ca30d7a7dc94040b4d8bbe71
Author: Thomas Hellström <[email protected]>

drm/xe: Update shrinker batch size based on average BO size

This commit updates the preferred vmscan batch size based on the average
number of pages per GEM object. It adjusts the batch size as an exponential
moving average to avoid firing the shrinker for requests that are too small
to free an average-sized object.

> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index cded230f5459c..284fce207705c 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
[ ... ]
> @@ -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.
[ ... ]
> +	 */
> +	if (populated_tts) {
> +		unsigned long avg = 2 * total_pages / populated_tts;
> +
> +		shrinker->shrink->batch =
> +			max((shrinker->shrink->batch + avg) >> 1,
> +			    128UL /* default SHRINK_BATCH */);

[Severity: Medium]
Will this concurrent update cause a data race?

Since the lock is released with read_unlock just before this block, multiple
tasks entering xe_shrinker_count during direct reclaim or kswapd could
perform this read-modify-write concurrently.

While occasional lost updates to this heuristic might be functionally benign,
could this unprotected access trigger concurrency sanitizers?

> +	}
> +
>  	return num_pages ? num_pages : SHRINK_EMPTY;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2