Re: [PATCH v3 09/33] drm/ttm: Preallocate beneficial-order defrag pages outside the lock

[sashiko-bot@kernel.org]

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 <matthew.brost@intel.com>

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/20260711025619.2540575-1-matthew.brost@intel.com?part=9