Re: [PATCH] btrfs: relocation: Use btrfs_find_all_leaves() to locate parent tree leaves of a data extent

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

[-- Attachment #1.1: Type: text/plain, Size: 5646 bytes --]



On 2020/3/13 上午4:48, David Sterba wrote:
> On Tue, Mar 10, 2020 at 04:14:15PM +0800, Qu Wenruo wrote:
>> In relocation, we need to locate all parent tree leaves referring one
>> data extent, thus we have a complex mechanism to iterate throught extent
>> tree and subvolume trees to locate related leaves.
>>
>> However this is already done in backref.c, we have
>> btrfs_find_all_leaves(), which can return a ulist containing all leaves
>> referring to that data extent.
>>
>> Use btrfs_find_all_leaves() to replace find_data_references().
>       ^^^^^^^^^^J^^^^^^^^^^
> 
> The function is called btrfs_find_all_leafs, which is wrong spelling of
> leaves, I was tempted to rename it in the same patch but unfortunately
> ther's one more untouched caller so it's for another one.

Mind me to send the spelling fix?

> 
>> There is a special handling for v1 space cache data extents, where we
>> need to delete the v1 space cache data extents, to avoid those data
>> extents to hang the data relocation.
>>
>> In this patch, the special handling is done by re-iterating the root
>> tree leaf.
>> Although it's a little less efficient than the old handling, considering
>> we can reuse a lot of code, it should be acceptable.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> This patch is originally in my backref cache branch, but since it's
>> pretty independent from other backref cache code, and straightforward to
>> test/review, it's sent for more comprehensive test/review/merge.
>> ---
>>  fs/btrfs/backref.c    |   8 +-
>>  fs/btrfs/backref.h    |   4 +
>>  fs/btrfs/relocation.c | 314 ++++++++----------------------------------
>>  3 files changed, 62 insertions(+), 264 deletions(-)
>>
>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>> index 327e4480957b..f2728fb3ee8f 100644
>> --- a/fs/btrfs/backref.c
>> +++ b/fs/btrfs/backref.c
>> @@ -1409,10 +1409,10 @@ static void free_leaf_list(struct ulist *blocks)
>>   *
>>   * returns 0 on success, <0 on error
>>   */
>> -static int btrfs_find_all_leafs(struct btrfs_trans_handle *trans,
>> -				struct btrfs_fs_info *fs_info, u64 bytenr,
>> -				u64 time_seq, struct ulist **leafs,
>> -				const u64 *extent_item_pos, bool ignore_offset)
>> +int btrfs_find_all_leafs(struct btrfs_trans_handle *trans,
>> +			 struct btrfs_fs_info *fs_info, u64 bytenr,
>> +			 u64 time_seq, struct ulist **leafs,
>> +			 const u64 *extent_item_pos, bool ignore_offset)
>>  {
>>  	int ret;
>>  
>> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
>> index 777f61dc081e..723d6da99114 100644
>> --- a/fs/btrfs/backref.h
>> +++ b/fs/btrfs/backref.h
>> @@ -40,6 +40,10 @@ int iterate_inodes_from_logical(u64 logical, struct btrfs_fs_info *fs_info,
>>  
>>  int paths_from_inode(u64 inum, struct inode_fs_paths *ipath);
>>  
>> +int btrfs_find_all_leafs(struct btrfs_trans_handle *trans,
>> +			 struct btrfs_fs_info *fs_info, u64 bytenr,
>> +			 u64 time_seq, struct ulist **leafs,
>> +			 const u64 *extent_item_pos, bool ignore_offset);
>>  int btrfs_find_all_roots(struct btrfs_trans_handle *trans,
>>  			 struct btrfs_fs_info *fs_info, u64 bytenr,
>>  			 u64 time_seq, struct ulist **roots, bool ignore_offset);
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 02afe294ee2d..319d50c7ada5 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -23,6 +23,7 @@
>>  #include "print-tree.h"
>>  #include "delalloc-space.h"
>>  #include "block-group.h"
>> +#include "backref.h"
>>  
>>  /*
>>   * Relocation overview
>> @@ -3620,31 +3621,6 @@ static int __add_tree_block(struct reloc_control *rc,
>>  	return ret;
>>  }
>>  
>> -/*
>> - * helper to check if the block use full backrefs for pointers in it
>> - */
>> -static int block_use_full_backref(struct reloc_control *rc,
>> -				  struct extent_buffer *eb)
>> -{
>> -	u64 flags;
>> -	int ret;
>> -
>> -	if (btrfs_header_flag(eb, BTRFS_HEADER_FLAG_RELOC) ||
>> -	    btrfs_header_backref_rev(eb) < BTRFS_MIXED_BACKREF_REV)
>> -		return 1;
>> -
>> -	ret = btrfs_lookup_extent_info(NULL, rc->extent_root->fs_info,
>> -				       eb->start, btrfs_header_level(eb), 1,
>> -				       NULL, &flags);
>> -	BUG_ON(ret);
>> -
>> -	if (flags & BTRFS_BLOCK_FLAG_FULL_BACKREF)
>> -		ret = 1;
>> -	else
>> -		ret = 0;
>> -	return ret;
>> -}
>> -
>>  static int delete_block_group_cache(struct btrfs_fs_info *fs_info,
>>  				    struct btrfs_block_group *block_group,
>>  				    struct inode *inode,
>> @@ -3688,174 +3664,42 @@ static int delete_block_group_cache(struct btrfs_fs_info *fs_info,
>>  }
>>  
>>  /*
>> - * helper to add tree blocks for backref of type BTRFS_EXTENT_DATA_REF_KEY
>> - * this function scans fs tree to find blocks reference the data extent
>> + * Helper function to locate the free space cache EXTENT_DATA in root tree leaf
>> + * and delete the cache inode, to avoid free space cache data extent blocking
>> + * data relocation.
>>   */
>> -static int find_data_references(struct reloc_control *rc,
>> -				struct btrfs_key *extent_key,
>> -				struct extent_buffer *leaf,
>> -				struct btrfs_extent_data_ref *ref,
>> -				struct rb_root *blocks)
>> +static int delete_v1_space_cache(struct btrfs_fs_info *fs_info,
>> +				 struct extent_buffer *leaf,
> 
> The fs_info seems to be redundant, so I've replaced it with
> leaf->fs_info here

Oh, right. Always forgot that eb::fs_info member.

Thanks,
Qu
> 
>> +	ret = delete_block_group_cache(fs_info, block_group, NULL,
>> +					space_cache_ino);
>> +	return ret;
>>  }


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]