[PATCH] btrfs: make btrfs_super_block::log_root_transid deprecated

[PATCH] btrfs: make btrfs_super_block::log_root_transid deprecated

[Qu Wenruo]

When using "btrfs inspect-internal dump-super" to inspect an fs with
dirty log, it always shows the log_root_transid as 0:

 log_root                30474240
 log_root_transid        0 <<<
 log_root_level          0

It turns out that, btrfs_super_block::log_root_transid is never really
utilized (even no read for it).

This can date back to the introduction of btrfs into upstream kernel.

In fact, when reading log tree root, we always use
btrfs_super_block::generation + 1 as the expected generation.
So here we're completely safe to mark this member deprecated.

In theory we can easily reuse this member for other purposes, but to be
extra safe, here we follow the leafsize way, by adding "__unused_" for
log_root_transid.
And we can safely remove the accessors, since there is no such callers
from the very beginning.

Cc: Chris Mason <clm@fb.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ctree.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0e49b1a0c071..71ba3b9fd8c2 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -248,8 +248,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;
@@ -2475,8 +2480,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,
-- 
2.36.1

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

[Filipe Manana]

On Tue, Jun 07, 2022 at 07:50:59PM +0800, Qu Wenruo wrote:
> When using "btrfs inspect-internal dump-super" to inspect an fs with
> dirty log, it always shows the log_root_transid as 0:
> 
>  log_root                30474240
>  log_root_transid        0 <<<
>  log_root_level          0
> 
> It turns out that, btrfs_super_block::log_root_transid is never really
> utilized (even no read for it).
> 
> This can date back to the introduction of btrfs into upstream kernel.
> 
> In fact, when reading log tree root, we always use
> btrfs_super_block::generation + 1 as the expected generation.
> So here we're completely safe to mark this member deprecated.
> 
> In theory we can easily reuse this member for other purposes, but to be
> extra safe, here we follow the leafsize way, by adding "__unused_" for
> log_root_transid.
> And we can safely remove the accessors, since there is no such callers
> from the very beginning.
> 
> Cc: Chris Mason <clm@fb.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/ctree.h | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 0e49b1a0c071..71ba3b9fd8c2 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -248,8 +248,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

utlized -> utilized (or used)

> +	 * it's always 0 no matter kernel versions.

no matter kernel versions -> no matter which kernel version

Other than that, it looks fine.
Thanks.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

> +	 * 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;
> @@ -2475,8 +2480,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,
> -- 
> 2.36.1
> 

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

[David Sterba]

On Tue, Jun 07, 2022 at 07:50:59PM +0800, Qu Wenruo wrote:
> When using "btrfs inspect-internal dump-super" to inspect an fs with
> dirty log, it always shows the log_root_transid as 0:
> 
>  log_root                30474240
>  log_root_transid        0 <<<
>  log_root_level          0
> 
> It turns out that, btrfs_super_block::log_root_transid is never really
> utilized (even no read for it).
> 
> This can date back to the introduction of btrfs into upstream kernel.
> 
> In fact, when reading log tree root, we always use
> btrfs_super_block::generation + 1 as the expected generation.
> So here we're completely safe to mark this member deprecated.
> 
> In theory we can easily reuse this member for other purposes, but to be
> extra safe, here we follow the leafsize way, by adding "__unused_" for
> log_root_transid.
> And we can safely remove the accessors, since there is no such callers
> from the very beginning.
> 
> Cc: Chris Mason <clm@fb.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to misc-next, thanks.