On 2018/11/20 下午4:51, Nikolay Borisov wrote:
>
>
> On 20.11.18 г. 10:46 ч., Qu Wenruo wrote:
>> Currently we only check @ref_root in btrfs_add_delayed_data_ref() to
>> determine whether a data delayed ref is for reloc tree.
>>
>> Such check is insufficient as for relocation we could pass @ref_root
>> as the source file tree, causing qgroup to trace unchanged data extents
>> even we're only relocating metadata chunks.
>>
>> We could insert qgroup extent record for the following call trace even
>> we're only relocating metadata block group:
>>
>> do_relocation()
>> |- btrfs_cow_block(root=reloc_root)
>> |- update_ref_for_cow(root=reloc_root)
>> |- __btrfs_mod_ref(root=reloc_root)
>> |- ref_root = btrfs_header_owner()
>> |- btrfs_add_delayed_data_ref(ref_root=source_root)
>>
>> And another case when dropping reloc tree:
>>
>> clean_dirty_root()
>> |- btrfs_drop_snapshot(root=relocc_root)
>> |- walk_up_tree(root=reloc_root)
>> |- walk_up_proc(root=reloc_root)
>> |- btrfs_dec_ref(root=reloc_root)
>> |- __btrfs_mod_ref(root=reloc_root)
>> |- ref_root = btrfs_header_owner()
>> |- btrfs_add_delayed_data_ref(ref_root=source_root)
>>
>> This patch will introduce @root parameter for
>> btrfs_add_delayed_data_ref(), so that we could determine if this data
>> extent belongs to reloc tree or not.
>>
>> This could skip data extents which aren't really modified during
>> relocation.
>>
>> For the same real world 4G data 16 snapshots 4K nodesize metadata
>> balance test:
>> | v4.20-rc1 + delaye* | w/ patch | diff
>> -----------------------------------------------------------------------
>> relocated extents | 22773 | 22656 | -0.1%
>> qgroup dirty extents | 122879 | 74316 | -39.5%
>> time (real) | 23.073s | 14.971 | -35.1%
>>
>> *: v4.20-rc1 + delayed subtree scan patchset
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>> ---
>> fs/btrfs/delayed-ref.c | 3 ++-
>> fs/btrfs/delayed-ref.h | 1 +
>> fs/btrfs/extent-tree.c | 6 +++---
>> 3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
>> index 9301b3ad9217..269bd6ecb8f3 100644
>> --- a/fs/btrfs/delayed-ref.c
>> +++ b/fs/btrfs/delayed-ref.c
>> @@ -798,6 +798,7 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
>> * add a delayed data ref. it's similar to btrfs_add_delayed_tree_ref.
>> */
>> int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
>> + struct btrfs_root *root,
>
> I'm beginning to wonder, should we document
> btrfs_add_delayed_data_ref/btrfs_add_tree_ref arguments separate for
> each function, or should only the differences be documented - in this
> case the newly added root parameter. The rest of the arguments are being
> documented at init_delayed_ref_common.
You won't be happy with my later plan, it will add new parameter for
btrfs_add_delayed_tree_ref(), and it may not be @root, but some bool.
So I think we may need to document at least the difference.
Thanks,
Qu
>
>> u64 bytenr, u64 num_bytes,
>> u64 parent, u64 ref_root,
>> u64 owner, u64 offset, u64 reserved, int action,
>> @@ -835,7 +836,7 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
>> }
>>
>> if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags) &&
>> - is_fstree(ref_root)) {
>> + is_fstree(ref_root) && is_fstree(root->root_key.objectid)) {
>> record = kmalloc(sizeof(*record), GFP_NOFS);
>> if (!record) {
>> kmem_cache_free(btrfs_delayed_data_ref_cachep, ref);
>> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
>> index 8e20c5cb5404..6c60737e55d6 100644
>> --- a/fs/btrfs/delayed-ref.h
>> +++ b/fs/btrfs/delayed-ref.h
>> @@ -240,6 +240,7 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
>> struct btrfs_delayed_extent_op *extent_op,
>> int *old_ref_mod, int *new_ref_mod);
>> int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
>> + struct btrfs_root *root,
>> u64 bytenr, u64 num_bytes,
>> u64 parent, u64 ref_root,
>> u64 owner, u64 offset, u64 reserved, int action,
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index a1febf155747..0554d2cc2ea1 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -2046,7 +2046,7 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>> BTRFS_ADD_DELAYED_REF, NULL,
>> &old_ref_mod, &new_ref_mod);
>> } else {
>> - ret = btrfs_add_delayed_data_ref(trans, bytenr,
>> + ret = btrfs_add_delayed_data_ref(trans, root, bytenr,
>> num_bytes, parent,
>> root_objectid, owner, offset,
>> 0, BTRFS_ADD_DELAYED_REF,
>> @@ -7104,7 +7104,7 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>> BTRFS_DROP_DELAYED_REF, NULL,
>> &old_ref_mod, &new_ref_mod);
>> } else {
>> - ret = btrfs_add_delayed_data_ref(trans, bytenr,
>> + ret = btrfs_add_delayed_data_ref(trans, root, bytenr,
>> num_bytes, parent,
>> root_objectid, owner, offset,
>> 0, BTRFS_DROP_DELAYED_REF,
>> @@ -8080,7 +8080,7 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
>> root->root_key.objectid, owner, offset,
>> BTRFS_ADD_DELAYED_EXTENT);
>>
>> - ret = btrfs_add_delayed_data_ref(trans, ins->objectid,
>> + ret = btrfs_add_delayed_data_ref(trans, root, ins->objectid,
>> ins->offset, 0,
>> root->root_key.objectid, owner,
>> offset, ram_bytes,
>>
