* Re: [PATCH v2] btrfs: force a transaction commit when fsyncing stripe-tree extents
2026-10-22 9:06 [PATCH v2] btrfs: force a transaction commit when fsyncing stripe-tree extents Johannes Thumshirn
@ 2026-10-22 9:39 ` Damien Le Moal
2026-10-22 14:17 ` Filipe Manana
2026-10-22 21:09 ` Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Damien Le Moal @ 2026-10-22 9:39 UTC (permalink / raw)
To: Johannes Thumshirn, linux-btrfs; +Cc: Filipe Manana, Naohiro Aota
On 2026/10/22 11:06, Johannes Thumshirn wrote:
> An extent in a block-group that uses the RAID stripe-tree has its logical
> to physical mapping stored in that tree, which only becomes persistent
> when the stripe-tree is written out at transaction commit. But the RAID
> stripe-tree is not logged in the log tree.
>
> A fsync therefore logs a file-extent item whose logical range cannot be
> resolved after a crash. Replay restores the file-extent item and the
> file has the correct size, but reading it fails with EIO and nothing is
> reported.
>
> Force a full commit when logging such an extent. Extents reach the log
> both from log_one_extent() and from copy_items(), so check in both.
> copy_items() skips extents from a past transaction, their mapping is
> already committed, the same reason their checksums are not logged.
>
> Fixes: 02c372e1f016 ("btrfs: add support for inserting raid stripe extents")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>
> Changes to v1:
> - Don't just fix the fast fsync
>
> fs/btrfs/tree-log.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 1d9dfb63fc5a..e50f924bca59 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -30,6 +30,8 @@
> #include "print-tree.h"
> #include "tree-checker.h"
> #include "delayed-inode.h"
> +#include "volumes.h"
> +#include "raid-stripe-tree.h"
>
> #define MAX_CONFLICT_INODES 10
>
> @@ -4741,6 +4743,24 @@ static int log_csums(struct btrfs_trans_handle *trans,
> return ret;
> }
>
> +static bool log_extent_needs_stripe_tree(struct btrfs_fs_info *fs_info,
> + u64 disk_bytenr)
> +{
> + struct btrfs_chunk_map *map __free(btrfs_free_chunk_map) = NULL;
> +
> + if (!btrfs_fs_incompat(fs_info, RAID_STRIPE_TREE))
> + return false;
> +
> + if (disk_bytenr == 0 || disk_bytenr >= EXTENT_MAP_LAST_BYTE)
> + return false;
> +
> + map = btrfs_find_chunk_map(fs_info, disk_bytenr, 1);
> + if (!map)
> + return false;
> +
> + return btrfs_need_stripe_tree_update(fs_info, map->type);
> +}
> +
> static noinline int copy_items(struct btrfs_trans_handle *trans,
> struct btrfs_inode *inode,
> struct btrfs_path *dst_path,
> @@ -4847,6 +4867,14 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
> inode->last_reflink_trans < trans->transid)
> continue;
>
> + if (!is_old_extent &&
> + btrfs_file_extent_type(src, extent) != BTRFS_FILE_EXTENT_INLINE) {
> + u64 bytenr = btrfs_file_extent_disk_bytenr(src, extent);
> +
> + if (log_extent_needs_stripe_tree(trans->fs_info, bytenr))
> + return BTRFS_LOG_FORCE_COMMIT;
> + }
Might be good to add a test on
btrfs_fs_incompat(fs_info, RAID_STRIPE_TREE)
as the first condition in the first if to avoid doing all the tests and calls in
the regular (no RAID) case, no?
But if this is not the hot path, maybe it doe not matter.
> +
> if (skip_csum)
> goto add_to_batch;
>
> @@ -5124,6 +5152,9 @@ static int log_one_extent(struct btrfs_trans_handle *trans,
> u64 block_len;
> int ret;
>
> + if (log_extent_needs_stripe_tree(inode->root->fs_info, em->disk_bytenr))
> + return BTRFS_LOG_FORCE_COMMIT;
> +
> btrfs_set_stack_file_extent_generation(&fi, trans->transid);
> if (em->flags & EXTENT_FLAG_PREALLOC)
> btrfs_set_stack_file_extent_type(&fi, BTRFS_FILE_EXTENT_PREALLOC);
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs: force a transaction commit when fsyncing stripe-tree extents
2026-10-22 9:06 [PATCH v2] btrfs: force a transaction commit when fsyncing stripe-tree extents Johannes Thumshirn
2026-10-22 9:39 ` Damien Le Moal
@ 2026-10-22 14:17 ` Filipe Manana
2026-10-22 21:09 ` Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Filipe Manana @ 2026-10-22 14:17 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: linux-btrfs, Damien Le Moal, Filipe Manana, Naohiro Aota
On Thu, Oct 22, 2026 at 10:07 AM Johannes Thumshirn
<johannes.thumshirn@wdc.com> wrote:
>
> An extent in a block-group that uses the RAID stripe-tree has its logical
> to physical mapping stored in that tree, which only becomes persistent
> when the stripe-tree is written out at transaction commit. But the RAID
> stripe-tree is not logged in the log tree.
>
> A fsync therefore logs a file-extent item whose logical range cannot be
> resolved after a crash. Replay restores the file-extent item and the
> file has the correct size, but reading it fails with EIO and nothing is
> reported.
>
> Force a full commit when logging such an extent. Extents reach the log
> both from log_one_extent() and from copy_items(), so check in both.
> copy_items() skips extents from a past transaction, their mapping is
> already committed, the same reason their checksums are not logged.
>
> Fixes: 02c372e1f016 ("btrfs: add support for inserting raid stripe extents")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Looks good now, thanks.
> ---
>
> Changes to v1:
> - Don't just fix the fast fsync
>
> fs/btrfs/tree-log.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 1d9dfb63fc5a..e50f924bca59 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -30,6 +30,8 @@
> #include "print-tree.h"
> #include "tree-checker.h"
> #include "delayed-inode.h"
> +#include "volumes.h"
> +#include "raid-stripe-tree.h"
>
> #define MAX_CONFLICT_INODES 10
>
> @@ -4741,6 +4743,24 @@ static int log_csums(struct btrfs_trans_handle *trans,
> return ret;
> }
>
> +static bool log_extent_needs_stripe_tree(struct btrfs_fs_info *fs_info,
> + u64 disk_bytenr)
> +{
> + struct btrfs_chunk_map *map __free(btrfs_free_chunk_map) = NULL;
> +
> + if (!btrfs_fs_incompat(fs_info, RAID_STRIPE_TREE))
> + return false;
> +
> + if (disk_bytenr == 0 || disk_bytenr >= EXTENT_MAP_LAST_BYTE)
> + return false;
> +
> + map = btrfs_find_chunk_map(fs_info, disk_bytenr, 1);
> + if (!map)
> + return false;
> +
> + return btrfs_need_stripe_tree_update(fs_info, map->type);
> +}
> +
> static noinline int copy_items(struct btrfs_trans_handle *trans,
> struct btrfs_inode *inode,
> struct btrfs_path *dst_path,
> @@ -4847,6 +4867,14 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
> inode->last_reflink_trans < trans->transid)
> continue;
>
> + if (!is_old_extent &&
> + btrfs_file_extent_type(src, extent) != BTRFS_FILE_EXTENT_INLINE) {
> + u64 bytenr = btrfs_file_extent_disk_bytenr(src, extent);
> +
> + if (log_extent_needs_stripe_tree(trans->fs_info, bytenr))
> + return BTRFS_LOG_FORCE_COMMIT;
> + }
> +
> if (skip_csum)
> goto add_to_batch;
>
> @@ -5124,6 +5152,9 @@ static int log_one_extent(struct btrfs_trans_handle *trans,
> u64 block_len;
> int ret;
>
> + if (log_extent_needs_stripe_tree(inode->root->fs_info, em->disk_bytenr))
> + return BTRFS_LOG_FORCE_COMMIT;
> +
> btrfs_set_stack_file_extent_generation(&fi, trans->transid);
> if (em->flags & EXTENT_FLAG_PREALLOC)
> btrfs_set_stack_file_extent_type(&fi, BTRFS_FILE_EXTENT_PREALLOC);
> --
> 2.56.0
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs: force a transaction commit when fsyncing stripe-tree extents
2026-10-22 9:06 [PATCH v2] btrfs: force a transaction commit when fsyncing stripe-tree extents Johannes Thumshirn
2026-10-22 9:39 ` Damien Le Moal
2026-10-22 14:17 ` Filipe Manana
@ 2026-10-22 21:09 ` Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-10-22 21:09 UTC (permalink / raw)
To: Johannes Thumshirn, linux-btrfs
Cc: Damien Le Moal, Filipe Manana, Naohiro Aota
在 2026/10/22 19:36, Johannes Thumshirn 写道:
> An extent in a block-group that uses the RAID stripe-tree has its logical
> to physical mapping stored in that tree, which only becomes persistent
> when the stripe-tree is written out at transaction commit. But the RAID
> stripe-tree is not logged in the log tree.
>
> A fsync therefore logs a file-extent item whose logical range cannot be
> resolved after a crash. Replay restores the file-extent item and the
> file has the correct size, but reading it fails with EIO and nothing is
> reported.
>
> Force a full commit when logging such an extent. Extents reach the log
> both from log_one_extent() and from copy_items(), so check in both.
> copy_items() skips extents from a past transaction, their mapping is
> already committed, the same reason their checksums are not logged.
>
> Fixes: 02c372e1f016 ("btrfs: add support for inserting raid stripe extents")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
> ---
>
> Changes to v1:
> - Don't just fix the fast fsync
>
> fs/btrfs/tree-log.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 1d9dfb63fc5a..e50f924bca59 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -30,6 +30,8 @@
> #include "print-tree.h"
> #include "tree-checker.h"
> #include "delayed-inode.h"
> +#include "volumes.h"
> +#include "raid-stripe-tree.h"
>
> #define MAX_CONFLICT_INODES 10
>
> @@ -4741,6 +4743,24 @@ static int log_csums(struct btrfs_trans_handle *trans,
> return ret;
> }
>
> +static bool log_extent_needs_stripe_tree(struct btrfs_fs_info *fs_info,
> + u64 disk_bytenr)
> +{
> + struct btrfs_chunk_map *map __free(btrfs_free_chunk_map) = NULL;
> +
> + if (!btrfs_fs_incompat(fs_info, RAID_STRIPE_TREE))
> + return false;
> +
> + if (disk_bytenr == 0 || disk_bytenr >= EXTENT_MAP_LAST_BYTE)
> + return false;
> +
> + map = btrfs_find_chunk_map(fs_info, disk_bytenr, 1);
> + if (!map)
> + return false;
> +
> + return btrfs_need_stripe_tree_update(fs_info, map->type);
> +}
> +
> static noinline int copy_items(struct btrfs_trans_handle *trans,
> struct btrfs_inode *inode,
> struct btrfs_path *dst_path,
> @@ -4847,6 +4867,14 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
> inode->last_reflink_trans < trans->transid)
> continue;
>
> + if (!is_old_extent &&
> + btrfs_file_extent_type(src, extent) != BTRFS_FILE_EXTENT_INLINE) {
> + u64 bytenr = btrfs_file_extent_disk_bytenr(src, extent);
> +
> + if (log_extent_needs_stripe_tree(trans->fs_info, bytenr))
> + return BTRFS_LOG_FORCE_COMMIT;
> + }
> +
> if (skip_csum)
> goto add_to_batch;
>
> @@ -5124,6 +5152,9 @@ static int log_one_extent(struct btrfs_trans_handle *trans,
> u64 block_len;
> int ret;
>
> + if (log_extent_needs_stripe_tree(inode->root->fs_info, em->disk_bytenr))
> + return BTRFS_LOG_FORCE_COMMIT;
> +
> btrfs_set_stack_file_extent_generation(&fi, trans->transid);
> if (em->flags & EXTENT_FLAG_PREALLOC)
> btrfs_set_stack_file_extent_type(&fi, BTRFS_FILE_EXTENT_PREALLOC);
^ permalink raw reply [flat|nested] 4+ messages in thread