[PATCH v2 3/5] btrfs: wait for ordered extent IO in fast fsync

[Josef Bacik <josef@toxicpanda.com>]

A fast fsync waits for writeback and then checks
BTRFS_INODE_COW_WRITE_ERROR to see whether a COW write failed. If one
did, it must wait for the ordered extents to complete before logging,
since until then the extent maps of the failed range point to extents
that were never written. Logging them puts unwritten extents in place
of the data that was there before when the log is replayed.

The flag can be set too late for that check. end_bbio_data_write()
clears writeback before it finishes the ordered extent:

  end_bbio_data_write()            fsync

  btrfs_folio_clear_writeback()
                                   filemap_fdatawait_range()
                                   BTRFS_INODE_COW_WRITE_ERROR clear
                                   btrfs_log_changed_extents()
  btrfs_finish_ordered_extent()
    set BTRFS_INODE_COW_WRITE_ERROR

The write paths that fail before submitting a bio, writepage_delalloc()
and submit_write_sector(), clear writeback and finish the ordered
extent through btrfs_mark_ordered_io_finished(), which never sets the
flag at all.

Fix by dropping the flag and having a fast fsync wait for
BTRFS_ORDERED_IO_DONE on the ordered extents it collected for logging.
can_finish_ordered_extent() sets it only after recording any error with
BTRFS_ORDERED_IOERR. The wait costs little, because IO_DONE is set
right after writeback is cleared, not when the ordered extent completes.

