Re: [PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2019/8/10 下午5:24, Qu Wenruo wrote:
>
> [...]
>>>
>>> Because we have extra transid check in
>>> btrfs_search_slot()/btrfs_cow_block().
>>>
>>> EXTENT_BAD_TRANSID is to suppress such warning.
>>
>> nod
>>
>>>
>>>>
>>>> So repair_root_generation could possibly be as simple as just linking
>>>> root->node to the fs_info->recow_ebs and leave the rest to the existing
>>>> logic?
>>>
>>> It doesn't change the root generation.
>>
>> recow_extent_buffer seems to be doing exactly the same thing
>> repair_root_generation does - findw the root item and commits the
>> transaction. What am I missing?
>
> The way it gets the root is not correct.
>
> Using header owner is not good enough for locating the tree owning the
> tree blocks.
>
> Cases like log trees and shared tree blocks (especially for cases like
> original tree got deleted) are the main corner cases where the old
> behavior can't handle.

My bad, in fact log tree won't exist in repair mode, and since it's the
tree root node, it won't be shared with other trees, so the existing
facility is completely fine with the use case.

Thanks for pointing this out!
Qu

>
> Thanks,
> Qu
>
>>
>>>
>>>>
>>>>> +
>>>>> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +
>>>>> +	btrfs_release_path(&path);
>>>>> +	ret = btrfs_commit_transaction(trans, root);
>>>>> +	return ret;
>>>>> +}
>>>>> diff --git a/check/mode-common.h b/check/mode-common.h
>>>>> index 4c169c6e3b29..4a3abeb02c81 100644
>>>>> --- a/check/mode-common.h
>>>>> +++ b/check/mode-common.h
>>>>> @@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
>>>>>  	return true;
>>>>>  }
>>>>>
>>>>> +int repair_root_generation(struct btrfs_root *root);
>>>>>  #endif
>>>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>>>> index a2be0e6d7034..bf3b57f5ad2d 100644
>>>>> --- a/check/mode-lowmem.c
>>>>> +++ b/check/mode-lowmem.c
>>>>> @@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>>>  	struct btrfs_path path;
>>>>>  	struct node_refs nrefs;
>>>>>  	struct btrfs_root_item *root_item = &root->root_item;
>>>>> +	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
>>>>>  	int ret;
>>>>>  	int level;
>>>>>  	int err = 0;
>>>>> @@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>>>  	level = btrfs_header_level(root->node);
>>>>>  	btrfs_init_path(&path);
>>>>>
>>>>> +	if (btrfs_root_generation(root_item) > super_generation + 1) {
>>>>> +		error(
>>>>> +	"invalid root generation for root %llu, have %llu expect (0, %llu)",
>>>>> +		      root->root_key.objectid, btrfs_root_generation(root_item),
>>>>> +		      super_generation + 1);
>>>>> +		err |= INVALID_GENERATION;
>>>>> +		if (repair) {
>>>>> +			ret = repair_root_generation(root);
>>>>> +			if (!ret) {
>>>>> +				err &= ~INVALID_GENERATION;
>>>>> +				printf("Reset generation for root %llu\n",
>>>>> +					root->root_key.objectid);
>>>>> +			}
>>>>> +		}
>>>>> +	}
>>>>>  	if (btrfs_root_refs(root_item) > 0 ||
>>>>>  	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
>>>>>  		path.nodes[level] = root->node;
>>>>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>>>>> index d2983fd12eb4..0361fb3382b1 100644
>>>>> --- a/check/mode-lowmem.h
>>>>> +++ b/check/mode-lowmem.h
>>>>> @@ -47,6 +47,7 @@
>>>>>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>>>>>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
>>>>>  #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
>>>>> +#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
>>>>>
>>>>>  /*
>>>>>   * Error bit for low memory mode check.
>>>>>
>>>