[PATCH v2] btrfs: Deprecate unused super block member, log_root_transid

[PATCH v2] btrfs: Deprecate unused super block member, log_root_transid

[Qu Wenruo]

The member log_root_transid is never used.
It's always kept untouched even when updating log tree root.

And populating it without introducing new incompat flags could easily
cause back-compatibility problems.
So just mark it unused.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
  Remove the redundant comment.
---
 fs/btrfs/ctree.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 53af9f5253f4..7adf5f4dcda4 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -214,8 +214,7 @@ 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;
+	__le64 __unused_log_root_transid;
 	__le64 total_bytes;
 	__le64 bytes_used;
 	__le64 root_dir_objectid;
@@ -2317,8 +2316,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.19.1

Re: [PATCH v2] btrfs: Deprecate unused super block member, log_root_transid

[Nikolay Borisov]

On 12.10.2018 09:37, Qu Wenruo wrote:
> The member log_root_transid is never used.
> It's always kept untouched even when updating log tree root.
> 
> And populating it without introducing new incompat flags could easily
> cause back-compatibility problems.
> So just mark it unused.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v2:
>   Remove the redundant comment.
> ---
>  fs/btrfs/ctree.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 53af9f5253f4..7adf5f4dcda4 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -214,8 +214,7 @@ 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;
> +	__le64 __unused_log_root_transid;

why do you keep insisting on having "log_root_transid" in the name of
this member? It was never used on-disk so for all intents and purposes
log_root_transid doesn't mean anything? No need to resend, I'm sure
David will change it on commit, nut I'm just curious.


>  	__le64 total_bytes;
>  	__le64 bytes_used;
>  	__le64 root_dir_objectid;
> @@ -2317,8 +2316,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,
> 

Re: [PATCH v2] btrfs: Deprecate unused super block member, log_root_transid

[Qu Wenruo]

On 2018/10/12 下午2:52, Nikolay Borisov wrote:
> 
> 
> On 12.10.2018 09:37, Qu Wenruo wrote:
>> The member log_root_transid is never used.
>> It's always kept untouched even when updating log tree root.
>>
>> And populating it without introducing new incompat flags could easily
>> cause back-compatibility problems.
>> So just mark it unused.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> changelog:
>> v2:
>>   Remove the redundant comment.
>> ---
>>  fs/btrfs/ctree.h | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 53af9f5253f4..7adf5f4dcda4 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -214,8 +214,7 @@ 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;
>> +	__le64 __unused_log_root_transid;
> 
> why do you keep insisting on having "log_root_transid" in the name of
> this member? It was never used on-disk so for all intents and purposes
> log_root_transid doesn't mean anything? No need to resend, I'm sure
> David will change it on commit, nut I'm just curious.

We have a similar case, leaf_size.
Although it differs from this case and it's still used, I prefer to have
something showing that we at least planned to use this member.

Just in case one day we tries to use it again.

Thanks,
Qu

> 
> 
>>  	__le64 total_bytes;
>>  	__le64 bytes_used;
>>  	__le64 root_dir_objectid;
>> @@ -2317,8 +2316,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,
>>