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.
> 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,
>