Fixes: f13e01b89daf ("btrfs: ensure fast fsync waits for ordered extents after a write failure")
Fixes: a11d6912fdd9 ("btrfs: remove folio parameter from ordered io related functions")
CC: stable@vger.kernel.org # 6.10+
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/btrfs_inode.h  | 10 ----------
 fs/btrfs/file.c         | 52 +++++++++++++++++++++++++++++++++++++++----------
 fs/btrfs/ordered-data.c | 31 -----------------------------
 3 files changed, 42 insertions(+), 51 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f7aa0d145185..65ba8ec23258 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -89,16 +89,6 @@ enum {
 	BTRFS_INODE_FREE_SPACE_INODE,
 	/* Set when there are no capabilities in XATTs for the inode. */
 	BTRFS_INODE_NO_CAP_XATTR,
-	/*
-	 * Set if an error happened when doing a COW write before submitting a
-	 * bio or during writeback. Used for both buffered writes and direct IO
-	 * writes. This is to signal a fast fsync that it has to wait for
-	 * ordered extents to complete and therefore not log extent maps that
-	 * point to unwritten extents (when an ordered extent completes and it
-	 * has the BTRFS_ORDERED_IOERR flag set, it drops extent maps in its
-	 * range).
-	 */
-	BTRFS_INODE_COW_WRITE_ERROR,
 	/*
 	 * Set by reflink while it holds the source's VFS inode lock shared.
 	 * Direct IO writes also take that lock shared; one that finds this bit
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index f978c6524aa0..4ae9aed8993f 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1512,6 +1512,42 @@ static inline bool skip_inode_logging(const struct btrfs_log_ctx *ctx)
 	return false;
 }
 
+/*
+ * Wait for the IO of the ordered extents collected for logging to be done, and
+ * return true if a COW write among them failed.
+ *
+ * Waiting for writeback is not enough to know that: the end IO handlers clear
+ * writeback before they account the IO in the ordered extent. An ordered
+ * extent gets BTRFS_ORDERED_IO_DONE only once all of its IO is accounted, and
+ * can_finish_ordered_extent() sets BTRFS_ORDERED_IOERR before that.
+ */
+static bool cow_write_failed(struct btrfs_inode *inode,
+			     struct list_head *ordered_extents)
+{
+	struct btrfs_ordered_extent *ordered;
+	bool failed = false;
+
+	list_for_each_entry(ordered, ordered_extents, log_list)
+		wait_event(ordered->wait,
+			   test_bit(BTRFS_ORDERED_IO_DONE, &ordered->flags));
+
+	/*
+	 * can_finish_ordered_extent() records a write error before it sets
+	 * BTRFS_ORDERED_IO_DONE, both under this lock.
+	 */
+	spin_lock(&inode->ordered_tree_lock);
+	list_for_each_entry(ordered, ordered_extents, log_list) {
+		if (test_bit(BTRFS_ORDERED_IOERR, &ordered->flags) &&
+		    !test_bit(BTRFS_ORDERED_NOCOW, &ordered->flags)) {
+			failed = true;
+			break;
+		}
+	}
+	spin_unlock(&inode->ordered_tree_lock);
+
+	return failed;
+}
+
 /*
  * fsync call for both files and directories.  This logs the inode into
  * the tree log instead of forcing full commits whenever possible.
@@ -1615,7 +1651,7 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	 * IO of a lower priority task while holding a transaction open.
 	 *
 	 * For a full fsync we wait for the ordered extents to complete while
-	 * for a fast fsync we wait just for writeback to complete, and then
+	 * for a fast fsync we wait just for their IO to complete, and then
 	 * attach the ordered extents to the transaction so that a transaction
 	 * commit waits for their completion, to avoid data loss if we fsync,
 	 * the current transaction commits before the ordered extents complete
@@ -1627,7 +1663,6 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 	 */
 	if (full_sync || btrfs_is_zoned(fs_info)) {
 		ret = btrfs_wait_ordered_range(inode, start, len);
-		clear_bit(BTRFS_INODE_COW_WRITE_ERROR, &inode->runtime_flags);
 	} else {
 		/*
 		 * Get our ordered extents as soon as possible to avoid doing
@@ -1640,15 +1675,12 @@ int btrfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
 			goto out_release_extents;
 
 		/*
-		 * Check and clear the BTRFS_INODE_COW_WRITE_ERROR now after
-		 * starting and waiting for writeback, because for buffered IO
-		 * it may have been set during the end IO callback
-		 * (end_bbio_data_write() -> btrfs_finish_ordered_extent()) in
-		 * case an error happened and we need to wait for ordered
-		 * extents to complete so that any extent maps that point to
-		 * unwritten locations are dropped and we don't log them.
+		 * If a COW write failed, before submitting a bio or during IO,
+		 * the extent maps of its range point to unwritten locations
+		 * until the ordered extent completes and drops them. Wait for
+		 * that, so that we don't log them.
 		 */
-		if (test_and_clear_bit(BTRFS_INODE_COW_WRITE_ERROR, &inode->runtime_flags))
+		if (cow_write_failed(inode, &ctx.ordered_extents))
 			ret = btrfs_wait_ordered_range(inode, start, len);
 	}
 
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index df74c75d6c29..27aca31866fd 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -436,37 +436,6 @@ void btrfs_finish_ordered_extent(struct btrfs_ordered_extent *ordered,
 					uptodate);
 	spin_unlock(&inode->ordered_tree_lock);
 
-	/*
-	 * If this is a COW write it means we created new extent maps for the
-	 * range and they point to unwritten locations if we got an error either
-	 * before submitting a bio or during IO.
-	 *
-	 * We have marked the ordered extent with BTRFS_ORDERED_IOERR, and we
-	 * are queuing its completion below. During completion, at
-	 * btrfs_finish_one_ordered(), we will drop the extent maps for the
-	 * unwritten extents.
-	 *
-	 * However because completion runs in a work queue we can end up having
-	 * a fast fsync running before that. In the case of direct IO, once we
-	 * unlock the inode the fsync might start, and we queue the completion
-	 * before unlocking the inode. In the case of buffered IO when writeback
-	 * finishes (end_bbio_data_write()) we queue the completion, so if the
-	 * writeback was triggered by a fast fsync, the fsync might start
-	 * logging before ordered extent completion runs in the work queue.
-	 *
-	 * The fast fsync will log file extent items based on the extent maps it
-	 * finds, so if by the time it collects extent maps the ordered extent
-	 * completion didn't happen yet, it will log file extent items that
-	 * point to unwritten extents, resulting in a corruption if a crash
-	 * happens and the log tree is replayed. Note that a fast fsync does not
-	 * wait for completion of ordered extents in order to reduce latency.
-	 *
-	 * Set a flag in the inode so that the next fast fsync will wait for
-	 * ordered extents to complete before starting to log.
-	 */
-	if (!uptodate && !test_bit(BTRFS_ORDERED_NOCOW, &ordered->flags))
-		set_bit(BTRFS_INODE_COW_WRITE_ERROR, &inode->runtime_flags);
-
 	if (ret)
 		btrfs_queue_ordered_fn(ordered);
 }

-- 
2.55.0