From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list
Date: Mon, 11 Feb 2019 13:16:44 +0800 [thread overview]
Message-ID: <20190211051653.3167-1-wqu@suse.com> (raw)
This patchset can be fetched from github:
https://github.com/adam900710/linux/tree/refactor_delayed_ref_parameter
Which is based on David's misc-next branch, the base commit is:
commit eb3e579e28f1c58e79176fbf5afe1cf3ee227190 (david/misc-next, david/misc-5.1)
Author: Anand Jain <anand.jain@oracle.com>
Date: Fri Feb 8 15:39:37 2019 +0800
Current delayed ref interface has several problems:
- Longer and longer parameter lists
bytenr
num_bytes
parent
---- So far so good
ref_root
owner
offset
---- I don't feel well now
for_reloc
^^^^ This parameter only makes sense for qgroup code, but we need
to pass the parameter a long way down.
This makes later parameter list add more and more tricky.
- Different interpretation for the same parameter
Above @owner for data ref is inode who owns this extent,
while for tree ref, it's level. They are even in different size range.
For level we only need 0~8, while for ino it's
BTRFS_FIRST_FREE_OBJECTID~BTRFS_LAST_FREE_OBJECTID, so it's still
possible to distinguish them, but it's never a straight-forward thing
to grasp.
And @offset doesn't even makes sense for tree ref.
Such parameter reuse may look clever as an hidden union, but it
destroys code readability.
This patchset will change the way how we pass parameters for delayed
ref.
Instead of calling delayed ref interface like:
ret = btrfs_inc_extent_ref(trans, root, bytenr, num_bytes, parent,
ref_root, owner, offset);
Or
ret = btrfs_inc_extent_ref(trans, root, bytenr, nodesize, parent,
level, ref_root, 0);
We now call like:
btrfs_init_generic_ref(&ref, bytenr, num_bytes,
root->root_key.objectid, parent);
btrfs_init_data_ref(&ref, ref_root, owner, offset);
ret = btrfs_inc_extent_ref(trans, &ref);
Or
btrfs_init_generic_ref(&ref, bytenr, num_bytes,
root->root_key.objectid, parent);
btrfs_init_tree_ref(&ref, level, ref_root);
ret = btrfs_inc_extent_ref(trans, &ref);
To determine if a ref is tree or data, instead of calling like:
if (owner < BTRFS_FIRST_FREE_OBJECTID) {
} else {
}
We do it straight-forward:
if (ref->type == BTRFS_REF_METADATA) {
} else {
}
And for new members determining some minor behavior, we don't need to add
a new parameter to btrfs_add_delayed_tree|data_ref() or
btrfs_inc_extent_ref(), we just assign them after generic/data/tree init, like:
btrfs_init_generic_ref(&ref, bytenr, num_bytes,
root->root_key.objectid, parent);
ref->real_root = root->root_key.objectid;
ref->skip_qgroup = true;
btrfs_init_data_ref(&ref, ref_root, owner, offset);
ret = btrfs_inc_extent_ref(trans, &ref);
This should improve the code readability and make later code easier to
write.
Furthermore, with the help of btrfs_ref::real_root parameter, qgroup
can skip quit a lot of delayed tree/data ref for reloc tree, which
makes qgroup + balance as fast as quota disabled:
Test VM:
- vRAM 8G
- vCPU 8
- block dev vitrio-blk, 'unsafe' cache mode
- host block 850evo
Test workload
- Copy 4G data from /usr/ to one subvolume
- Create 16 snapshots of that subvolume, and modify 3 files in each
snapshot
- Enable quota, rescan
- Time "btrfs balance start -m"
| base | w/ patchset | no qgrouos |
-------------------------------------------------------------
relocated | 23765 | 23772 | 23811 |
qgroup dirty | 124498 | 70 | 0 |
time (sec) | 23.353 | 3.505 | 3.421 |
Changelog:
v2:
- Better documentation for btrfs_ref declaration
- Rebase to newer delayed subtree rescan patchset
- Add reviewed-by tags
- Remove unnecessary ASSERT() for NULL pointer.
v3:
- Rebase to misc-next branch as that branch has all prerequisite now.
- Update benchmark result, compare with qgroups disabled case directly.
Qu Wenruo (9):
btrfs: delayed-ref: Introduce better documented delayed ref structures
btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref
btrfs: delayed-ref: Use btrfs_ref to refactor
btrfs_add_delayed_tree_ref()
btrfs: delayed-ref: Use btrfs_ref to refactor
btrfs_add_delayed_data_ref()
btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()
btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
btrfs: qgroup: Don't scan leaf if we're modifying reloc tree
fs/btrfs/ctree.h | 10 +--
fs/btrfs/delayed-ref.c | 44 ++++++---
fs/btrfs/delayed-ref.h | 126 ++++++++++++++++++++++++--
fs/btrfs/extent-tree.c | 199 ++++++++++++++++++++---------------------
fs/btrfs/file.c | 39 ++++----
fs/btrfs/inode.c | 23 +++--
fs/btrfs/ioctl.c | 15 ++--
fs/btrfs/ref-verify.c | 53 ++++++-----
fs/btrfs/ref-verify.h | 10 +--
fs/btrfs/relocation.c | 67 +++++++++-----
fs/btrfs/tree-log.c | 11 ++-
11 files changed, 382 insertions(+), 215 deletions(-)
--
2.20.1
next reply other threads:[~2019-02-11 5:17 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-11 5:16 Qu Wenruo [this message]
2019-02-11 5:16 ` [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures Qu Wenruo
2019-02-11 12:55 ` Nikolay Borisov
2019-02-11 13:23 ` Qu Wenruo
2019-02-11 14:20 ` Nikolay Borisov
2019-02-11 14:23 ` Qu Wenruo
2019-02-18 5:00 ` Qu Wenruo
2019-02-18 6:59 ` Su Yue
2019-02-11 5:16 ` [PATCH v3 2/9] btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 3/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref() Qu Wenruo
2019-02-11 12:58 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 4/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref() Qu Wenruo
2019-02-11 12:59 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 5/9] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod() Qu Wenruo
2019-02-11 13:00 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 6/9] btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes() Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 7/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref() Qu Wenruo
2019-02-11 13:04 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 8/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent() Qu Wenruo
2019-02-11 13:05 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 9/9] btrfs: qgroup: Don't scan leaf if we're modifying reloc tree Qu Wenruo
2019-04-03 16:29 ` [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list David Sterba
2019-04-04 1:12 ` Qu Wenruo
2019-04-04 6:44 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190211051653.3167-1-wqu@suse.com \
--to=wqu@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.