Re: [PATCH v2 5/5] squashfs: Convert squashfs_fill_page() to take a folio

[Matthew Wilcox <willy@infradead.org>]

On Thu, Jan 09, 2025 at 05:34:43PM +0000, Ryan Roberts wrote:
> I started getting a different oops after I fixed this issue. It turns out that
> __filemap_get_folio() returns ERR_PTR() on error whereas
> grab_cache_page_nowait() (used previously) returns NULL. So the continue
> condition needs to change. This fixes both for me:

Hey Ryan, can you try this amalgam of three patches?
If it works out, I'll send the two which aren't yet in akpm's tree to
him.

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index da25d6fa45ce..5ca2baa16dc2 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -400,7 +400,7 @@ void squashfs_copy_cache(struct folio *folio,
 			bytes -= PAGE_SIZE, offset += PAGE_SIZE) {
 		struct folio *push_folio;
 		size_t avail = buffer ? min(bytes, PAGE_SIZE) : 0;
-		bool uptodate = true;
+		bool updated = false;
 
 		TRACE("bytes %zu, i %d, available_bytes %zu\n", bytes, i, avail);
 
@@ -409,15 +409,15 @@ void squashfs_copy_cache(struct folio *folio,
 					FGP_LOCK|FGP_CREAT|FGP_NOFS|FGP_NOWAIT,
 					mapping_gfp_mask(mapping));
 
-		if (!push_folio)
+		if (IS_ERR(push_folio))
 			continue;
 
 		if (folio_test_uptodate(push_folio))
 			goto skip_folio;
 
-		uptodate = squashfs_fill_page(push_folio, buffer, offset, avail);
+		updated = squashfs_fill_page(push_folio, buffer, offset, avail);
 skip_folio:
-		folio_end_read(push_folio, uptodate);
+		folio_end_read(push_folio, updated);
 		if (i != folio->index)
 			folio_put(push_folio);
 	}
diff --git a/mm/filemap.c b/mm/filemap.c
index 12ba297bb85e..3b1eefa9aab8 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1505,7 +1505,7 @@ void folio_end_read(struct folio *folio, bool success)
 	/* Must be in bottom byte for x86 to work */
 	BUILD_BUG_ON(PG_uptodate > 7);
 	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
-	VM_BUG_ON_FOLIO(folio_test_uptodate(folio), folio);
+	VM_BUG_ON_FOLIO(success && folio_test_uptodate(folio), folio);
 
 	if (likely(success))
 		mask |= 1 << PG_uptodate;