linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] btrfs: Refactor delayed ref parameter list
@ 2018-12-06  6:58 Qu Wenruo
  2018-12-06  6:58 ` [PATCH 1/8] btrfs: delayed-ref: Introduce better documented delayed ref structures Qu Wenruo
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Qu Wenruo @ 2018-12-06  6:58 UTC (permalink / raw)
  To: linux-btrfs

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.

  This makes later expand on parameter list more and more tricky.

- Different interpretation for the same parameter
  Above @owner for data ref is ino 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 newer and minor new members, we don't need to add a new
parameter to btrfs_add_delayed_tree|data_ref() or
btrfs_inc_extent_ref(), just assign them after generic/data/tree init:
  btrfs_init_generic_ref(&ref, bytenr, num_bytes,
			 root->root_key.objectid, parent);
  btrfs_init_data_ref(&ref, ref_root, owner, offset);
  ref->skip_qgroup = true; /* @skip_qgroup is default to false, so new
			      code doesn't need to care */
  ret = btrfs_inc_extent_ref(trans, &ref);

This should improve the code readability and make later code easier to
write.


Qu Wenruo (8):
  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()

 fs/btrfs/ctree.h       |  11 +--
 fs/btrfs/delayed-ref.c |  43 ++++++---
 fs/btrfs/delayed-ref.h | 121 +++++++++++++++++++++++--
 fs/btrfs/extent-tree.c | 195 +++++++++++++++++++----------------------
 fs/btrfs/file.c        |  43 +++++----
 fs/btrfs/inode.c       |  23 +++--
 fs/btrfs/ioctl.c       |  17 ++--
 fs/btrfs/ref-verify.c  |  53 ++++++-----
 fs/btrfs/ref-verify.h  |  10 +--
 fs/btrfs/relocation.c  |  70 +++++++++------
 fs/btrfs/tree-log.c    |  12 ++-
 11 files changed, 375 insertions(+), 223 deletions(-)

-- 
2.19.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-12-12  5:51 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-06  6:58 [PATCH 0/8] btrfs: Refactor delayed ref parameter list Qu Wenruo
2018-12-06  6:58 ` [PATCH 1/8] btrfs: delayed-ref: Introduce better documented delayed ref structures Qu Wenruo
2018-12-10  9:48   ` Nikolay Borisov
2018-12-10 11:20     ` Qu Wenruo
2018-12-10 11:32       ` Nikolay Borisov
2018-12-10 11:37         ` Qu Wenruo
2018-12-10 17:03           ` David Sterba
2018-12-12  5:09     ` About enum convert (Old "Re: [PATCH 1/8] btrfs: delayed-ref: Introduce better documented delayed ref structures") Qu Wenruo
2018-12-06  6:58 ` [PATCH 2/8] btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref Qu Wenruo
2018-12-07 17:39   ` Nikolay Borisov
2018-12-06  6:58 ` [PATCH 3/8] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref() Qu Wenruo
2018-12-10  9:21   ` Nikolay Borisov
2018-12-12  5:50     ` Qu Wenruo
2018-12-06  6:58 ` [PATCH 4/8] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref() Qu Wenruo
2018-12-10  9:23   ` Nikolay Borisov
2018-12-06  6:59 ` [PATCH 5/8] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod() Qu Wenruo
2018-12-06  6:59 ` [PATCH 6/8] btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes() Qu Wenruo
2018-12-10  9:45   ` Nikolay Borisov
2018-12-06  6:59 ` [PATCH 7/8] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref() Qu Wenruo
2018-12-10  9:52   ` Nikolay Borisov
2018-12-10 17:28     ` David Sterba
2018-12-11  2:02       ` Qu Wenruo
2018-12-06  6:59 ` [PATCH 8/8] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent() Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).