Re: [PATCH v4 1/2] drm/xe/shrinker: Do not abort the walk on unshrinkable bos

[sashiko-bot@kernel.org]

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

New issues:
- [High] Skipping unshrinkable BOs with `continue` without incrementing the `scanned` counter creates an unbounded traversal of the LRU list.

Pre-existing issues:
- [High] The caller `xe_shrinker_walk()` drops all pages freed in earlier escalation passes if a subsequent pass fails, causing lost reclaim accounting.
--

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

drm/xe/shrinker: Do not abort the walk on unshrinkable bos

The commit makes the shrinker skip over unshrinkable BOs instead of aborting
the walk, and attempts to fix the lost reclaim count when returning an error.

> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index 83374cd576608..953a911b51b30 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> @@ -79,8 +79,10 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
>  				continue;
>  
>  			lret = xe_bo_shrink(ctx, ttm_bo, flags, scanned);
> +			if (lret == -EBUSY)
> +				continue;

[Severity: High]
Does skipping unshrinkable BOs here create an unbounded traversal of the LRU
list? 

If xe_bo_shrink() returns -EBUSY, the scanned counter is not incremented.
This bypasses the termination check, allowing the loop to endlessly traverse
the BO LRU list when it contains many pinned or unshrinkable buffers.

This is a pre-existing issue in the same loop, but the
ttm_bo_shrink_suitable() check also fails to increment the scanned counter:

			if (!ttm_bo_shrink_suitable(ttm_bo, ctx))
				continue;

>  			if (lret < 0)
> -				return lret;
> +				return freed ? freed : lret;

[Severity: High]
This isn't a bug introduced by this patch, but is there a similar lost reclaim
accounting issue in the caller xe_shrinker_walk()?

When xe_shrinker_walk() transitions to the second pass (non-idle objects), an
error drops the pages successfully freed in the first pass:

	freed = lret;
	if (!ctx->no_wait_gpu) {
		lret = __xe_shrinker_walk(xe, ctx, save_flags, to_scan, scanned);
		if (lret < 0)
			return lret;

Could this cause the memory management core to receive a report of 0 pages
freed, incorrectly assuming reclaim is failing and triggering premature OOM?

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

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