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.ursu...@igalia.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Thomas Hellström <thomas.hellst...@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));
