On 2022/6/7 17:40, Filipe Manana wrote:
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()),
Oh, right, I forgot to check that code, and just assumed every root read
from superblock would has its generation checked, but it's not the case
for log tree root.
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.
Then, considering we have never really set log_root_transid anywhere,
and in theory we're always using btrfs_super_block::generation + 1, why
not just deprecate that member?
In fact, every time (thankfully not that many times for me) I checked
log_root_transid, I can not help but to wonder why it's always 0.
Thanks,
Qu
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
