[PATCH 1/2] drm/ttm: Respect the shrinker core free target

[Christian König]

On 6/2/25 17:29, Tvrtko Ursulin wrote:
> Currently the TTM shrinker aborts shrinking as soon as it frees pages from
> any of the page order pools and by doing so it can fail to respect the
> freeing target which was configured by the shrinker core.
> 
> We use the wording "can fail" because the number of freed pages will
> depend on the presence of pages in the pools and the order of the pools on
> the LRU list. For example if there are no free pages in the high order
> pools the shrinker core may require multiple passes over the TTM shrinker
> before it will free the default target of 128 pages (assuming there are
> free pages in the low order pools). This inefficiency can be compounded by
> the pool LRU where multiple further calls into the TTM shrinker are
> required to end up looking at the pool with pages.
> 
> Improve this by never freeing less than the shrinker core has requested.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> ---
>  drivers/gpu/drm/ttm/ttm_pool.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index c2ea865be657..a3247a82cadd 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -1132,17 +1132,24 @@ void ttm_pool_fini(struct ttm_pool *pool)
>  }
>  EXPORT_SYMBOL(ttm_pool_fini);
>  
> -/* As long as pages are available make sure to release at least one */
>  static unsigned long ttm_pool_shrinker_scan(struct shrinker *shrink,
>  					    struct shrink_control *sc)
>  {
> -	unsigned long num_freed = 0;
> +	unsigned long to_scan, freed = 0;
>  
> -	do
> -		num_freed += ttm_pool_shrink();
> -	while (!num_freed && atomic_long_read(&allocated_pages));

This can be massively simplified by changing this here to (num_freed < sc->nr_to_scan && atomic_long_read(&allocated_pages)).

Regards,
Christian.

> +	to_scan = min_t(unsigned long,
> +			sc->nr_to_scan,
> +			atomic_long_read(&allocated_pages));
> +	while (freed < to_scan) {
> +		freed += ttm_pool_shrink();
> +		to_scan = min_t(unsigned long,
> +				to_scan,
> +				atomic_long_read(&allocated_pages));
> +	}
>  
> -	return num_freed;
> +	sc->nr_scanned = freed;
> +
> +	return freed ?: SHRINK_STOP;
>  }
>  
>  /* Return the number of pages available or SHRINK_EMPTY if we have none */
> @@ -1266,8 +1273,10 @@ EXPORT_SYMBOL(ttm_pool_debugfs);
>  /* Test the shrinker functions and dump the result */
>  static int ttm_pool_debugfs_shrink_show(struct seq_file *m, void *data)
>  {
> -	struct shrink_control sc = { .gfp_mask = GFP_NOFS };
> -
> +	struct shrink_control sc = {
> +		.gfp_mask = GFP_NOFS,
> +		.nr_to_scan = 1,
> +	};
>  	fs_reclaim_acquire(GFP_KERNEL);
>  	seq_printf(m, "%lu/%lu\n", ttm_pool_shrinker_count(mm_shrinker, &sc),
>  		   ttm_pool_shrinker_scan(mm_shrinker, &sc));