Re: [PATCH v2 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 2 potential issue(s) to consider:
- [Critical] Harvesting tail pages of a bisected chunk during defragmentation leads to Use-After-Free.
- [High] Missing __GFP_NOWARN and __GFP_RETRY_MAYFAIL in best-effort preallocation causes log spam and potential OOM killer invocation.
--

--- Patch [9]: [PATCH v2 09/33] drm/ttm: Preallocate beneficial-order defrag pages outside the lock ---
commit d825f68af024a3a3838c6e5d1ecfcf5ee8717374
Author: Matthew Brost <matthew.brost@intel.com>
Subject: drm/ttm: Preallocate beneficial-order defrag pages outside the lock

This commit introduces a mechanism to preallocate high-order pages outside
the DMA reservation lock during defragmentation moves. By using a prealloc
bag, it avoids stalling in direct reclaim or compaction while holding the
lock, improving concurrent rendering performance.

> 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
[ ... ]
> @@ -929,6 +939,172 @@ static int ttm_pool_harvest_remaining(struct ttm_pool *pool,
>  	return 0;
>  }
>  
> +unsigned int ttm_pool_prealloc_order(struct ttm_pool *pool)
> +{
> +	return ttm_pool_beneficial_order(pool);
> +}
> +EXPORT_SYMBOL(ttm_pool_prealloc_order);
> +
> +/*
> + * Build the gfp flags used for the high-order, possibly reclaiming, beneficial
> + * order page allocations, matching the in-line defrag alloc path.
> + */
> +static gfp_t ttm_pool_prealloc_gfp(struct ttm_pool *pool)
> +{
> +	gfp_t gfp = GFP_USER;
> +
> +	/*
> +	 * No highmem: prealloc applies caching in bulk via set_pages_array_*()
> +	 * on the kernel mapping, so the pages must be permanently mapped.
> +	 */
> +	if (ttm_pool_uses_dma32(pool))
> +		gfp |= GFP_DMA32;
> +
> +	return gfp;
> +}

[Severity: High]
Does this base GFP flag configuration risk invoking the OOM killer on a
fragmented system?

The preallocation performed in ttm_pool_prealloc_fill() appears to be an
opportunistic, best-effort operation:

ttm_pool_prealloc_fill() {
        ...
        struct page *p = ttm_pool_alloc_page(pool, gfp, order, false);

Without including __GFP_NOWARN and __GFP_RETRY_MAYFAIL here, a failure to
allocate these high-order pages will trigger aggressive reclaim. This could
unnecessarily invoke the OOM killer and spam the logs, even though the pool
can safely fall back to in-line allocation.

Should these suppression flags be added to the preallocation gfp mask?

[ ... ]
> @@ -1178,8 +1374,8 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
>  	     alloc->remaining_pages;
>  	     it.order = ttm_pool_alloc_find_order(it.order, alloc)) {
>  		/*
> -		 * Out of defrag budget: harvest the rest of the old tt as-is and
> -		 * stop (the tt is re-queued if the remainder is still
> +		 * Out of defrag budget/prealloc: harvest the rest of the old tt
> +		 * as-is and stop (the tt is re-queued if the remainder is still
>  		 * sub-optimal).
>  		 */
>  		if (ttm_pool_iter_defrag_exhausted(&it)) {

[Severity: Critical]
Can this lead to memory corruption if the budget is exhausted while the
offset is misaligned with the chunks in the old tt?

If the budget is exhausted here, ttm_pool_iter_harvest_rest() will
call ttm_pool_harvest_remaining(), which reads the order from the
current page:

ttm_pool_harvest_remaining() {
        ...
        struct page *p = old_tt->pages[off];
        unsigned int order = ttm_pool_page_order(pool, p);

If off points to a tail page of a previously allocated larger chunk,
ttm_pool_page_order() will return 0 because the private field is 0 on tail
pages. This causes the code to falsely harvest it as an order-0 chunk. When
the old tt is later freed, it frees the entire large chunk from the head
page, leaving the new tt with pointers to freed memory.

> +			r = ttm_pool_iter_harvest_rest(&it);
> +			if (r)
> +				goto error_free_all;
> +			break;
> +		}
[ ... ]

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