[PATCH] btrfs-progs: make btrfs_super_block::log_root_transid deprecated

[PATCH] btrfs-progs: make btrfs_super_block::log_root_transid deprecated

[Qu Wenruo]

This is the same on-disk format update synchronized from the kernel
code.

Unlike kernel, there are two callers reading this member:

- btrfs inspect dump-super
  It's just outputting the value, since it's always 0 we can skip
  that output.

- btrfs-find-root
  In that case, since we always got 0, the root search for log root
  should never find a perfect match.

  Use btrfs_super_geneartion() + 1 to provide a better result.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-find-root.c          |  2 +-
 kernel-shared/ctree.h      | 11 +++++++----
 kernel-shared/print-tree.c |  2 --
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index e328334034ea..5ae808cc5def 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -249,7 +249,7 @@ static void get_root_gen_and_level(u64 objectid, struct btrfs_fs_info *fs_info,
 		break;
 	case BTRFS_TREE_LOG_OBJECTID:
 		level = btrfs_super_log_root_level(super);
-		gen = btrfs_super_log_root_transid(super);
+		gen = btrfs_super_generation(super) + 1;
 		break;
 	case BTRFS_UUID_TREE_OBJECTID:
 		gen = btrfs_super_uuid_tree_generation(super);
diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index fc8b61eda829..5d1d07b1d308 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -426,8 +426,13 @@ struct btrfs_super_block {
 	__le64 chunk_root;
 	__le64 log_root;
 
-	/* this will help find the new super based on the log root */
-	__le64 log_root_transid;
+	/*
+	 * This member is never utlized from the very beginning of btrfs, thus
+	 * it's always 0 no matter kernel versions.
+	 * We always use generation + 1 to read log tree root.
+	 * So here we mark it deprecated.
+	 */
+	__le64 __unused_log_root_transid;
 	__le64 total_bytes;
 	__le64 bytes_used;
 	__le64 root_dir_objectid;
@@ -2334,8 +2339,6 @@ BTRFS_SETGET_STACK_FUNCS(super_chunk_root_level, struct btrfs_super_block,
 			 chunk_root_level, 8);
 BTRFS_SETGET_STACK_FUNCS(super_log_root, struct btrfs_super_block,
 			 log_root, 64);
-BTRFS_SETGET_STACK_FUNCS(super_log_root_transid, struct btrfs_super_block,
-			 log_root_transid, 64);
 BTRFS_SETGET_STACK_FUNCS(super_log_root_level, struct btrfs_super_block,
 			 log_root_level, 8);
 BTRFS_SETGET_STACK_FUNCS(super_total_bytes, struct btrfs_super_block,
diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
index a5886ff602ee..7285d471c81b 100644
--- a/kernel-shared/print-tree.c
+++ b/kernel-shared/print-tree.c
@@ -2014,8 +2014,6 @@ void btrfs_print_superblock(struct btrfs_super_block *sb, int full)
 	       (unsigned long long)btrfs_super_chunk_root_level(sb));
 	printf("log_root\t\t%llu\n",
 	       (unsigned long long)btrfs_super_log_root(sb));
-	printf("log_root_transid\t%llu\n",
-	       (unsigned long long)btrfs_super_log_root_transid(sb));
 	printf("log_root_level\t\t%llu\n",
 	       (unsigned long long)btrfs_super_log_root_level(sb));
 	printf("total_bytes\t\t%llu\n",
-- 
2.36.1

Re: [PATCH] btrfs-progs: make btrfs_super_block::log_root_transid deprecated

[David Sterba]

On Tue, Jun 07, 2022 at 08:01:17PM +0800, Qu Wenruo wrote:
> This is the same on-disk format update synchronized from the kernel
> code.
> 
> Unlike kernel, there are two callers reading this member:
> 
> - btrfs inspect dump-super
>   It's just outputting the value, since it's always 0 we can skip
>   that output.
> 
> - btrfs-find-root
>   In that case, since we always got 0, the root search for log root
>   should never find a perfect match.
> 
>   Use btrfs_super_geneartion() + 1 to provide a better result.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to devel, thanks.

> --- a/kernel-shared/print-tree.c
> +++ b/kernel-shared/print-tree.c
> @@ -2014,8 +2014,6 @@ void btrfs_print_superblock(struct btrfs_super_block *sb, int full)
>  	       (unsigned long long)btrfs_super_chunk_root_level(sb));
>  	printf("log_root\t\t%llu\n",
>  	       (unsigned long long)btrfs_super_log_root(sb));
> -	printf("log_root_transid\t%llu\n",
> -	       (unsigned long long)btrfs_super_log_root_transid(sb));

For dump super I'd like to keep it there, same as we print the value of
leafsize even if it's deprecated.

>  	printf("log_root_level\t\t%llu\n",
>  	       (unsigned long long)btrfs_super_log_root_level(sb));
>  	printf("total_bytes\t\t%llu\n",
> -- 
> 2.36.1

Re: [PATCH] btrfs-progs: make btrfs_super_block::log_root_transid deprecated

[Qu Wenruo]

On 2022/7/18 23:34, David Sterba wrote:
> On Tue, Jun 07, 2022 at 08:01:17PM +0800, Qu Wenruo wrote:
>> This is the same on-disk format update synchronized from the kernel
>> code.
>>
>> Unlike kernel, there are two callers reading this member:
>>
>> - btrfs inspect dump-super
>>    It's just outputting the value, since it's always 0 we can skip
>>    that output.
>>
>> - btrfs-find-root
>>    In that case, since we always got 0, the root search for log root
>>    should never find a perfect match.
>>
>>    Use btrfs_super_geneartion() + 1 to provide a better result.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Added to devel, thanks.
> 
>> --- a/kernel-shared/print-tree.c
>> +++ b/kernel-shared/print-tree.c
>> @@ -2014,8 +2014,6 @@ void btrfs_print_superblock(struct btrfs_super_block *sb, int full)
>>   	       (unsigned long long)btrfs_super_chunk_root_level(sb));
>>   	printf("log_root\t\t%llu\n",
>>   	       (unsigned long long)btrfs_super_log_root(sb));
>> -	printf("log_root_transid\t%llu\n",
>> -	       (unsigned long long)btrfs_super_log_root_transid(sb));
> 
> For dump super I'd like to keep it there, same as we print the value of
> leafsize even if it's deprecated.

In that case, we only need add "(deprecated)" to the string and adjust 
the offset.

Do I need to update the patch or send a incremental update?

Thanks,
Qu
> 
>>   	printf("log_root_level\t\t%llu\n",
>>   	       (unsigned long long)btrfs_super_log_root_level(sb));
>>   	printf("total_bytes\t\t%llu\n",
>> -- 
>> 2.36.1

Re: [PATCH] btrfs-progs: make btrfs_super_block::log_root_transid deprecated

[David Sterba]

On Tue, Jul 19, 2022 at 09:31:28AM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/7/18 23:34, David Sterba wrote:
> > On Tue, Jun 07, 2022 at 08:01:17PM +0800, Qu Wenruo wrote:
> >> This is the same on-disk format update synchronized from the kernel
> >> code.
> >>
> >> Unlike kernel, there are two callers reading this member:
> >>
> >> - btrfs inspect dump-super
> >>    It's just outputting the value, since it's always 0 we can skip
> >>    that output.
> >>
> >> - btrfs-find-root
> >>    In that case, since we always got 0, the root search for log root
> >>    should never find a perfect match.
> >>
> >>    Use btrfs_super_geneartion() + 1 to provide a better result.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> > 
> > Added to devel, thanks.
> > 
> >> --- a/kernel-shared/print-tree.c
> >> +++ b/kernel-shared/print-tree.c
> >> @@ -2014,8 +2014,6 @@ void btrfs_print_superblock(struct btrfs_super_block *sb, int full)
> >>   	       (unsigned long long)btrfs_super_chunk_root_level(sb));
> >>   	printf("log_root\t\t%llu\n",
> >>   	       (unsigned long long)btrfs_super_log_root(sb));
> >> -	printf("log_root_transid\t%llu\n",
> >> -	       (unsigned long long)btrfs_super_log_root_transid(sb));
> > 
> > For dump super I'd like to keep it there, same as we print the value of
> > leafsize even if it's deprecated.
> 
> In that case, we only need add "(deprecated)" to the string and adjust 
> the offset.
> 
> Do I need to update the patch or send a incremental update?

No need to, it was a simple fixup.