Re: [PATCH] drm/xe: Don't transfer shrinker pages for pinned bos on purgeable state change

[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 for display BOs on MADV_DONTNEED
--

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

    drm/xe: Don't transfer shrinker pages for pinned bos on purgeable state change
    
    Pinned bos have their pages removed from the shrinker's shrinkable /
    purgeable counts on pin, and added back on unpin according to the
    bo's purgeable state at that time.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 6921b6967330d..a752e01efbd75 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
[ ... ]
> @@ -876,7 +878,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;

[Severity: High]
Could this introduce a regression leading to a shrinker accounting underflow
for native display BOs?

Native display framebuffers are pinned for scanout via __xe_pin_fb_vma().
Unlike imported BOs, this function calls ttm_bo_pin() directly without calling
xe_ttm_tt_account_subtract():

drivers/gpu/drm/xe/display/xe_fb_pin.c:__xe_pin_fb_vma() {
    ...
    ret = ttm_bo_pin(&bo->ttm);
    ...
}

Because of this, their pages appear to remain actively accounted in the
shrinker's shrinkable_pages bucket while pinned.

If userspace sets MADV_DONTNEED on a display BO while it is pinned, won't this
new check skip the page transfer from shrinkable_pages to purgeable_pages
since xe_bo_is_pinned(bo) is true?

It seems this skips the page transfer but still sets xe_tt->purgeable = true.
When the display BO is later unpinned via __xe_unpin_fb_vma(), it calls
ttm_bo_unpin() directly without adding pages back to the shrinker:

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

When the BO is eventually unpopulated or destroyed, wouldn't
xe_ttm_tt_account_subtract() see xe_tt->purgeable == true and subtract the
pages from the purgeable_pages bucket?

Since the pages were never transferred, does this cause purgeable_pages to
underflow and shrinkable_pages to remain permanently inflated?

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