* [PATCH 0/3] Cleanups around extent increment
@ 2018-06-18 11:59 Nikolay Borisov
2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Nikolay Borisov @ 2018-06-18 11:59 UTC (permalink / raw)
To: linux-btrfs; +Cc: jeffm, Nikolay Borisov
Hello,
This series improves the functions involved around extent reference increment.
The first patch just removes a redundant argument, the second one documents the
parameters of __btrfs_inc_extent_ref. It can be considered v2 of the standalone
version which Jeff had some input to. The final patch fixes a comment in
lookup_inline_extent_backref which transpired while Jeff was revieweing the
documentation patch.
Nikolay Borisov (3):
btrfs: Remove fs_info argument from __btrfs_inc_extent_ref
btrfs: Document __btrfs_inc_extent_ref
btrfs: Fix comment in lookup_inline_extent_backref
fs/btrfs/extent-tree.c | 51 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 10 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref 2018-06-18 11:59 [PATCH 0/3] Cleanups around extent increment Nikolay Borisov @ 2018-06-18 11:59 ` Nikolay Borisov 2018-06-19 5:24 ` Qu Wenruo ` (2 more replies) 2018-06-18 11:59 ` [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref Nikolay Borisov ` (2 subsequent siblings) 3 siblings, 3 replies; 10+ messages in thread From: Nikolay Borisov @ 2018-06-18 11:59 UTC (permalink / raw) To: linux-btrfs; +Cc: jeffm, Nikolay Borisov This function already takes a transaction which holds a reference to the fs_info struct. Use that reference and remove the extra arg. No functional changes. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent-tree.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 4850e538ab10..59645ced6fbc 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -2208,12 +2208,12 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, } static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, - struct btrfs_fs_info *fs_info, struct btrfs_delayed_ref_node *node, u64 parent, u64 root_objectid, u64 owner, u64 offset, int refs_to_add, struct btrfs_delayed_extent_op *extent_op) { + struct btrfs_fs_info *fs_info = trans->fs_info; struct btrfs_path *path; struct extent_buffer *leaf; struct btrfs_extent_item *item; @@ -2297,10 +2297,9 @@ static int run_delayed_data_ref(struct btrfs_trans_handle *trans, ref->objectid, ref->offset, &ins, node->ref_mod); } else if (node->action == BTRFS_ADD_DELAYED_REF) { - ret = __btrfs_inc_extent_ref(trans, fs_info, node, parent, - ref_root, ref->objectid, - ref->offset, node->ref_mod, - extent_op); + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root, + ref->objectid, ref->offset, + node->ref_mod, extent_op); } else if (node->action == BTRFS_DROP_DELAYED_REF) { ret = __btrfs_free_extent(trans, fs_info, node, parent, ref_root, ref->objectid, @@ -2450,10 +2449,8 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans, BUG_ON(!extent_op || !extent_op->update_flags); ret = alloc_reserved_tree_block(trans, node, extent_op); } else if (node->action == BTRFS_ADD_DELAYED_REF) { - ret = __btrfs_inc_extent_ref(trans, fs_info, node, - parent, ref_root, - ref->level, 0, 1, - extent_op); + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root, + ref->level, 0, 1, extent_op); } else if (node->action == BTRFS_DROP_DELAYED_REF) { ret = __btrfs_free_extent(trans, fs_info, node, parent, ref_root, -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref 2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov @ 2018-06-19 5:24 ` Qu Wenruo 2018-06-19 13:01 ` Nikolay Borisov 2018-06-19 19:31 ` Jeff Mahoney 2 siblings, 0 replies; 10+ messages in thread From: Qu Wenruo @ 2018-06-19 5:24 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs; +Cc: jeffm On 2018年06月18日 19:59, Nikolay Borisov wrote: > This function already takes a transaction which holds a reference to > the fs_info struct. Use that reference and remove the extra arg. No > functional changes. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/extent-tree.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 4850e538ab10..59645ced6fbc 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2208,12 +2208,12 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > } > > static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > - struct btrfs_fs_info *fs_info, > struct btrfs_delayed_ref_node *node, > u64 parent, u64 root_objectid, > u64 owner, u64 offset, int refs_to_add, > struct btrfs_delayed_extent_op *extent_op) > { > + struct btrfs_fs_info *fs_info = trans->fs_info; > struct btrfs_path *path; > struct extent_buffer *leaf; > struct btrfs_extent_item *item; > @@ -2297,10 +2297,9 @@ static int run_delayed_data_ref(struct btrfs_trans_handle *trans, > ref->objectid, ref->offset, > &ins, node->ref_mod); > } else if (node->action == BTRFS_ADD_DELAYED_REF) { > - ret = __btrfs_inc_extent_ref(trans, fs_info, node, parent, > - ref_root, ref->objectid, > - ref->offset, node->ref_mod, > - extent_op); > + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root, > + ref->objectid, ref->offset, > + node->ref_mod, extent_op); > } else if (node->action == BTRFS_DROP_DELAYED_REF) { > ret = __btrfs_free_extent(trans, fs_info, node, parent, > ref_root, ref->objectid, > @@ -2450,10 +2449,8 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans, > BUG_ON(!extent_op || !extent_op->update_flags); > ret = alloc_reserved_tree_block(trans, node, extent_op); > } else if (node->action == BTRFS_ADD_DELAYED_REF) { > - ret = __btrfs_inc_extent_ref(trans, fs_info, node, > - parent, ref_root, > - ref->level, 0, 1, > - extent_op); > + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root, > + ref->level, 0, 1, extent_op); > } else if (node->action == BTRFS_DROP_DELAYED_REF) { > ret = __btrfs_free_extent(trans, fs_info, node, > parent, ref_root, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref 2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov 2018-06-19 5:24 ` Qu Wenruo @ 2018-06-19 13:01 ` Nikolay Borisov 2018-06-19 19:31 ` Jeff Mahoney 2 siblings, 0 replies; 10+ messages in thread From: Nikolay Borisov @ 2018-06-19 13:01 UTC (permalink / raw) To: linux-btrfs On 18.06.2018 14:59, Nikolay Borisov wrote: > This function already takes a transaction which holds a reference to > the fs_info struct. Use that reference and remove the extra arg. No > functional changes. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- Please ignore this patch, since I'm going to re-send it as apart of a larger series dealing specifically with fs_info cleanup. The other 2 are good. <snip> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref 2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov 2018-06-19 5:24 ` Qu Wenruo 2018-06-19 13:01 ` Nikolay Borisov @ 2018-06-19 19:31 ` Jeff Mahoney 2018-06-19 22:36 ` Nikolay Borisov 2 siblings, 1 reply; 10+ messages in thread From: Jeff Mahoney @ 2018-06-19 19:31 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 2855 bytes --] On 6/18/18 7:59 AM, Nikolay Borisov wrote: > This function already takes a transaction which holds a reference to > the fs_info struct. Use that reference and remove the extra arg. No > functional changes. I like the idea here. I wasn't sold at first, but I think if we can standardize on taking only a trans handle when one is required and both a trans and fs_info when it's optional, it'll make the code clearer. This cleanup can percolate up the stack to cover pretty much all of delayed refs. Reviewed-by: Jeff Mahoney <jeffm@suse.com> > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent-tree.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 4850e538ab10..59645ced6fbc 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2208,12 +2208,12 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > } > > static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > - struct btrfs_fs_info *fs_info, > struct btrfs_delayed_ref_node *node, > u64 parent, u64 root_objectid, > u64 owner, u64 offset, int refs_to_add, > struct btrfs_delayed_extent_op *extent_op) > { > + struct btrfs_fs_info *fs_info = trans->fs_info; > struct btrfs_path *path; > struct extent_buffer *leaf; > struct btrfs_extent_item *item; > @@ -2297,10 +2297,9 @@ static int run_delayed_data_ref(struct btrfs_trans_handle *trans, > ref->objectid, ref->offset, > &ins, node->ref_mod); > } else if (node->action == BTRFS_ADD_DELAYED_REF) { > - ret = __btrfs_inc_extent_ref(trans, fs_info, node, parent, > - ref_root, ref->objectid, > - ref->offset, node->ref_mod, > - extent_op); > + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root, > + ref->objectid, ref->offset, > + node->ref_mod, extent_op); > } else if (node->action == BTRFS_DROP_DELAYED_REF) { > ret = __btrfs_free_extent(trans, fs_info, node, parent, > ref_root, ref->objectid, > @@ -2450,10 +2449,8 @@ static int run_delayed_tree_ref(struct btrfs_trans_handle *trans, > BUG_ON(!extent_op || !extent_op->update_flags); > ret = alloc_reserved_tree_block(trans, node, extent_op); > } else if (node->action == BTRFS_ADD_DELAYED_REF) { > - ret = __btrfs_inc_extent_ref(trans, fs_info, node, > - parent, ref_root, > - ref->level, 0, 1, > - extent_op); > + ret = __btrfs_inc_extent_ref(trans, node, parent, ref_root, > + ref->level, 0, 1, extent_op); > } else if (node->action == BTRFS_DROP_DELAYED_REF) { > ret = __btrfs_free_extent(trans, fs_info, node, > parent, ref_root, > -- Jeff Mahoney SUSE Labs [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref 2018-06-19 19:31 ` Jeff Mahoney @ 2018-06-19 22:36 ` Nikolay Borisov 0 siblings, 0 replies; 10+ messages in thread From: Nikolay Borisov @ 2018-06-19 22:36 UTC (permalink / raw) To: Jeff Mahoney, linux-btrfs On 19.06.2018 22:31, Jeff Mahoney wrote: > I like the idea here. I wasn't sold at first, but I think if we can > standardize on taking only a trans handle when one is required and both > a trans and fs_info when it's optional, it'll make the code clearer. > This cleanup can percolate up the stack to cover pretty much all of > delayed refs. I have a 25-something patches which do exactly this. Still WIP, will likely send it tomorrow. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref 2018-06-18 11:59 [PATCH 0/3] Cleanups around extent increment Nikolay Borisov 2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov @ 2018-06-18 11:59 ` Nikolay Borisov 2018-06-19 5:28 ` Qu Wenruo 2018-06-18 11:59 ` [PATCH 3/3] btrfs: Fix comment in lookup_inline_extent_backref Nikolay Borisov 2018-06-19 13:35 ` [PATCH 0/3] Cleanups around extent increment David Sterba 3 siblings, 1 reply; 10+ messages in thread From: Nikolay Borisov @ 2018-06-18 11:59 UTC (permalink / raw) To: linux-btrfs; +Cc: jeffm, Nikolay Borisov Here is a doc-only patch which tires to deobfuscate the terra-incognita that arguments for delayed refs are. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent-tree.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 59645ced6fbc..39d0652bf3f3 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -2207,6 +2207,40 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, return ret; } +/* + * __btrfs_inc_extent_ref - insert backreference for a given extent + * + * @trans: Handle of transaction + * + * @node: The delayed ref node used to get the bytenr/length for + * extent whose references are incremented. + * + * @parent: If this is a shared extent (BTRFS_SHARED_DATA_REF_KEY/ + * BTRFS_SHARED_BLOCK_REF_KEY) then it holds the logical + * bytenr of the parent block. Since new extents are always + * created with indirect references, this will only be the case + * when relocating a shared extent. In that case, root_objectid + * will be BTRFS_TREE_RELOC_OBJECTID. Otheriwse, parent must + * be 0 + * + * @root_objectid: The id of the root where this modification has originated, + * this can be either one of the well-known metadata trees or + * the subvolume id which references this extent. + * + * @owner: For data extents it is the inode number of the owning file. + * For metadata extents this parameter holds the level in the + * tree of the extent. + * + * @offset: For metadata extents the offset is ignored and is currently + * always passed as 0. For data extents it is the fileoffset + * this extent belongs to. + * + * @refs_to_add Number of references to add + * + * @extent_op Pointer to a structure, holding information necessary when + * updating a tree block's flags + * + */ static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, struct btrfs_delayed_ref_node *node, u64 parent, u64 root_objectid, -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref 2018-06-18 11:59 ` [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref Nikolay Borisov @ 2018-06-19 5:28 ` Qu Wenruo 0 siblings, 0 replies; 10+ messages in thread From: Qu Wenruo @ 2018-06-19 5:28 UTC (permalink / raw) To: Nikolay Borisov, linux-btrfs; +Cc: jeffm On 2018年06月18日 19:59, Nikolay Borisov wrote: > Here is a doc-only patch which tires to deobfuscate the terra-incognita > that arguments for delayed refs are. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/extent-tree.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 59645ced6fbc..39d0652bf3f3 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2207,6 +2207,40 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > return ret; > } > > +/* > + * __btrfs_inc_extent_ref - insert backreference for a given extent > + * > + * @trans: Handle of transaction > + * > + * @node: The delayed ref node used to get the bytenr/length for > + * extent whose references are incremented. > + * > + * @parent: If this is a shared extent (BTRFS_SHARED_DATA_REF_KEY/ > + * BTRFS_SHARED_BLOCK_REF_KEY) then it holds the logical > + * bytenr of the parent block. Since new extents are always > + * created with indirect references, this will only be the case > + * when relocating a shared extent. In that case, root_objectid > + * will be BTRFS_TREE_RELOC_OBJECTID. Otheriwse, parent must > + * be 0 > + * > + * @root_objectid: The id of the root where this modification has originated, > + * this can be either one of the well-known metadata trees or > + * the subvolume id which references this extent. > + * > + * @owner: For data extents it is the inode number of the owning file. > + * For metadata extents this parameter holds the level in the > + * tree of the extent. @owner the naming itself is a little confusing, but it's the necessary evil. Or we will have too many parameters. Thanks, Qu > + * > + * @offset: For metadata extents the offset is ignored and is currently > + * always passed as 0. For data extents it is the fileoffset > + * this extent belongs to. > + * > + * @refs_to_add Number of references to add > + * > + * @extent_op Pointer to a structure, holding information necessary when > + * updating a tree block's flags > + * > + */ > static int __btrfs_inc_extent_ref(struct btrfs_trans_handle *trans, > struct btrfs_delayed_ref_node *node, > u64 parent, u64 root_objectid, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] btrfs: Fix comment in lookup_inline_extent_backref 2018-06-18 11:59 [PATCH 0/3] Cleanups around extent increment Nikolay Borisov 2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov 2018-06-18 11:59 ` [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref Nikolay Borisov @ 2018-06-18 11:59 ` Nikolay Borisov 2018-06-19 13:35 ` [PATCH 0/3] Cleanups around extent increment David Sterba 3 siblings, 0 replies; 10+ messages in thread From: Nikolay Borisov @ 2018-06-18 11:59 UTC (permalink / raw) To: linux-btrfs; +Cc: jeffm, Nikolay Borisov The comment wrongfully states that the owner parameter is the level of the parent block. In fact owner is the level of the current block and by adding 1 to it we can eventually get to the parent/root. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 39d0652bf3f3..a470a07d4f2a 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -1635,7 +1635,7 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, extra_size = -1; /* - * Owner is our parent level, so we can just add one to get the level + * Owner is our level, so we can just add one to get the level * for the block we are interested in. */ if (skinny_metadata && owner < BTRFS_FIRST_FREE_OBJECTID) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Cleanups around extent increment 2018-06-18 11:59 [PATCH 0/3] Cleanups around extent increment Nikolay Borisov ` (2 preceding siblings ...) 2018-06-18 11:59 ` [PATCH 3/3] btrfs: Fix comment in lookup_inline_extent_backref Nikolay Borisov @ 2018-06-19 13:35 ` David Sterba 3 siblings, 0 replies; 10+ messages in thread From: David Sterba @ 2018-06-19 13:35 UTC (permalink / raw) To: Nikolay Borisov; +Cc: linux-btrfs, jeffm On Mon, Jun 18, 2018 at 02:59:23PM +0300, Nikolay Borisov wrote: > This series improves the functions involved around extent reference increment. > The first patch just removes a redundant argument, the second one documents the > parameters of __btrfs_inc_extent_ref. It can be considered v2 of the standalone > version which Jeff had some input to. The final patch fixes a comment in > lookup_inline_extent_backref which transpired while Jeff was revieweing the > documentation patch. > > Nikolay Borisov (3): > btrfs: Remove fs_info argument from __btrfs_inc_extent_ref > btrfs: Document __btrfs_inc_extent_ref > btrfs: Fix comment in lookup_inline_extent_backref 2 and 3 added to misc-next, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-06-19 22:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-18 11:59 [PATCH 0/3] Cleanups around extent increment Nikolay Borisov 2018-06-18 11:59 ` [PATCH 1/3] btrfs: Remove fs_info argument from __btrfs_inc_extent_ref Nikolay Borisov 2018-06-19 5:24 ` Qu Wenruo 2018-06-19 13:01 ` Nikolay Borisov 2018-06-19 19:31 ` Jeff Mahoney 2018-06-19 22:36 ` Nikolay Borisov 2018-06-18 11:59 ` [PATCH 2/3] btrfs: Document __btrfs_inc_extent_ref Nikolay Borisov 2018-06-19 5:28 ` Qu Wenruo 2018-06-18 11:59 ` [PATCH 3/3] btrfs: Fix comment in lookup_inline_extent_backref Nikolay Borisov 2018-06-19 13:35 ` [PATCH 0/3] Cleanups around extent increment David Sterba
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.