Re: [PATCH 3/5] btrfs: make fsync wait after pre-submission errors

[Filipe Manana]

On Fri, Oct 30, 2026 at 2:39 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> Commit f13e01b89daf ("btrfs: ensure fast fsync waits for ordered extents
> after a write failure") made btrfs_finish_ordered_extent() set
> BTRFS_INODE_COW_WRITE_ERROR in the inode when a COW write fails. The flag
> makes the next fast fsync wait for the completion of the ordered extents,
> which is where the extent maps that point to unwritten extents are
> dropped. Otherwise the fsync logs file extent items for them, which is a
> corruption if the log is replayed.
>
> As the comment there says, this is also needed for errors that happen
> before a bio is submitted. But for buffered writes those don't go through
> btrfs_finish_ordered_extent(). When running delalloc fails for a folio
> that already got ordered extents, or a block can't be submitted,
> writepage_delalloc(), submit_write_sector(), extent_write_locked_range()
> and btrfs_cleanup_ordered_extents() call btrfs_mark_ordered_io_finished(),
> which marks the ordered extent with BTRFS_ORDERED_IOERR and queues its
> completion without setting the flag:
>
>   writeback (or a first fsync)             fsync
>
>   writepage_delalloc()
>     cow_file_range() for range A
>       creates extent map and ordered
>       extent
>     cow_file_range() for range B fails
>     btrfs_mark_ordered_io_finished(A)
>       queues the completion
>                                            no dirty folios, no writeback
>                                            flag is not set, does not wait
>                                            btrfs_log_changed_extents()
>                                              logs the extent map of A
>                                            btrfs_sync_log()
>   btrfs_finish_one_ordered()
>     drops the extent map
>     frees the reserved extent
>
> If the error was already reported, for example to a previous fsync that
> triggered the writeback, this fsync even returns success. After a power
> failure, replaying the log replaces data that was durable before with the
> unwritten extent, for which there are no checksums:
>
>   BTRFS warning: csum hole found for disk bytenr range [14434304, 14438400)
>   BTRFS warning: csum failed root 5 ino 267 off 0 csum 0x8941f998 expected
>   csum 0x00000000 mirror 1
>
> Both functions share can_finish_ordered_extent(), so set the flag there.

The idea seems good to me, but as Qu pointed out, sashiko has a valid point.
The writeback bit is cleared before we set the _COW_WRITE_ERROR flag,
so a concurrent fsync may not see the flag since it does not have to
wait for writeback,
but that also happens before this patch.

Thanks.


>
> Fixes: f13e01b89daf ("btrfs: ensure fast fsync waits for ordered extents after a write failure")
> CC: stable@xxxxxxxxxxxxxxx # 6.10+
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
>  fs/btrfs/ordered-data.c | 62 ++++++++++++++++++++++++-------------------------
>  1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index df74c75d6c29..6543e99055f2 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -401,6 +401,37 @@ static bool can_finish_ordered_extent(struct btrfs_ordered_extent *ordered,
>         if (!uptodate)
>                 btrfs_mark_ordered_extent_error(ordered);
>
> +       /*
> +        * 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 once all its IO is done. 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 (ordered->bytes_left)
>                 return false;
>
> @@ -436,37 +467,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
>
>