[PATCH v2 1/1] iomap: fix iomap_read_end() for already uptodate folios

[Joanne Koong <joannelkoong@gmail.com>]

There are some cases where when iomap_read_end() is called, the folio
may already have been marked uptodate. For example, if the iomap block
needed zeroing, then the folio may have been marked uptodate after the
zeroing.

iomap_read_end() should unlock the folio instead of calling
folio_end_read(), which is how these cases were handled prior to commit
f8eaf79406fe ("iomap: simplify ->read_folio_range() error handling for
reads"). Calling folio_end_read() on an uptodate folio leads to buggy
behavior where marking an already uptodate folio as uptodate will XOR it
to be marked nonuptodate.

Fixes: f8eaf79406fe ("iomap: simplify ->read_folio_range() error handling for reads")
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Reported-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Tested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/iomap/buffered-io.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 0475d949e5a0..c3e73203809d 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -459,25 +459,26 @@ static void iomap_read_end(struct folio *folio, size_t bytes_submitted)
 		spin_lock_irq(&ifs->state_lock);
 		if (!ifs->read_bytes_pending) {
 			WARN_ON_ONCE(bytes_submitted);
-			end_read = true;
-		} else {
-			/*
-			 * Subtract any bytes that were initially accounted to
-			 * read_bytes_pending but skipped for IO. The +1
-			 * accounts for the bias we added in iomap_read_init().
-			 */
-			size_t bytes_not_submitted = folio_size(folio) + 1 -
-					bytes_submitted;
-			ifs->read_bytes_pending -= bytes_not_submitted;
-			/*
-			 * If !ifs->read_bytes_pending, this means all pending
-			 * reads by the IO helper have already completed, which
-			 * means we need to end the folio read here. If
-			 * ifs->read_bytes_pending != 0, the IO helper will end
-			 * the folio read.
-			 */
-			end_read = !ifs->read_bytes_pending;
+			spin_unlock_irq(&ifs->state_lock);
+			folio_unlock(folio);
+			return;
 		}
+
+		/*
+		 * Subtract any bytes that were initially accounted to
+		 * read_bytes_pending but skipped for IO. The +1 accounts for
+		 * the bias we added in iomap_read_init().
+		 */
+		ifs->read_bytes_pending -=
+			(folio_size(folio) + 1 - bytes_submitted);
+
+		/*
+		 * If !ifs->read_bytes_pending, this means all pending reads by
+		 * the IO helper have already completed, which means we need to
+		 * end the folio read here. If ifs->read_bytes_pending != 0,
+		 * the IO helper will end the folio read.
+		 */
+		end_read = !ifs->read_bytes_pending;
 		if (end_read)
 			uptodate = ifs_is_fully_uptodate(folio, ifs);
 		spin_unlock_irq(&ifs->state_lock);
-- 
2.47.3