Re: [PATCH v6 1/2] drm/xe/shrinker: Return the freed page count through a parameter

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2026-09-04 at 21:50 +0000, Shuicheng Lin wrote:
> __xe_shrinker_walk() and xe_shrinker_walk() return either the number
> of
> pages freed or a negative error, so the two cannot be reported at
> once.
> On error the pages already freed are dropped, and since
> xe_shrinker_scan()
> only accumulates non-negative returns while *scanned is updated by
> pointer, the shrinker tells mm that it scanned without freeing.
> 
> Accumulate the count into a caller-provided counter and return only
> the
> status, so an error no longer discards what the walk had freed.
> 
> Fixes: 00c8efc3180f ("drm/xe: Add a shrinker for xe bos")
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

> ---
>  drivers/gpu/drm/xe/xe_shrinker.c | 62 ++++++++++++++----------------
> --
>  1 file changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c
> b/drivers/gpu/drm/xe/xe_shrinker.c
> index 83374cd57660..89445cd20238 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> @@ -54,13 +54,14 @@ xe_shrinker_mod_pages(struct xe_shrinker
> *shrinker, long shrinkable, long purgea
>  	write_unlock(&shrinker->lock);
>  }
>  
> -static s64 __xe_shrinker_walk(struct xe_device *xe,
> +static int __xe_shrinker_walk(struct xe_device *xe,
>  			      struct ttm_operation_ctx *ctx,
>  			      const struct xe_bo_shrink_flags flags,
> -			      unsigned long to_scan, unsigned long
> *scanned)
> +			      unsigned long to_scan, unsigned long
> *scanned,
> +			      unsigned long *freed)
>  {
>  	unsigned int mem_type;
> -	s64 freed = 0, lret;
> +	s64 lret;
>  
>  	for (mem_type = XE_PL_SYSTEM; mem_type <= XE_PL_TT;
> ++mem_type) {
>  		struct ttm_resource_manager *man =
> ttm_manager_type(&xe->ttm, mem_type);
> @@ -82,7 +83,7 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
>  			if (lret < 0)
>  				return lret;
>  
> -			freed += lret;
> +			*freed += lret;
>  			if (*scanned >= to_scan)
>  				break;
>  		}
> @@ -90,7 +91,7 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
>  		xe_assert(xe, !IS_ERR(ttm_bo));
>  	}
>  
> -	return freed;
> +	return 0;
>  }
>  
>  /*
> @@ -99,40 +100,35 @@ static s64 __xe_shrinker_walk(struct xe_device
> *xe,
>   * add writeback. This avoids stalls and explicit writebacks with
> light or
>   * moderate memory pressure.
>   */
> -static s64 xe_shrinker_walk(struct xe_device *xe,
> +static int xe_shrinker_walk(struct xe_device *xe,
>  			    struct ttm_operation_ctx *ctx,
>  			    const struct xe_bo_shrink_flags flags,
> -			    unsigned long to_scan, unsigned long
> *scanned)
> +			    unsigned long to_scan, unsigned long
> *scanned,
> +			    unsigned long *freed)
>  {
>  	bool no_wait_gpu = true;
>  	struct xe_bo_shrink_flags save_flags = flags;
> -	s64 lret, freed;
> +	int ret;
>  
>  	swap(no_wait_gpu, ctx->no_wait_gpu);
>  	save_flags.writeback = false;
> -	lret = __xe_shrinker_walk(xe, ctx, save_flags, to_scan,
> scanned);
> +	ret = __xe_shrinker_walk(xe, ctx, save_flags, to_scan,
> scanned, freed);
>  	swap(no_wait_gpu, ctx->no_wait_gpu);
> -	if (lret < 0 || *scanned >= to_scan)
> -		return lret;
> +	if (ret || *scanned >= to_scan)
> +		return ret;
>  
> -	freed = lret;
>  	if (!ctx->no_wait_gpu) {
> -		lret = __xe_shrinker_walk(xe, ctx, save_flags,
> to_scan, scanned);
> -		if (lret < 0)
> -			return lret;
> -		freed += lret;
> -		if (*scanned >= to_scan)
> -			return freed;
> +		ret = __xe_shrinker_walk(xe, ctx, save_flags,
> to_scan, scanned,
> +					 freed);
> +		if (ret || *scanned >= to_scan)
> +			return ret;
>  	}
>  
> -	if (flags.writeback) {
> -		lret = __xe_shrinker_walk(xe, ctx, flags, to_scan,
> scanned);
> -		if (lret < 0)
> -			return lret;
> -		freed += lret;
> -	}
> +	if (flags.writeback)
> +		ret = __xe_shrinker_walk(xe, ctx, flags, to_scan,
> scanned,
> +					 freed);
>  
> -	return freed;
> +	return ret;
>  }
>  
>  static unsigned long
> @@ -214,7 +210,6 @@ static unsigned long xe_shrinker_scan(struct
> shrinker *shrink, struct shrink_con
>  	bool runtime_pm;
>  	bool purgeable;
>  	bool can_backup = !!(sc->gfp_mask & __GFP_FS);
> -	s64 lret;
>  
>  	nr_to_scan = sc->nr_to_scan;
>  
> @@ -225,12 +220,9 @@ static unsigned long xe_shrinker_scan(struct
> shrinker *shrink, struct shrink_con
>  	/* Might need runtime PM. Try to wake early if it looks like
> it. */
>  	runtime_pm = xe_shrinker_runtime_pm_get(shrinker, false,
> nr_to_scan, can_backup);
>  
> -	if (purgeable && nr_scanned < nr_to_scan) {
> -		lret = xe_shrinker_walk(shrinker->xe, &ctx,
> shrink_flags,
> -					nr_to_scan, &nr_scanned);
> -		if (lret >= 0)
> -			freed += lret;
> -	}
> +	if (purgeable && nr_scanned < nr_to_scan)
> +		xe_shrinker_walk(shrinker->xe, &ctx, shrink_flags,
> +				 nr_to_scan, &nr_scanned, &freed);
>  
>  	sc->nr_scanned = nr_scanned;
>  	if (nr_scanned >= nr_to_scan || !can_backup)
> @@ -242,10 +234,8 @@ static unsigned long xe_shrinker_scan(struct
> shrinker *shrink, struct shrink_con
>  
>  	shrink_flags.purge = false;
>  
> -	lret = xe_shrinker_walk(shrinker->xe, &ctx, shrink_flags,
> -				nr_to_scan, &nr_scanned);
> -	if (lret >= 0)
> -		freed += lret;
> +	xe_shrinker_walk(shrinker->xe, &ctx, shrink_flags,
> +			 nr_to_scan, &nr_scanned, &freed);
>  
>  	sc->nr_scanned = nr_scanned;
>  out: