Re: [PATCH v3 1/3] drm/xe: Track number of populated ttm_tts in the shrinker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: shrinker accounting underflow on madvise of pinned BOs
--

commit 0b5d1dd0e073a31ff71e38b6052b00140aecc46a
Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>

drm/xe: Track number of populated ttm_tts in the shrinker

This commit adds a populated_tts counter to struct xe_shrinker to track
the number of ttm_tts that are populated and on the LRU. It also extends
xe_shrinker_mod_pages() to atomically update both page and object accounting.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -975,11 +975,11 @@ static void xe_bo_set_purgeable_shrinker(struct xe_bo *bo,
>  	if (!xe_tt->purgeable && new_state == XE_MADV_PURGEABLE_DONTNEED) {
>  		xe_tt->purgeable = true;
>  		/* Transfer pages from shrinkable to purgeable count */
> -		xe_shrinker_mod_pages(xe->mem.shrinker, -tt_pages, tt_pages);
> +		xe_shrinker_mod_pages(xe->mem.shrinker, -tt_pages, tt_pages, 0);
>  	} else if (xe_tt->purgeable && new_state == XE_MADV_PURGEABLE_WILLNEED) {
>  		xe_tt->purgeable = false;
>  		/* Transfer pages from purgeable to shrinkable count */
> -		xe_shrinker_mod_pages(xe->mem.shrinker, tt_pages, -tt_pages);
> +		xe_shrinker_mod_pages(xe->mem.shrinker, tt_pages, -tt_pages, 0);

[Severity: High]
Does this logic cause an underflow in the shrinker page accounting when a
buffer object is pinned?

When a BO is pinned (e.g., used as a scanout framebuffer which calls
xe_bo_pin()), its pages are explicitly excluded from shrinker tracking.
However, if userspace subsequently calls the DRM_XE_VM_MADVISE ioctl on
the VMA to transition its state to DONTNEED or WILLNEED,
xe_bo_willneed_put_locked() invokes xe_bo_set_purgeable_shrinker().

Because xe_bo_set_purgeable_shrinker() unconditionally transfers pages
between the shrinkable and purgeable buckets without checking if the BO
is pinned (bo->ttm.pin_count > 0), this can lead to an integer underflow
on shrinkable_pages or purgeable_pages.

When evaluated by xe_shrinker_count(), a negative signed value casts to a
large positive unsigned long, creating excessive reclaim pressure.
Furthermore, when the BO is eventually unpinned, its pages will be added
back to the shrinker accounting based on its new state, resulting in a
permanent double-counting of pages that triggers an xe_assert() during
xe_shrinker_fini().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008123024.38522-1-thomas.hellstrom@linux.intel.com?part=1