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

[Filipe Manana]

On Tue, Nov 3, 2026 at 3:57 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxx # 6.10+
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

The idea looks sound to me.

Sashiko found something and from a quick glance it seems valid to me:

https://sashiko.dev/#/patchset/20261103-b4-btrfs-data-integrity-v2-0-577dc2690061%40toxicpanda.com

Thanks.


> ---
>  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
>
>