On Tue, Jun 07, 2022 at 01:09:13PM +0800, Qu Wenruo wrote:
> [BUG]
> After creating a dirty log tree, although
> btrfs_super_block::log_root and log_root_level is correctly populated,
> its generation is still left 0:
>
> log_root 30474240
> log_root_transid 0 <<<
> log_root_level 0
>
> [CAUSE]
> We just forgot to update btrfs_super_block::log_root_transid completely.
>
> Thus it's always the original value (0) from the initial super block.
>
> Thankfully this old behavior won't break log replay, as in
> btrfs_read_tree(), parent generation 0 means we just skip the generation
btrfs_read_tree() does not exists, it's btrfs_read_tree_root().
This is actually irrelevant, because we don't read the root log tree with
btrfs_read_tree_root(). We use read_tree_block() for that (at btrfs_replay_log()),
and we use a generation matching the committed transaction + 1 (as it can never
be anything else).
For every other log tree, we use btrfs_read_tree_root(), but the generation is
stored in the root's root item stored in the root log tree.
The log_root_transid field was added to the super block by:
commit c3027eb5523d6983f12628f3fe13d8a7576db701
Author: Chris Mason <chris.mason@xxxxxxxxxx>
Date: Mon Dec 8 16:40:21 2008 -0500
Btrfs: Add inode sequence number for NFS and reserved space in a few structs
But it was never used.
So this change is not needed.
Thanks.
> check.
>
> And per-root log tree is still done properly using the root generation,
> so here we really only missed the generation check for log tree root,
> and even we fixed it, it should not cause any compatible problem.
>
> [FIX]
> Just update btrfs_super_block::log_root_transid properly.
We don't need this.
The log_root_transid field was added to the super block by:
commit c3027eb5523d6983f12628f3fe13d8a7576db701
Author: Chris Mason <chris.mason@xxxxxxxxxx>
Date: Mon Dec 8 16:40:21 2008 -0500
Btrfs: Add inode sequence number for NFS and reserved space in a few structs
But it was never used.
For btrfs_read_tree_root(), what we use is the
>
> Cc: stable@xxxxxxxxxxxxxxx #4.9+
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> fs/btrfs/tree-log.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 370388fadf96..27a76d6fef8c 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3083,7 +3083,8 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
> struct btrfs_log_ctx root_log_ctx;
> struct blk_plug plug;
> u64 log_root_start;
> - u64 log_root_level;
> + u64 log_root_transid;
> + u8 log_root_level;
>
> mutex_lock(&root->log_mutex);
> log_transid = ctx->log_transid;
> @@ -3297,6 +3298,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
>
> log_root_start = log_root_tree->node->start;
> log_root_level = btrfs_header_level(log_root_tree->node);
> + log_root_transid = btrfs_header_generation(log_root_tree->node);
> log_root_tree->log_transid++;
> mutex_unlock(&log_root_tree->log_mutex);
>
> @@ -3334,6 +3336,7 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
>
> btrfs_set_super_log_root(fs_info->super_for_commit, log_root_start);
> btrfs_set_super_log_root_level(fs_info->super_for_commit, log_root_level);
> + btrfs_set_super_log_root_transid(fs_info->super_for_commit, log_root_transid);
> ret = write_all_supers(fs_info, 1);
> mutex_unlock(&fs_info->tree_log_mutex);
> if (ret) {
> --
> 2.36.1
>
