Re: [PATCH v7 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 LRU shrinker walk aborts entirely upon encountering a buffer object (BO) that is unsuitable for shrinking, potentially causing a livelock where the shrinker fails to make progress.
--

commit fdbe7158df33982691ee678948191c46bf0f5148
Author: Shuicheng Lin <shuicheng.lin@intel.com>
drm/xe/shrinker: Return the freed page count through a parameter

This commit refactors __xe_shrinker_walk() and xe_shrinker_walk() to return
status errors and accumulate freed pages via a parameter. This prevents
dropping the count of already freed pages when an error occurs during the scan.

> 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;
>  
> -			freed += lret;
> +			*freed += lret;

[Severity: High]
This is a pre-existing issue, but does this early return abort the entire
shrinker walk if a single buffer object is unsuitable for shrinking?

In xe_bo_shrink() located in drivers/gpu/drm/xe/xe_bo.c, -EBUSY is
legitimately returned if the BO cannot be shrunk. For instance, when
a non-purgeable BO is encountered during a purge pass:

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

        if (!xe_bo_eviction_valuable(bo, &place))
                return -EBUSY;

If __xe_shrinker_walk() receives this -EBUSY, the entire iteration is aborted.
Because the iteration starts from the head of the LRU, an unshrinkable BO at
the head could perpetually abort the shrinker on its first step, failing to
make progress under memory pressure.

Should the loop ignore -EBUSY and continue scanning?

>  			if (*scanned >= to_scan)
>  				break;
>  		}

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