Re: [PATCH v7] drm/ttm/pool: back up at native page order

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On 7/15/26 10:00, Matthew Brost wrote:
> 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;
> +}
> +

This seems like an unrelated change?


>   /**
>    * 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);
Actually, it may be that the whole above block (setting reclaim etc) is 
duplicating what shmem_writeout already does and that got missed in the 
conversion to shmem_writeout. In any case, that might belong in a 
separate patch. Note also that if shmem happened to allocate a 2M page, 
and we call shmem_writeout on it, it might be split on further copies to 
it, like if we're trying to back up a 4K-populated bo to a 2M shmem 
page. Also pre-existing.


> +		} 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);
>   
Otherwise LGTM.

/Thomas