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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The shrinker scan aborts completely upon encountering a non-eligible BO, degrading reclaim effectiveness.
--

commit f040cd7950a53bb8a92b925733431f658c554101
Author: Shuicheng Lin <shuicheng.lin@intel.com>

drm/xe/shrinker: Return the freed page count through a parameter

This patch updates the xe_shrinker_walk and __xe_shrinker_walk functions to
return the number of freed pages through a pointer parameter. This prevents 
accumulated freed page counts from being discarded if an error occurs later
in the scan.

Fixes: 00c8efc3180f ("drm/xe: Add a shrinker for xe bos")

> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index 83374cd576608..89445cd202380 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c

[ ... ]

> @@ -82,7 +83,7 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
>  			if (lret < 0)
>  				return lret;

[Severity: High]
This is a pre-existing issue, but does this logic prematurely abort the
shrinker scan?

When xe_shrinker_scan() initiates a shrinker walk with shrink_flags.purge set
to true, __xe_shrinker_walk() iterates over the TTM LRU list. For each BO, it
calls xe_bo_shrink().

If flags.purge is true but the BO itself is not purgeable, xe_bo_shrink() in
drivers/gpu/drm/xe/xe_bo.c intentionally returns -EBUSY:

xe_bo_shrink() {
        ...
        if (!(tt->page_flags & TTM_TT_FLAG_EXTERNAL_MAPPABLE) ||
            (flags.purge && !xe_tt->purgeable))
                return -EBUSY;
        ...
}

Because -EBUSY is less than 0, __xe_shrinker_walk() will immediately abort
the entire scan instead of skipping the ineligible BO and continuing to the
next one.

Could this lead to poor reclaim effectiveness and potential out-of-memory
conditions if the shrinker encounters a single non-purgeable buffer object
on the LRU list?

> -			freed += lret;
> +			*freed += lret;
>  			if (*scanned >= to_scan)
>  				break;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904215049.543535-1-shuicheng.lin@intel.com?part=1