Re: Patch "drm/xe: Don't transfer shrinker pages for pinned bos on purgeable state change" has been added to the 7.2-stable tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: shrinker page accounting corruption in xe_bo_set_purgeable_shrinker()
--

commit 38fe9b0b7cfb46e5ebc7efb56c8bdf33363c1017
Author: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>

Patch "drm/xe: Don't transfer shrinker pages for pinned bos on purgeable state change" has been added to the 7.2-stable tree

This commit updates xe_bo_set_purgeable_shrinker() in
drivers/gpu/drm/xe/xe_bo.c to set tt_pages to 0 when xe_bo_is_pinned(bo) is
true, updating only xe_tt->purgeable without transferring pages between
shrinker buckets. However, 7.2-stable does not include upstream commit
6a117a2bb7ca65 ("drm/xe: Fix shrinker accounting double-subtraction on nested
external pins"), which changed scanout framebuffer pinning to use
xe_bo_pin_external().

Link: https://patch.msgid.link/20261009070053.7790-1-thomas.hellstrom@linux.intel.com

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 7ed76349075fd..2d8283f6010a6 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -838,6 +838,8 @@ static int xe_bo_move_notify(struct xe_bo *bo,
>   *
>   * Transfers pages between shrinkable and purgeable buckets when the BO
>   * purgeable state changes. Called automatically from xe_bo_set_purgeable_state().
> + * For pinned BOs, whose pages are not accounted, only the purgeable flag is
> + * updated, so that the correct bucket is used when the BO is unpinned.
>   */
>  static void xe_bo_set_purgeable_shrinker(struct xe_bo *bo,
>  					 enum xe_madv_purgeable_state new_state)
> @@ -854,7 +856,8 @@ static void xe_bo_set_purgeable_shrinker(struct xe_bo *bo,
>  		return;
>  
>  	xe_tt = container_of(tt, struct xe_ttm_tt, ttm);
> -	tt_pages = tt->num_pages;
> +	/* Pinned bos are not accounted. Only update the purgeable flag. */
> +	tt_pages = xe_bo_is_pinned(bo) ? 0 : tt->num_pages;
>  
>  	if (!xe_tt->purgeable && new_state == XE_MADV_PURGEABLE_DONTNEED) {
>  		xe_tt->purgeable = true;

[Severity: High]
Does setting tt_pages = 0 when xe_bo_is_pinned(bo) is true in
xe_bo_set_purgeable_shrinker() in drivers/gpu/drm/xe/xe_bo.c introduce a
shrinker page accounting regression on 7.2-stable when a scanout framebuffer
buffer object changes purgeable state while pinned?

In 7.2-stable (which lacks upstream commit 6a117a2bb7ca65, "drm/xe: Fix
shrinker accounting double-subtraction on nested external pins"), scanout
framebuffers are pinned and unpinned directly via ttm_bo_pin() and
ttm_bo_unpin() without calling xe_ttm_tt_account_subtract() or
xe_ttm_tt_account_add():

drivers/gpu/drm/xe/display/xe_fb_pin.c:__xe_pin_fb_vma() {
	...
		if (IS_DGFX(xe))
			ret = xe_bo_migrate(bo, XE_PL_VRAM0, NULL, &exec);
		else
			ret = xe_bo_validate(bo, NULL, true, &exec);
		drm_exec_retry_on_contention(&exec);
		xe_validation_retry_on_oom(&ctx, &ret);
		if (!ret)
			ttm_bo_pin(&bo->ttm);
	...
}

drivers/gpu/drm/xe/display/xe_fb_pin.c:__xe_unpin_fb_vma() {
	...
	ttm_bo_reserve(&vma->bo->ttm, false, false, NULL);
	ttm_bo_unpin(&vma->bo->ttm);
	ttm_bo_unreserve(&vma->bo->ttm);
	...
}

When a populated user buffer object has its pages accounted in
shrinker->shrinkable_pages via xe_ttm_tt_populate() ->
xe_ttm_tt_account_add() and is then pinned for scanout via
__xe_pin_fb_vma() -> ttm_bo_pin(&bo->ttm), its pages remain accounted in
shrinker->shrinkable_pages while bo->ttm.pin_count > 0.

If userspace then updates the purgeable state via madvise_purgeable() ->
xe_bo_willneed_put_locked() -> xe_bo_set_purgeable_state() ->
xe_bo_set_purgeable_shrinker(), xe_bo_is_pinned(bo) is true, so
xe_bo_set_purgeable_shrinker() sets tt_pages = 0 and updates
xe_tt->purgeable = true without moving tt->num_pages from
shrinker->shrinkable_pages to shrinker->purgeable_pages.

After __xe_unpin_fb_vma() -> ttm_bo_unpin(&vma->bo->ttm) drops
bo->ttm.pin_count to 0 without re-adding pages, subsequent unpopulation in
xe_ttm_tt_unpopulate(), purging in xe_bo_shrink_purge(), or another state
change while unpinned in xe_bo_set_purgeable_shrinker() subtracts from the
bucket selected by xe_tt->purgeable:

drivers/gpu/drm/xe/xe_bo.c:xe_ttm_tt_account_subtract() {
	struct xe_ttm_tt *xe_tt = container_of(tt, struct xe_ttm_tt, ttm);

	if (xe_tt->purgeable)
		xe_shrinker_mod_pages(xe->mem.shrinker, 0, -(long)tt->num_pages);
	else
		xe_shrinker_mod_pages(xe->mem.shrinker, -(long)tt->num_pages, 0);
}

Because xe_tt->purgeable was flipped while the pages stayed in
shrinker->shrinkable_pages, wouldn't this subtract tt->num_pages from
shrinker->purgeable_pages instead, underflowing shrinker->purgeable_pages to
a negative value, leaving shrinker->shrinkable_pages permanently inflated,
and causing unsigned long wrap-around in xe_shrinker_count() in
drivers/gpu/drm/xe/xe_shrinker.c:

drivers/gpu/drm/xe/xe_shrinker.c:xe_shrinker_count() {
	...
	if (can_backup)
		num_pages = min_t(unsigned long, num_pages, shrinker->shrinkable_pages);
	else
		num_pages = 0;

	num_pages += shrinker->purgeable_pages;
	...
}

when shrinker->purgeable_pages is negative?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026102151-compound-fondness-145b@gregkh?part=1