diff mbox series

btrfs: avoid unnecessary ref initialization when freeing log tree block

Message ID 06acd07b61968eaecded7c22d07cd72030fbef6b.1708347488.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: avoid unnecessary ref initialization when freeing log tree block | expand

Commit Message

Filipe Manana Feb. 19, 2024, 1 p.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

At btrfs_free_tree_block(), we are always initializing a delayed reference
to drop the given extent buffer but we only use if it does not belong to a
log root tree. So we are doing unnecessary work here and increasing the
duration of a critical section as this is normally called while holding a
lock on the parent tree block (if any) and while holding a log transaction
open.

So initialize the delayed reference only if the extent buffer is not from
a log tree, avoiding unnecessary work and making the code also a bit
easier to follow.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-tree.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

David Sterba Feb. 19, 2024, 2:46 p.m. UTC | #1 
On Mon, Feb 19, 2024 at 01:00:22PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> At btrfs_free_tree_block(), we are always initializing a delayed reference
> to drop the given extent buffer but we only use if it does not belong to a
> log root tree. So we are doing unnecessary work here and increasing the
> duration of a critical section as this is normally called while holding a
> lock on the parent tree block (if any) and while holding a log transaction
> open.
> 
> So initialize the delayed reference only if the extent buffer is not from
> a log tree, avoiding unnecessary work and making the code also a bit
> easier to follow.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>
Naohiro Aota Feb. 20, 2024, 12:12 a.m. UTC | #2 
On Mon, Feb 19, 2024 at 01:00:22PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> At btrfs_free_tree_block(), we are always initializing a delayed reference
> to drop the given extent buffer but we only use if it does not belong to a
> log root tree. So we are doing unnecessary work here and increasing the
> duration of a critical section as this is normally called while holding a
> lock on the parent tree block (if any) and while holding a log transaction
> open.
> 
> So initialize the delayed reference only if the extent buffer is not from
> a log tree, avoiding unnecessary work and making the code also a bit
> easier to follow.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/extent-tree.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

Exactly what I also thought while debugging around this function.

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
diff mbox series

Patch

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 0d72d0f7cefc..beedd6ed64d3 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3458,16 +3458,17 @@  void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
 			   u64 parent, int last_ref)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_ref generic_ref = { 0 };
 	struct btrfs_block_group *bg;
 	int ret;
 
-	btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF,
-			       buf->start, buf->len, parent, btrfs_header_owner(buf));
-	btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf),
-			    root_id, 0, false);
-
 	if (root_id != BTRFS_TREE_LOG_OBJECTID) {
+		struct btrfs_ref generic_ref = { 0 };
+
+		btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF,
+				       buf->start, buf->len, parent,
+				       btrfs_header_owner(buf));
+		btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf),
+				    root_id, 0, false);
 		btrfs_ref_tree_mod(fs_info, &generic_ref);
 		ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL);
 		BUG_ON(ret); /* -ENOMEM */