Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7] drm/ttm/pool: back up at native page order
@ 2026-07-15  8:00 Matthew Brost
  2026-07-15  8:08 ` ✓ CI.KUnit: success for drm/ttm/pool: back up at native page order (rev3) Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Matthew Brost @ 2026-07-15  8:00 UTC (permalink / raw)
  To: intel-xe, dri-devel
  Cc: Christian Koenig, Huang Rui, Matthew Auld, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	linux-kernel, stable, Thomas Hellström

ttm_pool_split_for_swap() unconditionally splits high-order pool pages
into order-0 pages before backup, so every compound the shrinker
touches is shattered even when the rest of the system would prefer it
stay intact. Under sustained kswapd pressure this fragments memory
enough to drive other parts of MM into recovery loops.

Back up each compound at its native order instead. In
ttm_pool_backup(), hand the full compound to the new
ttm_backup_backup_folio(), which backs up subpages to a contiguous
range of shmem indices and returns the base handle plus the number of
subpages actually backed up (@nr_backed). On full success, free the
compound once at its native order -- no split_page(), no per-4K
refcount juggling.

A per-folio backup can't be made fully atomic under memory pressure:
ttm_backup_backup_folio() must allocate shmem folios before source
subpages can be released, so under true OOM any subpage may fail
while the rest of the compound is still live. Two mechanisms handle
this without regressing reclaim behaviour:

  - alloc_gfp gets __GFP_NOMEMALLOC whenever order > 0 (cleared again
    for order-0), so a high-order backup fails fast with -ENOMEM
    instead of draining kernel reserves, leaving them for other
    allocations under the same pressure.

  - If ttm_backup_backup_folio() still returns a short @nr_backed with
    a valid handle for the successfully-backed prefix, split the
    source compound with ttm_pool_split_for_swap(), free the prefix as
    order-0 pages (already safely in shmem), and retry the remaining
    subpages at order 0, where __GFP_NOMEMALLOC is cleared and
    reserves may be used as a last resort.

This preserves the original split-on-OOM fallback while keeping the
common case fragmentation-free, and preserves the "partial backup is
allowed" contract (shrunken is incremented per subpage backed up).

The restore-side leftover-page split in ttm_pool_restore_commit() is
left as-is: it's unreachable in practice and not worth complicating
the restore state machine to avoid.

Testing: the existing backup_fault_inject point only truncated
tt->num_pages, which never exercised the reactive split path above
since it never left a compound partially backed up. Wire fault
injection into ttm_backup_backup_folio() itself: past the first
subpage of a compound, synthesize a -ENOMEM in place of
shmem_read_folio_gfp() when should_fail() trips, producing the same
short @nr_pages_backed a real failure would and forcing
ttm_pool_backup() through the split-and-retry path. The fault_attr
stays private to ttm_pool.c; ttm_backup.c reaches it through
ttm_backup_fault_inject_folio(), declared in ttm_pool_internal.h.

While converting the writeback branch to operate on the whole folio,
the unlock condition after shmem_writeout() also changed from `if
(ret)` to `if (ret == AOP_WRITEPAGE_ACTIVATE)`, matching the actual
contract: shmem_writeout()/swap_writeout() only leave the folio locked
when returning AOP_WRITEPAGE_ACTIVATE; any other return (including a
hard error from arch_prepare_to_swap()) means the folio was already
unlocked internally. The old `if (ret)` check would have double-
unlocked in that hard-error case.

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: stable@vger.kernel.org
Fixes: b63d715b8090 ("drm/ttm/pool, drm/ttm/tt: Provide a helper to shrink pages")
Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/ttm/ttm_backup.c        | 138 +++++++++++++++---------
 drivers/gpu/drm/ttm/ttm_pool.c          |  89 ++++++++++++---
 drivers/gpu/drm/ttm/ttm_pool_internal.h |   8 ++
 include/drm/ttm/ttm_backup.h            |  12 +--
 4 files changed, 178 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_backup.c b/drivers/gpu/drm/ttm/ttm_backup.c
index 81df4cb5606b..a8f0b12c86d2 100644
--- a/drivers/gpu/drm/ttm/ttm_backup.c
+++ b/drivers/gpu/drm/ttm/ttm_backup.c
@@ -6,23 +6,25 @@
 #include <drm/ttm/ttm_backup.h>
 
 #include <linux/export.h>
-#include <linux/page-flags.h>
 #include <linux/swap.h>
 
+#include "ttm_pool_internal.h"
+
 /*
  * Need to map shmem indices to handle since a handle value
  * of 0 means error, following the swp_entry_t convention.
  */
-static unsigned long ttm_backup_shmem_idx_to_handle(pgoff_t idx)
-{
-	return (unsigned long)idx + 1;
-}
 
 static pgoff_t ttm_backup_handle_to_shmem_idx(pgoff_t handle)
 {
 	return handle - 1;
 }
 
+static unsigned long ttm_backup_shmem_idx_to_handle(pgoff_t idx)
+{
+	return (unsigned long)idx + 1;
+}
+
 /**
  * ttm_backup_drop() - release memory associated with a handle
  * @backup: The struct backup pointer used to obtain the handle
@@ -68,17 +70,23 @@ int ttm_backup_copy_page(struct file *backup, struct page *dst,
 }
 
 /**
- * ttm_backup_backup_page() - Backup a page
+ * ttm_backup_backup_folio() - Backup a folio
  * @backup: The struct backup pointer to use.
- * @page: The page to back up.
- * @writeback: Whether to perform immediate writeback of the page.
+ * @folio: The folio to back up.
+ * @order: The allocation order of @folio.  Since TTM allocates higher-order
+ *         pages without __GFP_COMP, folio_nr_pages(@folio) would always
+ *         return 1; the caller must pass the true order explicitly.
+ * @writeback: Whether to perform immediate writeback of the folio's pages.
  * This may have performance implications.
- * @idx: A unique integer for each page and each struct backup.
+ * @idx: A unique integer for the first page of the folio and each struct backup.
  * This allows the backup implementation to avoid managing
  * its address space separately.
- * @page_gfp: The gfp value used when the page was allocated.
- * This is used for accounting purposes.
+ * @folio_gfp: The gfp value used when the folio was allocated.
+ * Currently unused.
  * @alloc_gfp: The gfp to be used when allocating memory.
+ * @nr_pages_backed: Output. On a successful return, set to the number of
+ * pages actually backed up, which may be less than (1 << @order)
+ * if an -ENOMEM was encountered mid-folio.
  *
  * Context: If called from reclaim context, the caller needs to
  * assert that the shrinker gfp has __GFP_FS set, to avoid
@@ -87,53 +95,87 @@ int ttm_backup_copy_page(struct file *backup, struct page *dst,
  * that the shrinker gfp has __GFP_IO set, since without it,
  * we're not allowed to start backup IO.
  *
- * Return: A handle on success. Negative error code on failure.
- *
- * Note: This function could be extended to back up a folio and
- * implementations would then split the folio internally if needed.
- * Drawback is that the caller would then have to keep track of
- * the folio size- and usage.
+ * Return: A handle for the first backed-up page on success (handles for
+ * subsequent pages follow sequentially). -ENOMEM if no pages could be backed
+ * up. Any other negative error code if a non-ENOMEM failure occurred; in that
+ * case any pages backed up so far are truncated before returning.
  */
 s64
-ttm_backup_backup_page(struct file *backup, struct page *page,
-		       bool writeback, pgoff_t idx, gfp_t page_gfp,
-		       gfp_t alloc_gfp)
+ttm_backup_backup_folio(struct file *backup, struct folio *folio,
+			unsigned int order, bool writeback, pgoff_t idx,
+			gfp_t folio_gfp, gfp_t alloc_gfp,
+			pgoff_t *nr_pages_backed)
 {
 	struct address_space *mapping = backup->f_mapping;
-	unsigned long handle = 0;
+	int nr_pages = 1 << order;
 	struct folio *to_folio;
-	int ret;
-
-	to_folio = shmem_read_folio_gfp(mapping, idx, alloc_gfp);
-	if (IS_ERR(to_folio))
-		return PTR_ERR(to_folio);
-
-	folio_mark_accessed(to_folio);
-	folio_lock(to_folio);
-	folio_mark_dirty(to_folio);
-	copy_highpage(folio_file_page(to_folio, idx), page);
-	handle = ttm_backup_shmem_idx_to_handle(idx);
-
-	if (writeback && !folio_mapped(to_folio) &&
-	    folio_clear_dirty_for_io(to_folio)) {
-		folio_set_reclaim(to_folio);
-		ret = shmem_writeout(to_folio, NULL, NULL);
-		if (!folio_test_writeback(to_folio))
-			folio_clear_reclaim(to_folio);
+	int ret, i;
+
+	*nr_pages_backed = 0;
+
+	for (i = 0; i < nr_pages; ) {
+		int to_nr, j;
+
 		/*
-		 * If writeout succeeds, it unlocks the folio.	errors
-		 * are otherwise dropped, since writeout is only best
-		 * effort here.
+		 * Only inject past the first subpage so *nr_pages_backed is
+		 * always > 0 here, matching a genuine mid-compound -ENOMEM
+		 * and driving the caller's reactive split fallback instead
+		 * of an early, no-progress failure.
 		 */
-		if (ret)
+		if (IS_ENABLED(CONFIG_FAULT_INJECTION) && i &&
+		    ttm_backup_fault_inject_folio())
+			to_folio = ERR_PTR(-ENOMEM);
+		else
+			to_folio = shmem_read_folio_gfp(mapping, idx + i, alloc_gfp);
+		if (IS_ERR(to_folio)) {
+			int err = PTR_ERR(to_folio);
+
+			if (err == -ENOMEM && *nr_pages_backed)
+				return ttm_backup_shmem_idx_to_handle(idx);
+
+			if (*nr_pages_backed) {
+				shmem_truncate_range(file_inode(backup),
+						     (loff_t)idx << PAGE_SHIFT,
+						     ((loff_t)(idx + i) << PAGE_SHIFT) - 1);
+				/*
+				 * The pages just truncated are no longer
+				 * backed up; don't let the caller mistake
+				 * them for valid handles.
+				 */
+				*nr_pages_backed = 0;
+			}
+			return err;
+		}
+
+		to_nr = min_t(int, nr_pages - i,
+			      folio_next_index(to_folio) - (idx + i));
+
+		folio_mark_accessed(to_folio);
+		folio_lock(to_folio);
+		folio_mark_dirty(to_folio);
+
+		for (j = 0; j < to_nr; j++)
+			copy_highpage(folio_file_page(to_folio, idx + i + j),
+				      folio_page(folio, i + j));
+
+		if (writeback && !folio_mapped(to_folio) &&
+		    folio_clear_dirty_for_io(to_folio)) {
+			folio_set_reclaim(to_folio);
+			ret = shmem_writeout(to_folio, NULL, NULL);
+			if (!folio_test_writeback(to_folio))
+				folio_clear_reclaim(to_folio);
+			if (ret == AOP_WRITEPAGE_ACTIVATE)
+				folio_unlock(to_folio);
+		} else {
 			folio_unlock(to_folio);
-	} else {
-		folio_unlock(to_folio);
-	}
+		}
 
-	folio_put(to_folio);
+		folio_put(to_folio);
+		i += to_nr;
+		*nr_pages_backed = i;
+	}
 
-	return handle;
+	return ttm_backup_shmem_idx_to_handle(idx);
 }
 
 /**
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 80d0ad41456a..1bf37023fed6 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -53,8 +53,23 @@
 #ifdef CONFIG_FAULT_INJECTION
 #include <linux/fault-inject.h>
 static DECLARE_FAULT_ATTR(backup_fault_inject);
+
+/*
+ * Exposed to ttm_backup.c so a mid-compound subpage can be made to fail
+ * with -ENOMEM, exercising the reactive split-and-retry fallback in
+ * ttm_pool_backup() for high-order backups.
+ */
+bool ttm_backup_fault_inject_folio(void)
+{
+	return should_fail(&backup_fault_inject, 1);
+}
 #else
 #define should_fail(...) false
+
+bool ttm_backup_fault_inject_folio(void)
+{
+	return false;
+}
 #endif
 
 /**
@@ -492,7 +507,7 @@ static void ttm_pool_split_for_swap(struct ttm_pool *pool, struct page *p)
 /**
  * DOC: Partial backup and restoration of a struct ttm_tt.
  *
- * Swapout using ttm_backup_backup_page() and swapin using
+ * Swapout using ttm_backup_backup_folio() and swapin using
  * ttm_backup_copy_page() may fail.
  * The former most likely due to lack of swap-space or memory, the latter due
  * to lack of memory or because of signal interruption during waits.
@@ -1050,12 +1065,12 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
 {
 	struct file *backup = tt->backup;
 	struct page *page;
-	unsigned long handle;
 	gfp_t alloc_gfp;
 	gfp_t gfp;
 	int ret = 0;
 	pgoff_t shrunken = 0;
-	pgoff_t i, num_pages;
+	pgoff_t i, j, num_pages, npages;
+	pgoff_t nr_backed;
 
 	if (WARN_ON(ttm_tt_is_backed_up(tt)))
 		return -EINVAL;
@@ -1133,9 +1148,11 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
 	if (IS_ENABLED(CONFIG_FAULT_INJECTION) && should_fail(&backup_fault_inject, 1))
 		num_pages = DIV_ROUND_UP(num_pages, 2);
 
-	for (i = 0; i < num_pages; ++i) {
-		s64 shandle;
+	for (i = 0; i < num_pages; i += npages) {
+		unsigned int order;
+		s64 handle;
 
+		npages = 1;
 		page = tt->pages[i];
 		if (unlikely(!page))
 			continue;
@@ -1144,19 +1161,61 @@ long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
 		if (unlikely(ttm_backup_page_ptr_is_handle(page)))
 			continue;
 
-		ttm_pool_split_for_swap(pool, page);
+		order = ttm_pool_page_order(pool, page);
+		npages = 1UL << order;
+
+		/*
+		 * We don't allow dipping kernel reserves for high order backup
+		 */
+		if (order)
+			alloc_gfp |= __GFP_NOMEMALLOC;
+		else
+			alloc_gfp &= ~__GFP_NOMEMALLOC;
+
+		/*
+		 * Back up the compound atomically at its native order. If
+		 * fault injection truncated num_pages mid-compound, skip
+		 * the partial tail rather than splitting.
+		 */
+		if (unlikely(i + npages > num_pages))
+			break;
 
-		shandle = ttm_backup_backup_page(backup, page, flags->writeback, i,
-						 gfp, alloc_gfp);
-		if (shandle < 0) {
-			/* We allow partially shrunken tts */
-			ret = shandle;
+		handle = ttm_backup_backup_folio(backup, page_folio(page),
+						 order, flags->writeback, i,
+						 gfp, alloc_gfp,
+						 &nr_backed);
+		/*
+		 * Zero progress on this compound (whether order 0 or a
+		 * high-order compound that failed before backing up even
+		 * its first subpage) is unrecoverable: bail out rather than
+		 * looping forever with npages == nr_backed == 0 below.
+		 */
+		if (unlikely(handle < 0 && !nr_backed)) {
+			ret = handle;
 			break;
 		}
-		handle = shandle;
-		tt->pages[i] = ttm_backup_handle_to_page_ptr(handle);
-		__free_pages_gpu_account(page, 0, false);
-		shrunken++;
+
+		for (j = 0; j < nr_backed; j++)
+			tt->pages[i + j] = ttm_backup_handle_to_page_ptr(handle + j);
+
+		shrunken += nr_backed;
+
+		if (unlikely(nr_backed < npages)) {
+			/*
+			 * Partial OOM backup: split the compound and free the
+			 * subpages whose content is now in shmem. Continue the
+			 * loop from the first un-backed order-0 page.
+			 */
+			ttm_pool_split_for_swap(pool, page);
+			for (j = 0; j < nr_backed; j++)
+				__free_pages_gpu_account(page + j, 0, false);
+			npages = nr_backed;
+			continue;
+		}
+
+		/* Fully backed up: free at native order. */
+		page->private = 0;
+		__free_pages_gpu_account(page, order, false);
 	}
 
 	return shrunken ? shrunken : ret;
diff --git a/drivers/gpu/drm/ttm/ttm_pool_internal.h b/drivers/gpu/drm/ttm/ttm_pool_internal.h
index 24c179fd69d1..cbb17a2129fe 100644
--- a/drivers/gpu/drm/ttm/ttm_pool_internal.h
+++ b/drivers/gpu/drm/ttm/ttm_pool_internal.h
@@ -22,4 +22,12 @@ static inline unsigned int ttm_pool_beneficial_order(struct ttm_pool *pool)
 	return pool->alloc_flags & 0xff;
 }
 
+/*
+ * Implemented in ttm_pool.c, used by ttm_backup.c. Returns true if a fault
+ * should be injected mid-compound to test the reactive split-and-retry
+ * fallback in ttm_pool_backup(). Always returns false when
+ * CONFIG_FAULT_INJECTION is disabled.
+ */
+bool ttm_backup_fault_inject_folio(void);
+
 #endif
diff --git a/include/drm/ttm/ttm_backup.h b/include/drm/ttm/ttm_backup.h
index 29b9c855af77..49efa713e87c 100644
--- a/include/drm/ttm/ttm_backup.h
+++ b/include/drm/ttm/ttm_backup.h
@@ -13,9 +13,8 @@
  * ttm_backup_handle_to_page_ptr() - Convert handle to struct page pointer
  * @handle: The handle to convert.
  *
- * Converts an opaque handle received from the
- * ttm_backup_backup_page() function to an (invalid)
- * struct page pointer suitable for a struct page array.
+ * Converts an opaque handle received from a ttm_backup_backup_*()
+ * function to an (invalid) struct page pointer suitable for a struct page array.
  *
  * Return: An (invalid) struct page pointer.
  */
@@ -59,9 +58,10 @@ int ttm_backup_copy_page(struct file *backup, struct page *dst,
 			 pgoff_t handle, bool intr, gfp_t additional_gfp);
 
 s64
-ttm_backup_backup_page(struct file *backup, struct page *page,
-		       bool writeback, pgoff_t idx, gfp_t page_gfp,
-		       gfp_t alloc_gfp);
+ttm_backup_backup_folio(struct file *backup, struct folio *folio,
+			unsigned int order, bool writeback, pgoff_t idx,
+			gfp_t folio_gfp, gfp_t alloc_gfp,
+			pgoff_t *nr_pages_backed);
 
 void ttm_backup_fini(struct file *backup);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-07-16 10:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15  8:00 [PATCH v7] drm/ttm/pool: back up at native page order Matthew Brost
2026-07-15  8:08 ` ✓ CI.KUnit: success for drm/ttm/pool: back up at native page order (rev3) Patchwork
2026-07-15  8:57 ` ✓ Xe.CI.BAT: " Patchwork
2026-07-15  9:28 ` ✓ Xe.CI.FULL: " Patchwork
2026-07-15 16:13 ` [PATCH v7] drm/ttm/pool: back up at native page order Thomas Hellström
2026-07-15 18:51   ` Matthew Brost
2026-07-16 10:16     ` Thomas Hellström

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox