Re: [PATCH 3/3] drm/ttm: allocate pool pages as compound (__GFP_COMP)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] ttm_pool_split_for_swap() ignores the return value of folio_split_driver_managed(), leading to severe buddy allocator corruption if the split fails.
- [High] Kconfig hazard: Forcing TRANSPARENT_HUGEPAGE via select bypasses critical architectural dependencies.
--

--- Patch [3]: [PATCH 3/3] drm/ttm: allocate pool pages as compound (__GFP_COMP) ---
commit dae665417eeff981f7bae0e75da08ad47a4dd0a0
Author: Matthew Brost <matthew.brost@intel.com>

drm/ttm: allocate pool pages as compound (__GFP_COMP)

This commit changes the non-DMA path in ttm_pool_alloc_page() to allocate
compound pages with __GFP_COMP instead of stashing the order in page->private.
It updates the backup, restore, and free paths to read the order via
folio_order() and split compound folios using folio_split_driver_managed().

> folio_split_driver_managed() lives in the THP split machinery
> (mm/huge_memory.c), which only builds when CONFIG_TRANSPARENT_HUGEPAGE
> is enabled. Drivers that drive the TTM shrinker and therefore reach the
> split path must select TRANSPARENT_HUGEPAGE.

[Severity: High]
Is it safe to force select TRANSPARENT_HUGEPAGE in drivers?

The Kconfig select directive forcefully enables the target symbol without
evaluating its dependencies. TRANSPARENT_HUGEPAGE strictly depends on
HAVE_ARCH_TRANSPARENT_HUGEPAGE and !PREEMPT_RT. 

Using select could force THP on architectures that lack support, breaking
the build, or bypass the !PREEMPT_RT dependency and violate strict latency
requirements.

> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 1bf37023fed66..364cc7ec7469c 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
[ ... ]
> @@ -493,15 +493,31 @@ static unsigned int ttm_pool_page_order(struct ttm_pool *pool, struct page *p)
>  static void ttm_pool_split_for_swap(struct ttm_pool *pool, struct page *p)
>  {
>  	unsigned int order = ttm_pool_page_order(pool, p);
>  
>  	if (!order)
>  		return;
>  
> -	split_page(p, order);
> -	nr = 1UL << order;
> -	while (nr--)
> -		(p++)->private = 0;
> +	if (ttm_pool_uses_dma_alloc(pool)) {
> +		pgoff_t nr;
> +
> +		/*
> +		 * DMA-alloc pages are not compound; split the plain
> +		 * higher-order allocation and clear the per-page private
> +		 * (which held the order for the non-compound case).
> +		 */
> +		split_page(p, order);
> +		nr = 1UL << order;
> +		while (nr--)
> +			(p++)->private = 0;
> +		return;
> +	}
> +
> +	/*
> +	 * The non-DMA path allocates compound folios (__GFP_COMP). Split the
> +	 * driver-owned, off-LRU, unmapped folio into order-0 folios so each
> +	 * page can be freed as soon as it has been backed up.
> +	 */
> +	folio_split_driver_managed(page_folio(p), 0);
>  }

[Severity: Critical]
Does this code handle split failures correctly?

If CONFIG_TRANSPARENT_HUGEPAGE is disabled, the fallback stub for
folio_split_driver_managed() unconditionally returns -EINVAL and the folio
remains an intact compound page. Because the return value is ignored,
callers like ttm_pool_backup() will incorrectly assume the split succeeded.

Will this corrupt the buddy allocator in ttm_pool_backup()? It follows the
split with this loop:

			ttm_pool_split_for_swap(pool, page);
			for (j = 0; j < nr_backed; j++)
				__free_pages_gpu_account(page + j, 0, false);

Calling __free_pages(page + j, 0) on a tail page of an active compound folio
redirects the refcount decrement to the head page, rapidly underflowing it.

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