Re: [PATCH v3] drm/xe/shrinker: Take a runtime PM ref before shrinking non-system memory

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The shrinker outer loop does not check if the scan target is met, leading to unintended GPU wakeups via PM worker.

Pre-existing issues:
- [High] Shrinker scan aborts entirely on unshrinkable BOs and discards successfully freed page counts.
--

commit 6f70bd030eb1841c2ee7446baf465783428a0cc6
Author: Shuicheng Lin <shuicheng.lin@intel.com>
Subject: drm/xe/shrinker: Take a runtime PM ref before shrinking non-system memory

This commit modifies the shrinker walk to take a runtime PM reference
before scanning non-system memory. This prevents GPU mapping invalidation
failures when the device is runtime suspended by queuing a worker to wake
the GPU if it is not currently active.

> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index 83374cd576608..175218b5edd99 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
> @@ -74,13 +76,25 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
>  		if (!man || !man->use_tt)
>  			continue;
>  
> +		if (mem_type != XE_PL_SYSTEM && !rpm &&
> +		    xe_device_is_l2_flush_optimized(xe)) {
> +			if (!xe_pm_runtime_get_if_active(xe)) {
> +				queue_work(xe->unordered_wq,
> +					   &shrinker->pm_worker);
> +				break;
> +			}
> +			rpm = true;
> +		}
> +
>  		ttm_bo_lru_for_each_reserved_guarded(&curs, man, &arg, ttm_bo) {
>  			if (!ttm_bo_shrink_suitable(ttm_bo, ctx))
>  				continue;
>  
>  			lret = xe_bo_shrink(ctx, ttm_bo, flags, scanned);
> -			if (lret < 0)
> -				return lret;
> +			if (lret < 0) {
> +				freed = lret;
> +				goto out;
> +			}

[Severity: High]
This is a pre-existing issue, but does this error path unintentionally
discard the count of successfully freed pages?

If __xe_shrinker_walk() successfully frees some pages and increments freed,
but then encounters an unshrinkable BO, xe_bo_shrink() will return a
negative error code (such as -EBUSY).

When that happens, this path overwrites the accumulated freed page count
with the error code before jumping to out. This causes any memory
successfully freed prior to the error to be lost and not reported back
to the kernel's memory management subsystem.

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

[Severity: Medium]
Does this inner break cause unintended GPU wakeups?

When the scan target (to_scan) is satisfied while scanning system memory
(XE_PL_SYSTEM), this break only exits the inner loop. The outer loop will
then proceed to scan XE_PL_TT.

When the outer loop proceeds to XE_PL_TT, the newly added runtime PM check
above will execute and can queue shrinker->pm_worker to wake up the
suspended GPU, even though no further scanning is needed.

>  		}
>  		/* Trylocks should never error, just fail. */
>  		xe_assert(xe, !IS_ERR(ttm_bo));
>  	}
>  
> +out:
> +	if (rpm)
> +		xe_pm_runtime_put(xe);
> +
>  	return freed;
>  }

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