[PATCH] drm/i915: look up shmem folios by index for writeback

[Matthias Goergens]

Since commit 776a853a43c9 ("i915: Use writeback_iter()"),
__shmem_writeback() starts writeback on no folios at all.  In
WB_SYNC_NONE mode writeback_iter() only returns folios tagged
PAGECACHE_TAG_DIRTY, and shmem never sets that tag: it dirties folios
with noop_dirty_folio(), because shmem relies on LRU-based swap writeout
rather than on the dirty tag (see the comment in __folio_mark_dirty() in
mm/page-writeback.c).  The loop before that commit looked pages up by
index and tested the page dirty flag, so it did write.

This affects the shrinker's I915_SHRINK_WRITEBACK pass and the i915 TTM
backend, which both call __shmem_writeback().  The pages stay dirty and
on the LRU, so reclaim still swaps them out later; what is lost is the
early writeback.

Go back to walking the object by index, with the folio API.  Mapped
folios are still skipped.  Unlike the old loop, do not wait for a folio
lock, since this also runs from the OOM notifier; a locked folio is left
to reclaim.  shmem_write_folio() returns with the folio unlocked except
for AOP_WRITEPAGE_ACTIVATE, so unlock it only in that case.  It may
also split a large folio, so read the folio size only after writing, to
find the next index.

I came across this while measuring an earlier patch of mine to this
loop, which changed nothing because the loop never visits a folio.
This patch was measured without Intel hardware, with a test-only mock
selftest in a QEMU guest with swap.  Dirty, unpinned objects of 1 to
256 MiB, with and without THP, went through i915_gem_shrink() with
I915_SHRINK_WRITEBACK.  Before this patch, no page was written in any
case; with it, every page that was not mmapped was.  The contents read
back intact after swap-in, and the mock selftests give the same results
as before.  This has not been tested on hardware.

Fixes: 776a853a43c9 ("i915: Use writeback_iter()")
Cc: stable@xxxxxxxxxxxxxxx # v6.16+
Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
---
This replaces my two earlier patches to this loop, which can be dropped:
the loop they change never visits a folio.

https://lore.kernel.org/all/20260915062924.2410550-1-matthias.goergens@xxxxxxxxx/
https://lore.kernel.org/all/20260914105606.3997649-1-matthias.goergens@xxxxxxxxx/

In 6.18.y and 7.2.y the call is shmem_writeout(folio, NULL, NULL)
instead of shmem_write_folio(folio), with the same return convention.

In review of 776a853a43c9, Christoph asked for this loop to move behind
a shmem API instead of living in drivers, and Matthew agreed:

https://lore.kernel.org/all/Z--XtaM7Z3zbjzAu@xxxxxxxxxxxxx/
https://lore.kernel.org/all/Z-_hQwNeiOnNYJVp@xxxxxxxxxxxxxxxxxxxx/

I kept this fix inside i915 so that it is one patch for stable.

The test harness is not part of the patch; I can post it if that helps.
Testing on Intel hardware under memory pressure would be very welcome.

 drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 56 ++++++++++++++++++-----
 1 file changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index ef9440166295..54424e434f3b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -304,28 +304,60 @@ shmem_truncate(struct drm_i915_gem_object *obj)
 	return 0;
 }
 
+/* Start writing a locked folio to swap.  It is unlocked on return. */
+static void shmem_writeback_folio(struct folio *folio)
+{
+	int ret;
+
+	folio_set_reclaim(folio);
+	ret = shmem_write_folio(folio);
+	if (!folio_test_writeback(folio))
+		folio_clear_reclaim(folio);
+
+	/* shmem_write_folio() unlocks the folio unless it returns this. */
+	if (ret == AOP_WRITEPAGE_ACTIVATE)
+		folio_unlock(folio);
+}
+
 void __shmem_writeback(size_t size, struct address_space *mapping)
 {
-	struct writeback_control wbc = {
-		.sync_mode = WB_SYNC_NONE,
-		.nr_to_write = SWAP_CLUSTER_MAX,
-		.range_start = 0,
-		.range_end = LLONG_MAX,
-	};
-	struct folio *folio = NULL;
-	int error = 0;
+	pgoff_t nr_pages = size >> PAGE_SHIFT;
+	pgoff_t index = 0;
 
 	/*
+	 * shmem marks folios dirty with noop_dirty_folio(), which does not
+	 * set PAGECACHE_TAG_DIRTY, so writeback_iter() would find none of
+	 * them.  Look up each folio of the object by index and test its
+	 * dirty flag.
+	 *
 	 * Leave mmapings intact (GTT will have been revoked on unbinding,
 	 * leaving only CPU mmapings around) and add those folios to the LRU
 	 * instead of invoking writeback so they are aged and paged out
 	 * as normal.
 	 */
-	while ((folio = writeback_iter(mapping, &wbc, folio, &error))) {
-		if (folio_mapped(folio))
-			folio_redirty_for_writepage(&wbc, folio);
+	while (index < nr_pages) {
+		struct folio *folio;
+
+		/*
+		 * Skip folios that are absent or locked: this runs from the
+		 * shrinker and the OOM notifier, so do not wait for a lock.
+		 */
+		folio = __filemap_get_folio(mapping, index,
+					    FGP_LOCK | FGP_NOWAIT, 0);
+		if (IS_ERR(folio)) {
+			index++;
+			continue;
+		}
+
+		index = folio->index;
+		if (!folio_mapped(folio) && folio_clear_dirty_for_io(folio))
+			shmem_writeback_folio(folio);
 		else
-			error = shmem_write_folio(folio);
+			folio_unlock(folio);
+
+		/* Writing may have split the folio, so read its size only now. */
+		index += folio_nr_pages(folio);
+		folio_put(folio);
 	}
 }
 

base-commit: ce1e0223d8ad4211275c82a17ed6d43ab81e13d9
-- 
2.56.0