Re: [PATCH v2 15/33] drm/xe: Track BOs backed at a sub-optimal page order

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] BOs moved to SYSTEM memory during a failed shrinker purge are incorrectly left on the defrag list.
- [Medium] Framebuffer pinning bypasses defrag list tracking updates, violating the invariant that pinned BOs are removed from the list.
--

--- Patch [15]: [PATCH v2 15/33] drm/xe: Track BOs backed at a sub-optimal page order ---
commit d8c62c56c566775d903f43756778e071af0acb80
Author: Matthew Brost <matthew.brost@intel.com>

    drm/xe: Track BOs backed at a sub-optimal page order

    Add per-device tracking of BOs whose backing TT pages were allocated
    below the pool's beneficial order, as reported by the
    TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED tt page flag. These BOs are future
    candidates for a defragmentation pass that reallocates their pages at the
    beneficial order.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index cca617cf34d13..d7b0536564ab2 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
[ ... ]
> @@ -1261,6 +1358,15 @@ static long xe_bo_shrink_purge(struct ttm_operation_ctx *ctx,
>  	if (lret > 0) {
>  		xe_ttm_tt_account_subtract(xe, bo->ttm);
>  		update_global_total_pages(bo->bdev, -(long)tt->num_pages);
> +
> +		/*
> +		 * The pages are gone, so a tracked BO no longer needs defrag.
> +		 * This shrinker purge path frees pages without going through
> +		 * xe_bo_move(), so drop any stale defrag list entry here. Ghost
> +		 * bos are never tracked, so guard for real xe bos.
> +		 */
> +		if (xe_bo_is_xe_bo(bo))
> +			xe_bo_defrag_remove(ttm_to_xe_bo(bo));
>  	}
>  
>  	return lret;

[Severity: Medium]
When xe_bo_shrink_purge() attempts to purge a BO, it uses ttm_bo_move_null() to
move it to XE_PL_SYSTEM, which bypasses xe_bo_defrag_update(). 

If the subsequent ttm_bo_shrink() call fails (lret <= 0), this explicit
xe_bo_defrag_remove() is skipped.

Could this leave a tracked BO in SYSTEM memory while incorrectly remaining on
the defrag list, violating the tracking invariant and wasting worker cycles?

[ ... ]
> @@ -3072,6 +3183,8 @@ int xe_bo_pin(struct xe_bo *bo, struct drm_exec *exec)
>  	if (bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
>  		xe_ttm_tt_account_subtract(xe, bo->ttm.ttm);
>  
> +	xe_bo_defrag_remove(bo);
> +
>  	/*
>  	 * FIXME: If we always use the reserve / unreserve functions for locking
>  	 * we do not need this.

[Severity: Medium]
Will this miss defrag list tracking updates for framebuffer BOs?

xe_fb_pin() and __xe_unpin_fb_vma() in drivers/gpu/drm/xe/display/xe_fb_pin.c
call ttm_bo_pin() and ttm_bo_unpin() directly rather than using this
xe_bo_pin() wrapper.

If a framebuffer BO resides in XE_PL_TT with sub-optimal page allocations,
could it remain on the defrag list even when pinned, causing the defrag
worker to incorrectly process pinned BOs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710215442.2444235-1-matthew.brost@intel.com?part=15