Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] An empty preallocation bag causes a defrag move to perform all allocations in-line under the dma-resv lock, causing severe reclaim/compaction stalls and defeating the design goal. --
--- Patch [9]: [PATCH v3 09/33] drm/ttm: Preallocate beneficial-order defrag pages outside the lock --- commit a99093b0428c713eac3edfaabcb993852aeabdbd Author: Matthew Brost <[email protected]> drm/ttm: Preallocate beneficial-order defrag pages outside the lock This commit introduces a preallocation bag for TTM pool allocations to hoist expensive high-order allocations out of the dma-resv critical section. It records suboptimal pages during populate so a defragmentation pass can appropriately size its out-of-lock preallocation, and harvests the remainder of the old tt if the preallocation runs out. > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c [ ... ] > @@ -990,16 +1168,20 @@ struct ttm_pool_alloc_iter { > }; > > /* > - * Defrag move budget exhausted: the upgrade can make no further progress > this > - * pass. Snapshot @defrag_capped is set only when a byte budget was in force > at > - * entry. > + * Defrag move budget exhausted, or the out-of-lock prealloc bag ran dry: the > + * upgrade can make no further progress this pass. Snapshot @defrag_capped is > + * set only when a byte budget was in force at entry. > */ > static bool ttm_pool_iter_defrag_exhausted(const struct ttm_pool_alloc_iter > *it) > { > + const struct ttm_pool_prealloc *pp = it->prealloc; > + > if (!it->defrag_old_tt) > return false; > + if (it->defrag_capped && it->alloc->defrag_bytes_remaining <= 0) > + return true; > > - return it->defrag_capped && it->alloc->defrag_bytes_remaining <= 0; > + return pp && pp->count && pp->used >= pp->count; [Severity: High] If the out-of-lock preallocation fails to allocate any pages due to memory pressure, pp->count will be 0. Does the && pp->count condition cause this function to return false in that scenario? If this returns false for an empty preallocation bag, __ttm_pool_alloc() will bypass the exhaustion check. The allocator would then proceed with in-line system page allocations while holding the dma-resv lock. Because an empty bag is most likely on a fragmented system, these in-line allocations could stall aggressively in direct reclaim and compaction, which seems to bypass the goal of hoisting these allocations out of the critical section. Could this check be updated to return pp && pp->used >= pp->count; so that an empty bag correctly aborts the defrag move? > } > > /* -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
