Re: [PATCH] btrfs: correctly populate btrfs_super_block::log_root_transid

[Filipe Manana <fdmanana@kernel.org>]

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@oracle.com>
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@oracle.com>
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@vger.kernel.org #4.9+
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  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
>