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 <[email protected]> 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/[email protected]?part=23
