Re: [PATCH v3 03/33] drm/ttm: Record sub-optimal page order allocations in ttm_tt

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Fri, Jul 10, 2026 at 07:55:49PM -0700, Matthew Brost wrote:
>When __ttm_pool_alloc() fails to allocate a chunk at the device's
>beneficial order and falls back to a smaller order, the object ends up
>backed by a sub-optimal set of pages. Nothing currently records this, so
>a driver has no way to know an object would benefit from being re-backed
>with beneficial-order pages later.
>
>Add TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED to struct ttm_tt page_flags,
>cleared at the start of every fresh allocation (in both ttm_pool_alloc()
>and ttm_pool_restore_and_alloc()). Set it in __ttm_pool_alloc() when an
>allocation at exactly the beneficial order fails before dropping to a
>lower order.
>
>Drivers can use this hint to queue the buffer object for later
>defragmentation.
>
>Cc: Carlos Santa <carlos.santa@intel.com>
>Cc: Ryan Neph <ryanneph@google.com>
>Cc: Christian Koenig <christian.koenig@amd.com>
>Cc: Huang Rui <ray.huang@amd.com>
>Cc: Matthew Auld <matthew.auld@intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>Cc: Maxime Ripard <mripard@kernel.org>
>Cc: Thomas Zimmermann <tzimmermann@suse.de>
>Cc: David Airlie <airlied@gmail.com>
>Cc: Simona Vetter <simona@ffwll.ch>
>Cc: dri-devel@lists.freedesktop.org
>Cc: linux-kernel@vger.kernel.org
>Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>Assisted-by: GitHub_Copilot:claude-opus-4.8
>Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>---
> drivers/gpu/drm/ttm/ttm_pool.c | 13 +++++++++++++
> include/drm/ttm/ttm_tt.h       | 24 +++++++++++++++++++++++-
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
>index 5b7d7f5ae53d..e9175c0290d7 100644
>--- a/drivers/gpu/drm/ttm/ttm_pool.c
>+++ b/drivers/gpu/drm/ttm/ttm_pool.c
>@@ -796,6 +796,7 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
> 			    struct ttm_pool_alloc_state *alloc,
> 			    struct ttm_pool_tt_restore *restore)
> {
>+	const unsigned int beneficial_order = ttm_pool_beneficial_order(pool);
> 	enum ttm_caching page_caching;
> 	gfp_t gfp_flags = GFP_USER;
> 	pgoff_t caching_divide;
>@@ -846,6 +847,16 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
> 		/* If that fails, lower the order if possible and retry. */
> 		if (!p) {
> 			if (order) {
>+				/*
>+				 * Failing to allocate at the device's beneficial
>+				 * order means we are about to back this object
>+				 * with a sub-optimal (smaller order) set of
>+				 * pages. Record it so the driver can later try to
>+				 * defragment the object back to beneficial order.
>+				 */
>+				if (beneficial_order && order == beneficial_order)
>+					tt->page_flags |=
>+						TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED;

I haven't looked at beneficial_reclaim_backoff usecase yet, but this flag
gets set irrespective of whether the allocation failure resulted with/without
the beneficial_reclaim_backoff request from the driver. Is that intended?

Other than that, the change here LGTM, though I still need to look at how driver uses
this flag in more detail.

Niranjana

> 				--order;
> 				page_caching = tt->caching;
> 				allow_pools = true;
>@@ -910,6 +921,7 @@ int ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
> 	if (WARN_ON(ttm_tt_is_backed_up(tt)))
> 		return -EINVAL;
>
>+	tt->page_flags &= ~TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED;
> 	ttm_pool_alloc_state_init(tt, &alloc);
>
> 	return __ttm_pool_alloc(pool, tt, ctx, &alloc, NULL);
>@@ -942,6 +954,7 @@ int ttm_pool_restore_and_alloc(struct ttm_pool *pool, struct ttm_tt *tt,
> 	if (!restore) {
> 		gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
>
>+		tt->page_flags &= ~TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED;
> 		ttm_pool_alloc_state_init(tt, &alloc);
> 		if (ctx->gfp_retry_mayfail)
> 			gfp |= __GFP_RETRY_MAYFAIL;
>diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
>index 406437ad674b..ce7533677d77 100644
>--- a/include/drm/ttm/ttm_tt.h
>+++ b/include/drm/ttm/ttm_tt.h
>@@ -90,6 +90,14 @@ struct ttm_tt {
> 	 * TTM_TT_FLAG_BACKED_UP: TTM internal only. This is set if the
> 	 * struct ttm_tt has been (possibly partially) backed up.
> 	 *
>+	 * TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED: Set by the TTM pool allocator
>+	 * when at least one chunk that could have been allocated at the pool's
>+	 * beneficial order had to fall back to a smaller order. This indicates
>+	 * that a sub-optimal set of pages was chosen for this object, and can
>+	 * be used by drivers to queue the buffer object for later
>+	 * defragmentation. Drivers should access this via the
>+	 * ttm_tt_is_beneficial_order_failed() helper.
>+	 *
> 	 * TTM_TT_FLAG_PRIV_POPULATED: TTM internal only. DO NOT USE. This is
> 	 * set by TTM after ttm_tt_populate() has successfully returned, and is
> 	 * then unset when TTM calls ttm_tt_unpopulate().
>@@ -101,8 +109,9 @@ struct ttm_tt {
> #define TTM_TT_FLAG_EXTERNAL_MAPPABLE	BIT(3)
> #define TTM_TT_FLAG_DECRYPTED		BIT(4)
> #define TTM_TT_FLAG_BACKED_UP	        BIT(5)
>+#define TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED	BIT(6)
>
>-#define TTM_TT_FLAG_PRIV_POPULATED	BIT(6)
>+#define TTM_TT_FLAG_PRIV_POPULATED	BIT(7)
> 	uint32_t page_flags;
> 	/** @num_pages: Number of pages in the page array. */
> 	uint32_t num_pages;
>@@ -179,6 +188,19 @@ static inline void ttm_tt_clear_backed_up(struct ttm_tt *tt)
> 	tt->page_flags &= ~TTM_TT_FLAG_BACKED_UP;
> }
>
>+/**
>+ * ttm_tt_is_beneficial_order_failed() - Whether the tt is backed at a
>+ * sub-optimal page order
>+ * @tt: The struct ttm_tt.
>+ *
>+ * Return: true if the pool allocator had to fall back below the pool's
>+ * beneficial order when backing this tt, false otherwise.
>+ */
>+static inline bool ttm_tt_is_beneficial_order_failed(const struct ttm_tt *tt)
>+{
>+	return tt->page_flags & TTM_TT_FLAG_BENEFICIAL_ORDER_FAILED;
>+}
>+
> /**
>  * ttm_tt_create
>  *
>-- 
>2.34.1
>