Re: [PATCH v2 23/33] drm/xe: Defrag using out-of-lock page preallocation

[sashiko-bot@kernel.org]

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

New issues:
- [Low] 64-bit division using the '/' operator causes a link-time build failure on 32-bit architectures (missing __udivdi3).

Pre-existing issues:
- [High] Accessing `bo->ttm.ttm` without holding the BO's dma-resv lock can lead to a NULL pointer dereference or UAF if the BO is concurrently evicted or destroyed.
--

--- Patch [23]: [PATCH v2 23/33] drm/xe: Defrag using out-of-lock page preallocation ---
commit d11ab210d871e5fedb57edb2f956ed1814f6d5a4
Author: Matthew Brost <matthew.brost@intel.com>
Date:   Fri Jul 10 21:54:32 2026 +0000

    drm/xe: Defrag using out-of-lock page preallocation
    
    Convert xe_bo_defrag_one() to a three-phase scheme that moves the
    expensive beneficial-order allocations out of the BO dma-resv lock.
    This hoists allocation and caching out to reduce BO-lock held time.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 405316d0d116b..2f014fc9f988e 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -1249,15 +1249,55 @@ static int xe_bo_defrag_one(struct xe_device *xe, struct xe_bo *bo,

[ ... ]

> +	if (order && ttm_bo->ttm) {
> +		u32 suboptimal = ttm_tt_suboptimal_pages(ttm_bo->ttm);
> +		u64 cap = min_t(u64, budget, xe_bo_size(bo)) >> PAGE_SHIFT;
> +
> +		/* Prealloc only the beneficial-order chunks the move replaces. */
> +		want = DIV_ROUND_UP(min_t(u64, suboptimal, cap), 1UL << order);
                 ^^^^^^^^^^^^

[Severity: Low]
Does this code cause a link-time build failure on 32-bit architectures?

The first argument to DIV_ROUND_UP() is a u64, and the second evaluates
to an unsigned long (32-bit integer) and is not a compile-time constant.
This results in a 64-bit division using the '/' operator, which emits a
call to __udivdi3 on 32-bit GCC. Because the kernel doesn't provide this
intrinsic, it will break the build.

Could DIV_ROUND_UP_ULL() be used here instead?

> +	}
> +	xe_bo_unlock(bo);

[ ... ]

This is a pre-existing issue, but I noticed a potential problem in
xe_bo_defrag_worker() regarding how the BO is accessed after unlocking:

drivers/gpu/drm/xe/xe_bo.c:xe_bo_defrag_worker() {
	...
		ret = xe_bo_defrag_one(xe, bo, XE_BO_DEFRAG_SIZE_LIMIT - defrag_bytes,
				       &consumed);
		defrag_bytes += consumed;

		if (ret || ttm_tt_is_beneficial_order_failed(bo->ttm.ttm)) {
	...
}

[Severity: High]
Can accessing bo->ttm.ttm here without holding the BO's dma-resv lock lead
to a NULL pointer dereference or use-after-free?

xe_bo_defrag_worker() calls xe_bo_defrag_one(), which acquires the BO lock,
performs the move, and then unlocks the BO before returning. After returning,
the worker evaluates ttm_tt_is_beneficial_order_failed(bo->ttm.ttm) without
holding the BO lock.

If another thread concurrently evicts the BO to VRAM, the system memory
ttm_tt is destroyed and bo->ttm.ttm is set to NULL. Additionally, if
xe_bo_defrag_one() finds the BO no longer needs defragmentation, it removes
it from the list and returns 0, and bo->ttm.ttm could also be NULL at this
point.

Should this check be performed while the BO lock is still held?

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