[PATCH 0/8] Minor cleanups in relocation.c

[PATCH 0/8] Minor cleanups in relocation.c

[David Sterba]

Various type cleanups in relocation.c, on-stack variable to replace
frequent allocation in build_backref_tree().

David Sterba (8):
  btrfs: relocation: use more natural types for tree_block bitfields
  btrfs: relocation: use enum for stages
  btrfs: relocation: switch bitfields to bool in reloc_control
  btrfs: relocation: open code mapping_tree_init
  btrfs: switch btrfs_backref_cache::is_reloc to bool
  btrfs: relocation: return bool from btrfs_should_ignore_reloc_root
  btrfs: relocation: use on-stack iterator in build_backref_tree
  btrfs: relocation: constify parameters where possible

 fs/btrfs/backref.c    |  28 ++++-----
 fs/btrfs/backref.h    |  15 ++---
 fs/btrfs/relocation.c | 139 ++++++++++++++++++++----------------------
 fs/btrfs/relocation.h |   9 +--
 4 files changed, 87 insertions(+), 104 deletions(-)

-- 
2.41.0

[PATCH 1/8] btrfs: relocation: use more natural types for tree_block bitfields

[David Sterba]

We don't need to use bitfields for tree_block::level and
tree_block::key_ready, there's enough padding in the structure for
proper types.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 6e8e14d1aeaa..9ff3572a8451 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -111,8 +111,8 @@ struct tree_block {
 	}; /* Use rb_simple_node for search/insert */
 	u64 owner;
 	struct btrfs_key key;
-	unsigned int level:8;
-	unsigned int key_ready:1;
+	u8 level;
+	bool key_ready;
 };
 
 #define MAX_EXTENTS 128
@@ -2664,7 +2664,7 @@ static int get_tree_block_key(struct btrfs_fs_info *fs_info,
 	else
 		btrfs_node_key_to_cpu(eb, &block->key, 0);
 	free_extent_buffer(eb);
-	block->key_ready = 1;
+	block->key_ready = true;
 	return 0;
 }
 
@@ -3313,7 +3313,7 @@ static int add_tree_block(struct reloc_control *rc,
 	block->key.objectid = rc->extent_root->fs_info->nodesize;
 	block->key.offset = generation;
 	block->level = level;
-	block->key_ready = 0;
+	block->key_ready = false;
 	block->owner = owner;
 
 	rb_node = rb_simple_insert(blocks, block->bytenr, &block->rb_node);
-- 
2.41.0

[PATCH 2/8] btrfs: relocation: use enum for stages

[David Sterba]

Add an enum type for data relocation stages.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 9ff3572a8451..3afe499f00b1 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -125,6 +125,12 @@ struct file_extent_cluster {
 	u64 owning_root;
 };
 
+/* Stages of data relocation. */
+enum reloc_stage {
+	MOVE_DATA_EXTENTS,
+	UPDATE_DATA_PTRS
+};
+
 struct reloc_control {
 	/* block group to relocate */
 	struct btrfs_block_group *block_group;
@@ -156,16 +162,12 @@ struct reloc_control {
 	u64 search_start;
 	u64 extents_found;
 
-	unsigned int stage:8;
+	enum reloc_stage stage;
 	unsigned int create_reloc_tree:1;
 	unsigned int merge_reloc_tree:1;
 	unsigned int found_file_extent:1;
 };
 
-/* stages of data relocation */
-#define MOVE_DATA_EXTENTS	0
-#define UPDATE_DATA_PTRS	1
-
 static void mark_block_processed(struct reloc_control *rc,
 				 struct btrfs_backref_node *node)
 {
@@ -4054,7 +4056,7 @@ static void describe_relocation(struct btrfs_fs_info *fs_info,
 		   block_group->start, buf);
 }
 
-static const char *stage_to_string(int stage)
+static const char *stage_to_string(enum reloc_stage stage)
 {
 	if (stage == MOVE_DATA_EXTENTS)
 		return "move data extents";
@@ -4170,7 +4172,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 	WARN_ON(ret && ret != -EAGAIN);
 
 	while (1) {
-		int finishes_stage;
+		enum reloc_stage finishes_stage;
 
 		mutex_lock(&fs_info->cleaner_mutex);
 		ret = relocate_block_group(rc);
-- 
2.41.0

[PATCH 3/8] btrfs: relocation: switch bitfields to bool in reloc_control

[David Sterba]

Use bool types for the indicators instead of bitfields. The structure
size slightly grows but the new types are placed within the padding.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 3afe499f00b1..87ac8528032c 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -163,9 +163,9 @@ struct reloc_control {
 	u64 extents_found;
 
 	enum reloc_stage stage;
-	unsigned int create_reloc_tree:1;
-	unsigned int merge_reloc_tree:1;
-	unsigned int found_file_extent:1;
+	bool create_reloc_tree;
+	bool merge_reloc_tree;
+	bool found_file_extent;
 };
 
 static void mark_block_processed(struct reloc_control *rc,
@@ -1902,7 +1902,7 @@ int prepare_to_merge(struct reloc_control *rc, int err)
 		}
 	}
 
-	rc->merge_reloc_tree = 1;
+	rc->merge_reloc_tree = true;
 
 	while (!list_empty(&rc->reloc_roots)) {
 		reloc_root = list_entry(rc->reloc_roots.next,
@@ -3659,7 +3659,7 @@ int prepare_to_relocate(struct reloc_control *rc)
 	if (ret)
 		return ret;
 
-	rc->create_reloc_tree = 1;
+	rc->create_reloc_tree = true;
 	set_reloc_control(rc);
 
 	trans = btrfs_join_transaction(rc->extent_root);
@@ -3786,7 +3786,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
 
 		if (rc->stage == MOVE_DATA_EXTENTS &&
 		    (flags & BTRFS_EXTENT_FLAG_DATA)) {
-			rc->found_file_extent = 1;
+			rc->found_file_extent = true;
 			ret = relocate_data_extent(rc->data_inode,
 						   &key, &rc->cluster);
 			if (ret < 0) {
@@ -3823,7 +3823,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
 			err = ret;
 	}
 
-	rc->create_reloc_tree = 0;
+	rc->create_reloc_tree = false;
 	set_reloc_control(rc);
 
 	btrfs_backref_release_cache(&rc->backref_cache);
@@ -3841,7 +3841,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
 
 	merge_reloc_roots(rc);
 
-	rc->merge_reloc_tree = 0;
+	rc->merge_reloc_tree = false;
 	unset_reloc_control(rc);
 	btrfs_block_rsv_release(fs_info, rc->block_rsv, (u64)-1, NULL);
 
@@ -4355,7 +4355,7 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
 		goto out_unset;
 	}
 
-	rc->merge_reloc_tree = 1;
+	rc->merge_reloc_tree = true;
 
 	while (!list_empty(&reloc_roots)) {
 		reloc_root = list_entry(reloc_roots.next,
-- 
2.41.0

[PATCH 4/8] btrfs: relocation: open code mapping_tree_init

[David Sterba]

There's only one user of mapping_tree_init, we don't need a helper for
the simple initialization.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 87ac8528032c..3e662cadecaf 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -183,13 +183,6 @@ static void mark_block_processed(struct reloc_control *rc,
 	node->processed = 1;
 }
 
-
-static void mapping_tree_init(struct mapping_tree *tree)
-{
-	tree->rb_root = RB_ROOT;
-	spin_lock_init(&tree->lock);
-}
-
 /*
  * walk up backref nodes until reach node presents tree root
  */
@@ -4024,7 +4017,8 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 	INIT_LIST_HEAD(&rc->reloc_roots);
 	INIT_LIST_HEAD(&rc->dirty_subvol_roots);
 	btrfs_backref_init_cache(fs_info, &rc->backref_cache, 1);
-	mapping_tree_init(&rc->reloc_root_tree);
+	rc->reloc_root_tree.rb_root = RB_ROOT;
+	spin_lock_init(&rc->reloc_root_tree.lock);
 	extent_io_tree_init(fs_info, &rc->processed_blocks, IO_TREE_RELOC_BLOCKS);
 	return rc;
 }
-- 
2.41.0

[PATCH 5/8] btrfs: switch btrfs_backref_cache::is_reloc to bool

[David Sterba]

The btrfs_backref_cache::is_reloc is an indicator variable and should
use a bool type.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/backref.c    | 2 +-
 fs/btrfs/backref.h    | 4 ++--
 fs/btrfs/relocation.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 0cde873bdee2..0dc91bf654b5 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -3001,7 +3001,7 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
 }
 
 void btrfs_backref_init_cache(struct btrfs_fs_info *fs_info,
-			      struct btrfs_backref_cache *cache, int is_reloc)
+			      struct btrfs_backref_cache *cache, bool is_reloc)
 {
 	int i;
 
diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
index 3b077d10bbc0..83a9a34e948e 100644
--- a/fs/btrfs/backref.h
+++ b/fs/btrfs/backref.h
@@ -440,11 +440,11 @@ struct btrfs_backref_cache {
 	 * Reloction backref cache require more info for reloc root compared
 	 * to generic backref cache.
 	 */
-	unsigned int is_reloc;
+	bool is_reloc;
 };
 
 void btrfs_backref_init_cache(struct btrfs_fs_info *fs_info,
-			      struct btrfs_backref_cache *cache, int is_reloc);
+			      struct btrfs_backref_cache *cache, bool is_reloc);
 struct btrfs_backref_node *btrfs_backref_alloc_node(
 		struct btrfs_backref_cache *cache, u64 bytenr, int level);
 struct btrfs_backref_edge *btrfs_backref_alloc_edge(
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 3e662cadecaf..75463377f418 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4016,7 +4016,7 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 
 	INIT_LIST_HEAD(&rc->reloc_roots);
 	INIT_LIST_HEAD(&rc->dirty_subvol_roots);
-	btrfs_backref_init_cache(fs_info, &rc->backref_cache, 1);
+	btrfs_backref_init_cache(fs_info, &rc->backref_cache, true);
 	rc->reloc_root_tree.rb_root = RB_ROOT;
 	spin_lock_init(&rc->reloc_root_tree.lock);
 	extent_io_tree_init(fs_info, &rc->processed_blocks, IO_TREE_RELOC_BLOCKS);
-- 
2.41.0

[PATCH 6/8] btrfs: relocation: return bool from btrfs_should_ignore_reloc_root

[David Sterba]

btrfs_should_ignore_reloc_root() is a predicate so it should return
bool.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 19 +++++++++----------
 fs/btrfs/relocation.h |  2 +-
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 75463377f418..d1dcbb15baa7 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -325,31 +325,30 @@ static bool have_reloc_root(struct btrfs_root *root)
 	return true;
 }
 
-int btrfs_should_ignore_reloc_root(struct btrfs_root *root)
+bool btrfs_should_ignore_reloc_root(struct btrfs_root *root)
 {
 	struct btrfs_root *reloc_root;
 
 	if (!test_bit(BTRFS_ROOT_SHAREABLE, &root->state))
-		return 0;
+		return false;
 
 	/* This root has been merged with its reloc tree, we can ignore it */
 	if (reloc_root_is_dead(root))
-		return 1;
+		return true;
 
 	reloc_root = root->reloc_root;
 	if (!reloc_root)
-		return 0;
+		return false;
 
 	if (btrfs_header_generation(reloc_root->commit_root) ==
 	    root->fs_info->running_transaction->transid)
-		return 0;
+		return false;
 	/*
-	 * if there is reloc tree and it was created in previous
-	 * transaction backref lookup can find the reloc tree,
-	 * so backref node for the fs tree root is useless for
-	 * relocation.
+	 * If there is reloc tree and it was created in previous transaction
+	 * backref lookup can find the reloc tree, so backref node for the fs
+	 * tree root is useless for relocation.
 	 */
-	return 1;
+	return true;
 }
 
 /*
diff --git a/fs/btrfs/relocation.h b/fs/btrfs/relocation.h
index 77d69f6ae967..af749c780b4e 100644
--- a/fs/btrfs/relocation.h
+++ b/fs/btrfs/relocation.h
@@ -18,7 +18,7 @@ int btrfs_reloc_post_snapshot(struct btrfs_trans_handle *trans,
 			      struct btrfs_pending_snapshot *pending);
 int btrfs_should_cancel_balance(struct btrfs_fs_info *fs_info);
 struct btrfs_root *find_reloc_root(struct btrfs_fs_info *fs_info, u64 bytenr);
-int btrfs_should_ignore_reloc_root(struct btrfs_root *root);
+bool btrfs_should_ignore_reloc_root(struct btrfs_root *root);
 u64 btrfs_get_reloc_bg_bytenr(struct btrfs_fs_info *fs_info);
 
 #endif
-- 
2.41.0

[PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[David Sterba]

build_backref_tree() is called in a loop by relocate_tree_blocks()
for each relocated block. The iterator is allocated and freed repeatedly
while we could simply use an on-stack variable to avoid the allocation
and remove one more failure case. The stack grows by 48 bytes.

This was the only use of btrfs_backref_iter_alloc() so it's changed to
be an initializer and btrfs_backref_iter_free() can be removed
completely.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/backref.c    | 26 ++++++++++----------------
 fs/btrfs/backref.h    | 11 ++---------
 fs/btrfs/relocation.c | 12 ++++++------
 3 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 0dc91bf654b5..691b20b47065 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -2828,26 +2828,20 @@ void free_ipath(struct inode_fs_paths *ipath)
 	kfree(ipath);
 }
 
-struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
+int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
+			    struct btrfs_backref_iter *iter)
 {
-	struct btrfs_backref_iter *ret;
-
-	ret = kzalloc(sizeof(*ret), GFP_NOFS);
-	if (!ret)
-		return NULL;
-
-	ret->path = btrfs_alloc_path();
-	if (!ret->path) {
-		kfree(ret);
-		return NULL;
-	}
+	memset(iter, 0, sizeof(struct btrfs_backref_iter));
+	iter->path = btrfs_alloc_path();
+	if (!iter->path)
+		return -ENOMEM;
 
 	/* Current backref iterator only supports iteration in commit root */
-	ret->path->search_commit_root = 1;
-	ret->path->skip_locking = 1;
-	ret->fs_info = fs_info;
+	iter->path->search_commit_root = 1;
+	iter->path->skip_locking = 1;
+	iter->fs_info = fs_info;
 
-	return ret;
+	return 0;
 }
 
 int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
index 83a9a34e948e..24fabbd2a80a 100644
--- a/fs/btrfs/backref.h
+++ b/fs/btrfs/backref.h
@@ -269,15 +269,8 @@ struct btrfs_backref_iter {
 	u32 end_ptr;
 };
 
-struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
-
-static inline void btrfs_backref_iter_free(struct btrfs_backref_iter *iter)
-{
-	if (!iter)
-		return;
-	btrfs_free_path(iter->path);
-	kfree(iter);
-}
+int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
+			    struct btrfs_backref_iter *iter);
 
 static inline struct extent_buffer *btrfs_backref_get_eb(
 		struct btrfs_backref_iter *iter)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d1dcbb15baa7..6a31e73c3df7 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -464,7 +464,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
 			struct reloc_control *rc, struct btrfs_key *node_key,
 			int level, u64 bytenr)
 {
-	struct btrfs_backref_iter *iter;
+	struct btrfs_backref_iter iter;
 	struct btrfs_backref_cache *cache = &rc->backref_cache;
 	/* For searching parent of TREE_BLOCK_REF */
 	struct btrfs_path *path;
@@ -474,9 +474,9 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
 	int ret;
 	int err = 0;
 
-	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
-	if (!iter)
-		return ERR_PTR(-ENOMEM);
+	ret = btrfs_backref_iter_init(rc->extent_root->fs_info, &iter);
+	if (ret < 0)
+		return ERR_PTR(ret);
 	path = btrfs_alloc_path();
 	if (!path) {
 		err = -ENOMEM;
@@ -494,7 +494,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
 
 	/* Breadth-first search to build backref cache */
 	do {
-		ret = btrfs_backref_add_tree_node(cache, path, iter, node_key,
+		ret = btrfs_backref_add_tree_node(cache, path, &iter, node_key,
 						  cur);
 		if (ret < 0) {
 			err = ret;
@@ -522,7 +522,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
 	if (handle_useless_nodes(rc, node))
 		node = NULL;
 out:
-	btrfs_backref_iter_free(iter);
+	btrfs_backref_iter_release(&iter);
 	btrfs_free_path(path);
 	if (err) {
 		btrfs_backref_error_cleanup(cache, node);
-- 
2.41.0

[PATCH 8/8] btrfs: relocation: constify parameters where possible

[David Sterba]

Lots of the functions in relocation.c don't change pointer parameters
but lack the annotations. Add them and reformat according to current
coding style if needed.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/relocation.c | 56 +++++++++++++++++++++----------------------
 fs/btrfs/relocation.h |  9 +++----
 2 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 6a31e73c3df7..903621a65244 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -295,7 +295,7 @@ static int update_backref_cache(struct btrfs_trans_handle *trans,
 	return 1;
 }
 
-static bool reloc_root_is_dead(struct btrfs_root *root)
+static bool reloc_root_is_dead(const struct btrfs_root *root)
 {
 	/*
 	 * Pair with set_bit/clear_bit in clean_dirty_subvols and
@@ -316,7 +316,7 @@ static bool reloc_root_is_dead(struct btrfs_root *root)
  * from no reloc root.  But btrfs_should_ignore_reloc_root() below is a
  * special case.
  */
-static bool have_reloc_root(struct btrfs_root *root)
+static bool have_reloc_root(const struct btrfs_root *root)
 {
 	if (reloc_root_is_dead(root))
 		return false;
@@ -325,7 +325,7 @@ static bool have_reloc_root(struct btrfs_root *root)
 	return true;
 }
 
-bool btrfs_should_ignore_reloc_root(struct btrfs_root *root)
+bool btrfs_should_ignore_reloc_root(const struct btrfs_root *root)
 {
 	struct btrfs_root *reloc_root;
 
@@ -541,7 +541,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
  */
 static int clone_backref_node(struct btrfs_trans_handle *trans,
 			      struct reloc_control *rc,
-			      struct btrfs_root *src,
+			      const struct btrfs_root *src,
 			      struct btrfs_root *dest)
 {
 	struct btrfs_root *reloc_root = src->reloc_root;
@@ -1181,9 +1181,9 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static noinline_for_stack
-int memcmp_node_keys(struct extent_buffer *eb, int slot,
-		     struct btrfs_path *path, int level)
+static noinline_for_stack int memcmp_node_keys(const struct extent_buffer *eb,
+					       int slot, const struct btrfs_path *path,
+					       int level)
 {
 	struct btrfs_disk_key key1;
 	struct btrfs_disk_key key2;
@@ -1515,8 +1515,8 @@ int walk_down_reloc_tree(struct btrfs_root *root, struct btrfs_path *path,
  * [min_key, max_key)
  */
 static int invalidate_extent_cache(struct btrfs_root *root,
-				   struct btrfs_key *min_key,
-				   struct btrfs_key *max_key)
+				   const struct btrfs_key *min_key,
+				   const struct btrfs_key *max_key)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct inode *inode = NULL;
@@ -2828,7 +2828,7 @@ int relocate_tree_blocks(struct btrfs_trans_handle *trans,
 
 static noinline_for_stack int prealloc_file_extent_cluster(
 				struct btrfs_inode *inode,
-				struct file_extent_cluster *cluster)
+				const struct file_extent_cluster *cluster)
 {
 	u64 alloc_hint = 0;
 	u64 start;
@@ -2963,7 +2963,7 @@ static noinline_for_stack int setup_relocation_extent_mapping(struct inode *inod
 /*
  * Allow error injection to test balance/relocation cancellation
  */
-noinline int btrfs_should_cancel_balance(struct btrfs_fs_info *fs_info)
+noinline int btrfs_should_cancel_balance(const struct btrfs_fs_info *fs_info)
 {
 	return atomic_read(&fs_info->balance_cancel_req) ||
 		atomic_read(&fs_info->reloc_cancel_req) ||
@@ -2971,7 +2971,7 @@ noinline int btrfs_should_cancel_balance(struct btrfs_fs_info *fs_info)
 }
 ALLOW_ERROR_INJECTION(btrfs_should_cancel_balance, TRUE);
 
-static u64 get_cluster_boundary_end(struct file_extent_cluster *cluster,
+static u64 get_cluster_boundary_end(const struct file_extent_cluster *cluster,
 				    int cluster_nr)
 {
 	/* Last extent, use cluster end directly */
@@ -2983,7 +2983,7 @@ static u64 get_cluster_boundary_end(struct file_extent_cluster *cluster,
 }
 
 static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
-			     struct file_extent_cluster *cluster,
+			     const struct file_extent_cluster *cluster,
 			     int *cluster_nr, unsigned long page_index)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
@@ -3118,7 +3118,7 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,
 }
 
 static int relocate_file_extent_cluster(struct inode *inode,
-					struct file_extent_cluster *cluster)
+					const struct file_extent_cluster *cluster)
 {
 	u64 offset = BTRFS_I(inode)->index_cnt;
 	unsigned long index;
@@ -3156,9 +3156,9 @@ static int relocate_file_extent_cluster(struct inode *inode,
 	return ret;
 }
 
-static noinline_for_stack
-int relocate_data_extent(struct inode *inode, struct btrfs_key *extent_key,
-			 struct file_extent_cluster *cluster)
+static noinline_for_stack int relocate_data_extent(struct inode *inode,
+				const struct btrfs_key *extent_key,
+				struct file_extent_cluster *cluster)
 {
 	int ret;
 	struct btrfs_root *root = BTRFS_I(inode)->root;
@@ -3222,7 +3222,7 @@ int relocate_data_extent(struct inode *inode, struct btrfs_key *extent_key,
  * the major work is getting the generation and level of the block
  */
 static int add_tree_block(struct reloc_control *rc,
-			  struct btrfs_key *extent_key,
+			  const struct btrfs_key *extent_key,
 			  struct btrfs_path *path,
 			  struct rb_root *blocks)
 {
@@ -3473,11 +3473,10 @@ static int delete_v1_space_cache(struct extent_buffer *leaf,
 /*
  * helper to find all tree blocks that reference a given data extent
  */
-static noinline_for_stack
-int add_data_references(struct reloc_control *rc,
-			struct btrfs_key *extent_key,
-			struct btrfs_path *path,
-			struct rb_root *blocks)
+static noinline_for_stack int add_data_references(struct reloc_control *rc,
+						  const struct btrfs_key *extent_key,
+						  struct btrfs_path *path,
+						  struct rb_root *blocks)
 {
 	struct btrfs_backref_walk_ctx ctx = { 0 };
 	struct ulist_iterator leaf_uiter;
@@ -3918,9 +3917,9 @@ static void delete_orphan_inode(struct btrfs_trans_handle *trans,
  * helper to create inode for data relocation.
  * the inode is in data relocation tree and its link count is 0
  */
-static noinline_for_stack
-struct inode *create_reloc_inode(struct btrfs_fs_info *fs_info,
-				 struct btrfs_block_group *group)
+static noinline_for_stack struct inode *create_reloc_inode(
+					struct btrfs_fs_info *fs_info,
+					const struct btrfs_block_group *group)
 {
 	struct inode *inode = NULL;
 	struct btrfs_trans_handle *trans;
@@ -4467,7 +4466,8 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
 }
 
 int btrfs_reloc_cow_block(struct btrfs_trans_handle *trans,
-			  struct btrfs_root *root, struct extent_buffer *buf,
+			  struct btrfs_root *root,
+			  const struct extent_buffer *buf,
 			  struct extent_buffer *cow)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -4606,7 +4606,7 @@ int btrfs_reloc_post_snapshot(struct btrfs_trans_handle *trans,
  *
  * Return U64_MAX if no running relocation.
  */
-u64 btrfs_get_reloc_bg_bytenr(struct btrfs_fs_info *fs_info)
+u64 btrfs_get_reloc_bg_bytenr(const struct btrfs_fs_info *fs_info)
 {
 	u64 logical = U64_MAX;
 
diff --git a/fs/btrfs/relocation.h b/fs/btrfs/relocation.h
index af749c780b4e..5fb60f2deb53 100644
--- a/fs/btrfs/relocation.h
+++ b/fs/btrfs/relocation.h
@@ -10,15 +10,16 @@ int btrfs_update_reloc_root(struct btrfs_trans_handle *trans,
 int btrfs_recover_relocation(struct btrfs_fs_info *fs_info);
 int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered);
 int btrfs_reloc_cow_block(struct btrfs_trans_handle *trans,
-			  struct btrfs_root *root, struct extent_buffer *buf,
+			  struct btrfs_root *root,
+			  const struct extent_buffer *buf,
 			  struct extent_buffer *cow);
 void btrfs_reloc_pre_snapshot(struct btrfs_pending_snapshot *pending,
 			      u64 *bytes_to_reserve);
 int btrfs_reloc_post_snapshot(struct btrfs_trans_handle *trans,
 			      struct btrfs_pending_snapshot *pending);
-int btrfs_should_cancel_balance(struct btrfs_fs_info *fs_info);
+int btrfs_should_cancel_balance(const struct btrfs_fs_info *fs_info);
 struct btrfs_root *find_reloc_root(struct btrfs_fs_info *fs_info, u64 bytenr);
-bool btrfs_should_ignore_reloc_root(struct btrfs_root *root);
-u64 btrfs_get_reloc_bg_bytenr(struct btrfs_fs_info *fs_info);
+bool btrfs_should_ignore_reloc_root(const struct btrfs_root *root);
+u64 btrfs_get_reloc_bg_bytenr(const struct btrfs_fs_info *fs_info);
 
 #endif
-- 
2.41.0

Re: [PATCH 1/8] btrfs: relocation: use more natural types for tree_block bitfields

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 2/8] btrfs: relocation: use enum for stages

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 3/8] btrfs: relocation: switch bitfields to bool in reloc_control

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 4/8] btrfs: relocation: open code mapping_tree_init

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 5/8] btrfs: switch btrfs_backref_cache::is_reloc to bool

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 6/8] btrfs: relocation: return bool from btrfs_should_ignore_reloc_root

[Johannes Thumshirn]

On 22.09.23 13:14, David Sterba wrote:
> btrfs_should_ignore_reloc_root() is a predicate so it should return
> bool.

Btw both callsites also check for btrfs_backref_cache::is_reloc, so 
maybe that check can be merged into btrfs_should_ignore_reloc_root() as 
well?

Anyways,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 8/8] btrfs: relocation: constify parameters where possible

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 6/8] btrfs: relocation: return bool from btrfs_should_ignore_reloc_root

[David Sterba]

On Fri, Sep 22, 2023 at 12:36:31PM +0000, Johannes Thumshirn wrote:
> On 22.09.23 13:14, David Sterba wrote:
> > btrfs_should_ignore_reloc_root() is a predicate so it should return
> > bool.
> 
> Btw both callsites also check for btrfs_backref_cache::is_reloc, so 
> maybe that check can be merged into btrfs_should_ignore_reloc_root() as 
> well?

The helper is related to the root, for the is_reloc it would also have
to take the cache which looks like a separate thing. I think it would
obscure the condition.

Re: [PATCH 1/8] btrfs: relocation: use more natural types for tree_block bitfields

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> We don't need to use bitfields for tree_block::level and
> tree_block::key_ready, there's enough padding in the structure for
> proper types.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/relocation.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 6e8e14d1aeaa..9ff3572a8451 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -111,8 +111,8 @@ struct tree_block {
>   	}; /* Use rb_simple_node for search/insert */
>   	u64 owner;
>   	struct btrfs_key key;
> -	unsigned int level:8;
> -	unsigned int key_ready:1;
> +	u8 level;
> +	bool key_ready;
>   };
>
>   #define MAX_EXTENTS 128
> @@ -2664,7 +2664,7 @@ static int get_tree_block_key(struct btrfs_fs_info *fs_info,
>   	else
>   		btrfs_node_key_to_cpu(eb, &block->key, 0);
>   	free_extent_buffer(eb);
> -	block->key_ready = 1;
> +	block->key_ready = true;
>   	return 0;
>   }
>
> @@ -3313,7 +3313,7 @@ static int add_tree_block(struct reloc_control *rc,
>   	block->key.objectid = rc->extent_root->fs_info->nodesize;
>   	block->key.offset = generation;
>   	block->level = level;
> -	block->key_ready = 0;
> +	block->key_ready = false;
>   	block->owner = owner;
>
>   	rb_node = rb_simple_insert(blocks, block->bytenr, &block->rb_node);

Re: [PATCH 2/8] btrfs: relocation: use enum for stages

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> Add an enum type for data relocation stages.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/relocation.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 9ff3572a8451..3afe499f00b1 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -125,6 +125,12 @@ struct file_extent_cluster {
>   	u64 owning_root;
>   };
>
> +/* Stages of data relocation. */
> +enum reloc_stage {
> +	MOVE_DATA_EXTENTS,
> +	UPDATE_DATA_PTRS
> +};
> +
>   struct reloc_control {
>   	/* block group to relocate */
>   	struct btrfs_block_group *block_group;
> @@ -156,16 +162,12 @@ struct reloc_control {
>   	u64 search_start;
>   	u64 extents_found;
>
> -	unsigned int stage:8;
> +	enum reloc_stage stage;

Wouldn't this cause extra hole?

Thanks,
Qu
>   	unsigned int create_reloc_tree:1;
>   	unsigned int merge_reloc_tree:1;
>   	unsigned int found_file_extent:1;
>   };
>
> -/* stages of data relocation */
> -#define MOVE_DATA_EXTENTS	0
> -#define UPDATE_DATA_PTRS	1
> -
>   static void mark_block_processed(struct reloc_control *rc,
>   				 struct btrfs_backref_node *node)
>   {
> @@ -4054,7 +4056,7 @@ static void describe_relocation(struct btrfs_fs_info *fs_info,
>   		   block_group->start, buf);
>   }
>
> -static const char *stage_to_string(int stage)
> +static const char *stage_to_string(enum reloc_stage stage)
>   {
>   	if (stage == MOVE_DATA_EXTENTS)
>   		return "move data extents";
> @@ -4170,7 +4172,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
>   	WARN_ON(ret && ret != -EAGAIN);
>
>   	while (1) {
> -		int finishes_stage;
> +		enum reloc_stage finishes_stage;
>
>   		mutex_lock(&fs_info->cleaner_mutex);
>   		ret = relocate_block_group(rc);

Re: [PATCH 3/8] btrfs: relocation: switch bitfields to bool in reloc_control

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> Use bool types for the indicators instead of bitfields. The structure
> size slightly grows but the new types are placed within the padding.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/relocation.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 3afe499f00b1..87ac8528032c 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -163,9 +163,9 @@ struct reloc_control {
>   	u64 extents_found;
>
>   	enum reloc_stage stage;
> -	unsigned int create_reloc_tree:1;
> -	unsigned int merge_reloc_tree:1;
> -	unsigned int found_file_extent:1;
> +	bool create_reloc_tree;
> +	bool merge_reloc_tree;
> +	bool found_file_extent;
>   };
>
>   static void mark_block_processed(struct reloc_control *rc,
> @@ -1902,7 +1902,7 @@ int prepare_to_merge(struct reloc_control *rc, int err)
>   		}
>   	}
>
> -	rc->merge_reloc_tree = 1;
> +	rc->merge_reloc_tree = true;
>
>   	while (!list_empty(&rc->reloc_roots)) {
>   		reloc_root = list_entry(rc->reloc_roots.next,
> @@ -3659,7 +3659,7 @@ int prepare_to_relocate(struct reloc_control *rc)
>   	if (ret)
>   		return ret;
>
> -	rc->create_reloc_tree = 1;
> +	rc->create_reloc_tree = true;
>   	set_reloc_control(rc);
>
>   	trans = btrfs_join_transaction(rc->extent_root);
> @@ -3786,7 +3786,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
>
>   		if (rc->stage == MOVE_DATA_EXTENTS &&
>   		    (flags & BTRFS_EXTENT_FLAG_DATA)) {
> -			rc->found_file_extent = 1;
> +			rc->found_file_extent = true;
>   			ret = relocate_data_extent(rc->data_inode,
>   						   &key, &rc->cluster);
>   			if (ret < 0) {
> @@ -3823,7 +3823,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
>   			err = ret;
>   	}
>
> -	rc->create_reloc_tree = 0;
> +	rc->create_reloc_tree = false;
>   	set_reloc_control(rc);
>
>   	btrfs_backref_release_cache(&rc->backref_cache);
> @@ -3841,7 +3841,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
>
>   	merge_reloc_roots(rc);
>
> -	rc->merge_reloc_tree = 0;
> +	rc->merge_reloc_tree = false;
>   	unset_reloc_control(rc);
>   	btrfs_block_rsv_release(fs_info, rc->block_rsv, (u64)-1, NULL);
>
> @@ -4355,7 +4355,7 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
>   		goto out_unset;
>   	}
>
> -	rc->merge_reloc_tree = 1;
> +	rc->merge_reloc_tree = true;
>
>   	while (!list_empty(&reloc_roots)) {
>   		reloc_root = list_entry(reloc_roots.next,

Re: [PATCH 4/8] btrfs: relocation: open code mapping_tree_init

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> There's only one user of mapping_tree_init, we don't need a helper for
> the simple initialization.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/relocation.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 87ac8528032c..3e662cadecaf 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -183,13 +183,6 @@ static void mark_block_processed(struct reloc_control *rc,
>   	node->processed = 1;
>   }
>
> -
> -static void mapping_tree_init(struct mapping_tree *tree)
> -{
> -	tree->rb_root = RB_ROOT;
> -	spin_lock_init(&tree->lock);
> -}
> -
>   /*
>    * walk up backref nodes until reach node presents tree root
>    */
> @@ -4024,7 +4017,8 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
>   	INIT_LIST_HEAD(&rc->reloc_roots);
>   	INIT_LIST_HEAD(&rc->dirty_subvol_roots);
>   	btrfs_backref_init_cache(fs_info, &rc->backref_cache, 1);
> -	mapping_tree_init(&rc->reloc_root_tree);
> +	rc->reloc_root_tree.rb_root = RB_ROOT;
> +	spin_lock_init(&rc->reloc_root_tree.lock);
>   	extent_io_tree_init(fs_info, &rc->processed_blocks, IO_TREE_RELOC_BLOCKS);
>   	return rc;
>   }

Re: [PATCH 5/8] btrfs: switch btrfs_backref_cache::is_reloc to bool

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> The btrfs_backref_cache::is_reloc is an indicator variable and should
> use a bool type.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/backref.c    | 2 +-
>   fs/btrfs/backref.h    | 4 ++--
>   fs/btrfs/relocation.c | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 0cde873bdee2..0dc91bf654b5 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -3001,7 +3001,7 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
>   }
>
>   void btrfs_backref_init_cache(struct btrfs_fs_info *fs_info,
> -			      struct btrfs_backref_cache *cache, int is_reloc)
> +			      struct btrfs_backref_cache *cache, bool is_reloc)
>   {
>   	int i;
>
> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
> index 3b077d10bbc0..83a9a34e948e 100644
> --- a/fs/btrfs/backref.h
> +++ b/fs/btrfs/backref.h
> @@ -440,11 +440,11 @@ struct btrfs_backref_cache {
>   	 * Reloction backref cache require more info for reloc root compared
>   	 * to generic backref cache.
>   	 */
> -	unsigned int is_reloc;
> +	bool is_reloc;
>   };
>
>   void btrfs_backref_init_cache(struct btrfs_fs_info *fs_info,
> -			      struct btrfs_backref_cache *cache, int is_reloc);
> +			      struct btrfs_backref_cache *cache, bool is_reloc);
>   struct btrfs_backref_node *btrfs_backref_alloc_node(
>   		struct btrfs_backref_cache *cache, u64 bytenr, int level);
>   struct btrfs_backref_edge *btrfs_backref_alloc_edge(
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 3e662cadecaf..75463377f418 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4016,7 +4016,7 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
>
>   	INIT_LIST_HEAD(&rc->reloc_roots);
>   	INIT_LIST_HEAD(&rc->dirty_subvol_roots);
> -	btrfs_backref_init_cache(fs_info, &rc->backref_cache, 1);
> +	btrfs_backref_init_cache(fs_info, &rc->backref_cache, true);
>   	rc->reloc_root_tree.rb_root = RB_ROOT;
>   	spin_lock_init(&rc->reloc_root_tree.lock);
>   	extent_io_tree_init(fs_info, &rc->processed_blocks, IO_TREE_RELOC_BLOCKS);

Re: [PATCH 6/8] btrfs: relocation: return bool from btrfs_should_ignore_reloc_root

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> btrfs_should_ignore_reloc_root() is a predicate so it should return
> bool.
>
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/relocation.c | 19 +++++++++----------
>   fs/btrfs/relocation.h |  2 +-
>   2 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 75463377f418..d1dcbb15baa7 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -325,31 +325,30 @@ static bool have_reloc_root(struct btrfs_root *root)
>   	return true;
>   }
>
> -int btrfs_should_ignore_reloc_root(struct btrfs_root *root)
> +bool btrfs_should_ignore_reloc_root(struct btrfs_root *root)
>   {
>   	struct btrfs_root *reloc_root;
>
>   	if (!test_bit(BTRFS_ROOT_SHAREABLE, &root->state))
> -		return 0;
> +		return false;
>
>   	/* This root has been merged with its reloc tree, we can ignore it */
>   	if (reloc_root_is_dead(root))
> -		return 1;
> +		return true;
>
>   	reloc_root = root->reloc_root;
>   	if (!reloc_root)
> -		return 0;
> +		return false;
>
>   	if (btrfs_header_generation(reloc_root->commit_root) ==
>   	    root->fs_info->running_transaction->transid)
> -		return 0;
> +		return false;
>   	/*
> -	 * if there is reloc tree and it was created in previous
> -	 * transaction backref lookup can find the reloc tree,
> -	 * so backref node for the fs tree root is useless for
> -	 * relocation.
> +	 * If there is reloc tree and it was created in previous transaction
> +	 * backref lookup can find the reloc tree, so backref node for the fs
> +	 * tree root is useless for relocation.
>   	 */
> -	return 1;
> +	return true;
>   }
>
>   /*
> diff --git a/fs/btrfs/relocation.h b/fs/btrfs/relocation.h
> index 77d69f6ae967..af749c780b4e 100644
> --- a/fs/btrfs/relocation.h
> +++ b/fs/btrfs/relocation.h
> @@ -18,7 +18,7 @@ int btrfs_reloc_post_snapshot(struct btrfs_trans_handle *trans,
>   			      struct btrfs_pending_snapshot *pending);
>   int btrfs_should_cancel_balance(struct btrfs_fs_info *fs_info);
>   struct btrfs_root *find_reloc_root(struct btrfs_fs_info *fs_info, u64 bytenr);
> -int btrfs_should_ignore_reloc_root(struct btrfs_root *root);
> +bool btrfs_should_ignore_reloc_root(struct btrfs_root *root);
>   u64 btrfs_get_reloc_bg_bytenr(struct btrfs_fs_info *fs_info);
>
>   #endif

Re: [PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[Qu Wenruo]

On 2023/9/22 20:37, David Sterba wrote:
> build_backref_tree() is called in a loop by relocate_tree_blocks()
> for each relocated block. The iterator is allocated and freed repeatedly
> while we could simply use an on-stack variable to avoid the allocation
> and remove one more failure case. The stack grows by 48 bytes.
>
> This was the only use of btrfs_backref_iter_alloc() so it's changed to
> be an initializer and btrfs_backref_iter_free() can be removed
> completely.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/backref.c    | 26 ++++++++++----------------
>   fs/btrfs/backref.h    | 11 ++---------
>   fs/btrfs/relocation.c | 12 ++++++------
>   3 files changed, 18 insertions(+), 31 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 0dc91bf654b5..691b20b47065 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -2828,26 +2828,20 @@ void free_ipath(struct inode_fs_paths *ipath)
>   	kfree(ipath);
>   }
>
> -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
> +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> +			    struct btrfs_backref_iter *iter)
>   {
> -	struct btrfs_backref_iter *ret;
> -
> -	ret = kzalloc(sizeof(*ret), GFP_NOFS);
> -	if (!ret)
> -		return NULL;
> -
> -	ret->path = btrfs_alloc_path();
> -	if (!ret->path) {
> -		kfree(ret);
> -		return NULL;
> -	}
> +	memset(iter, 0, sizeof(struct btrfs_backref_iter));
> +	iter->path = btrfs_alloc_path();
> +	if (!iter->path)
> +		return -ENOMEM;

We can do one step further, by integrating the btrfs_path into @iter,
other than re-allocating it again and again.

Thanks,
Qu
>
>   	/* Current backref iterator only supports iteration in commit root */
> -	ret->path->search_commit_root = 1;
> -	ret->path->skip_locking = 1;
> -	ret->fs_info = fs_info;
> +	iter->path->search_commit_root = 1;
> +	iter->path->skip_locking = 1;
> +	iter->fs_info = fs_info;
>
> -	return ret;
> +	return 0;
>   }
>
>   int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
> index 83a9a34e948e..24fabbd2a80a 100644
> --- a/fs/btrfs/backref.h
> +++ b/fs/btrfs/backref.h
> @@ -269,15 +269,8 @@ struct btrfs_backref_iter {
>   	u32 end_ptr;
>   };
>
> -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
> -
> -static inline void btrfs_backref_iter_free(struct btrfs_backref_iter *iter)
> -{
> -	if (!iter)
> -		return;
> -	btrfs_free_path(iter->path);
> -	kfree(iter);
> -}
> +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> +			    struct btrfs_backref_iter *iter);
>
>   static inline struct extent_buffer *btrfs_backref_get_eb(
>   		struct btrfs_backref_iter *iter)
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index d1dcbb15baa7..6a31e73c3df7 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -464,7 +464,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>   			struct reloc_control *rc, struct btrfs_key *node_key,
>   			int level, u64 bytenr)
>   {
> -	struct btrfs_backref_iter *iter;
> +	struct btrfs_backref_iter iter;
>   	struct btrfs_backref_cache *cache = &rc->backref_cache;
>   	/* For searching parent of TREE_BLOCK_REF */
>   	struct btrfs_path *path;
> @@ -474,9 +474,9 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>   	int ret;
>   	int err = 0;
>
> -	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
> -	if (!iter)
> -		return ERR_PTR(-ENOMEM);
> +	ret = btrfs_backref_iter_init(rc->extent_root->fs_info, &iter);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
>   	path = btrfs_alloc_path();
>   	if (!path) {
>   		err = -ENOMEM;
> @@ -494,7 +494,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>
>   	/* Breadth-first search to build backref cache */
>   	do {
> -		ret = btrfs_backref_add_tree_node(cache, path, iter, node_key,
> +		ret = btrfs_backref_add_tree_node(cache, path, &iter, node_key,
>   						  cur);
>   		if (ret < 0) {
>   			err = ret;
> @@ -522,7 +522,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>   	if (handle_useless_nodes(rc, node))
>   		node = NULL;
>   out:
> -	btrfs_backref_iter_free(iter);
> +	btrfs_backref_iter_release(&iter);
>   	btrfs_free_path(path);
>   	if (err) {
>   		btrfs_backref_error_cleanup(cache, node);

Re: [PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[David Sterba]

On Sat, Sep 23, 2023 at 08:05:25AM +0930, Qu Wenruo wrote:
> On 2023/9/22 20:37, David Sterba wrote:
> > build_backref_tree() is called in a loop by relocate_tree_blocks()
> > for each relocated block. The iterator is allocated and freed repeatedly
> > while we could simply use an on-stack variable to avoid the allocation
> > and remove one more failure case. The stack grows by 48 bytes.
> >
> > This was the only use of btrfs_backref_iter_alloc() so it's changed to
> > be an initializer and btrfs_backref_iter_free() can be removed
> > completely.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/backref.c    | 26 ++++++++++----------------
> >   fs/btrfs/backref.h    | 11 ++---------
> >   fs/btrfs/relocation.c | 12 ++++++------
> >   3 files changed, 18 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index 0dc91bf654b5..691b20b47065 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -2828,26 +2828,20 @@ void free_ipath(struct inode_fs_paths *ipath)
> >   	kfree(ipath);
> >   }
> >
> > -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
> > +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> > +			    struct btrfs_backref_iter *iter)
> >   {
> > -	struct btrfs_backref_iter *ret;
> > -
> > -	ret = kzalloc(sizeof(*ret), GFP_NOFS);
> > -	if (!ret)
> > -		return NULL;
> > -
> > -	ret->path = btrfs_alloc_path();
> > -	if (!ret->path) {
> > -		kfree(ret);
> > -		return NULL;
> > -	}
> > +	memset(iter, 0, sizeof(struct btrfs_backref_iter));
> > +	iter->path = btrfs_alloc_path();
> > +	if (!iter->path)
> > +		return -ENOMEM;
> 
> We can do one step further, by integrating the btrfs_path into @iter,
> other than re-allocating it again and again.

Possible but needs to be evaluated separately, the size of path is 112
and this starts to become noticeable on the stack.

Re: [PATCH 2/8] btrfs: relocation: use enum for stages

[David Sterba]

On Sat, Sep 23, 2023 at 07:59:54AM +0930, Qu Wenruo wrote:
> 
> 
> On 2023/9/22 20:37, David Sterba wrote:
> > Add an enum type for data relocation stages.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/relocation.c | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 9ff3572a8451..3afe499f00b1 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -125,6 +125,12 @@ struct file_extent_cluster {
> >   	u64 owning_root;
> >   };
> >
> > +/* Stages of data relocation. */
> > +enum reloc_stage {
> > +	MOVE_DATA_EXTENTS,
> > +	UPDATE_DATA_PTRS
> > +};
> > +
> >   struct reloc_control {
> >   	/* block group to relocate */
> >   	struct btrfs_block_group *block_group;
> > @@ -156,16 +162,12 @@ struct reloc_control {
> >   	u64 search_start;
> >   	u64 extents_found;
> >
> > -	unsigned int stage:8;
> > +	enum reloc_stage stage;
> 
> Wouldn't this cause extra hole?

No, it's an int and after u64, the bitfields are in an int as well, so
both are aligned. The diff after the change:

@@ -7711,15 +7711,14 @@ struct reloc_control {
        u64                        reserved_bytes;       /*  1496     8 */
        u64                        search_start;         /*  1504     8 */
        u64                        extents_found;        /*  1512     8 */
-       unsigned int               stage:8;              /*  1520: 0  4 */
-       unsigned int               create_reloc_tree:1;  /*  1520: 8  4 */
-       unsigned int               merge_reloc_tree:1;   /*  1520: 9  4 */
-       unsigned int               found_file_extent:1;  /*  1520:10  4 */
+       enum reloc_stage           stage;                /*  1520     4 */
+       unsigned int               create_reloc_tree:1;  /*  1524: 0  4 */
+       unsigned int               merge_reloc_tree:1;   /*  1524: 1  4 */
+       unsigned int               found_file_extent:1;  /*  1524: 2  4 */
 
        /* size: 1528, cachelines: 24, members: 19 */
-       /* padding: 4 */
        /* paddings: 2, sum paddings: 8 */
-       /* bit_padding: 21 bits */
+       /* bit_padding: 29 bits */
        /* last cacheline: 56 bytes */
 };
---

Re: [PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[Filipe Manana]

On Fri, Sep 22, 2023 at 2:26 PM David Sterba <dsterba@suse.com> wrote:
>
> build_backref_tree() is called in a loop by relocate_tree_blocks()
> for each relocated block. The iterator is allocated and freed repeatedly
> while we could simply use an on-stack variable to avoid the allocation
> and remove one more failure case. The stack grows by 48 bytes.
>
> This was the only use of btrfs_backref_iter_alloc() so it's changed to
> be an initializer and btrfs_backref_iter_free() can be removed
> completely.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/backref.c    | 26 ++++++++++----------------
>  fs/btrfs/backref.h    | 11 ++---------
>  fs/btrfs/relocation.c | 12 ++++++------
>  3 files changed, 18 insertions(+), 31 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 0dc91bf654b5..691b20b47065 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -2828,26 +2828,20 @@ void free_ipath(struct inode_fs_paths *ipath)
>         kfree(ipath);
>  }
>
> -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
> +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> +                           struct btrfs_backref_iter *iter)
>  {
> -       struct btrfs_backref_iter *ret;
> -
> -       ret = kzalloc(sizeof(*ret), GFP_NOFS);
> -       if (!ret)
> -               return NULL;
> -
> -       ret->path = btrfs_alloc_path();
> -       if (!ret->path) {
> -               kfree(ret);
> -               return NULL;
> -       }
> +       memset(iter, 0, sizeof(struct btrfs_backref_iter));
> +       iter->path = btrfs_alloc_path();

So this is breaking misc-next, as paths are leaking here, easily
visible after "rmmod btrfs":

[ 2265.115295] =============================================================================
[ 2265.115938] BUG btrfs_path (Not tainted): Objects remaining in
btrfs_path on __kmem_cache_shutdown()
[ 2265.116615] -----------------------------------------------------------------------------

[ 2265.117614] Slab 0x00000000dbb6fd30 objects=36 used=3
fp=0x000000001768ab21
flags=0x17fffc000000800(slab|node=0|zone=2|lastcpupid=0x1ffff)
[ 2265.118423] CPU: 1 PID: 402761 Comm: rmmod Not tainted
6.6.0-rc3-btrfs-next-139+ #1
[ 2265.118440] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
[ 2265.118457] Call Trace:
[ 2265.118483]  <TASK>
[ 2265.118491]  dump_stack_lvl+0x44/0x60
[ 2265.118521]  slab_err+0xb6/0xf0
[ 2265.118547]  __kmem_cache_shutdown+0x15f/0x2f0
[ 2265.118565]  kmem_cache_destroy+0x4c/0x170
[ 2265.118588]  exit_btrfs_fs+0x24/0x40 [btrfs]
[ 2265.119121]  __x64_sys_delete_module+0x193/0x290
[ 2265.119137]  ? exit_to_user_mode_prepare+0x3d/0x170
[ 2265.119154]  do_syscall_64+0x38/0x90
[ 2265.119169]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
[ 2265.119185] RIP: 0033:0x7f55ec127997
[ 2265.119199] Code: 73 01 c3 48 8b 0d 81 94 0c 00 f7 d8 64 89 01 48
83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00
00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 51 94 0c 00 f7 d8 64 89
01 48
[ 2265.119210] RSP: 002b:00007ffe06412d98 EFLAGS: 00000206 ORIG_RAX:
00000000000000b0
[ 2265.119225] RAX: ffffffffffffffda RBX: 00005589627f26f0 RCX: 00007f55ec127997
[ 2265.119234] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 00005589627f2758
[ 2265.119241] RBP: 0000000000000000 R08: 1999999999999999 R09: 0000000000000000
[ 2265.119249] R10: 00007f55ec19aac0 R11: 0000000000000206 R12: 00007ffe06412fe0
[ 2265.119257] R13: 00007ffe064133da R14: 00005589627f22a0 R15: 00007ffe06412fe8
[ 2265.119276]  </TASK>
[ 2265.119281] Disabling lock debugging due to kernel taint
[ 2265.119289] Object 0x0000000062d6ea78 @offset=784
[ 2265.120073] Object 0x0000000042bd66e6 @offset=1904
[ 2265.120712] Object 0x00000000603962f0 @offset=2240
[ 2265.121397] =============================================================================
[ 2265.122021] BUG btrfs_path (Tainted: G    B             ): Objects
remaining in btrfs_path on __kmem_cache_shutdown()
(...)

I get thousands and thousands of these messages after running fstests
and doing "rmmod btrfs".

The problem here is the code is reusing the iterator, and every time
allocating a new path without freeing the previous one.
It could simply avoid path allocation and reuse it.

Thanks.



> +       if (!iter->path)
> +               return -ENOMEM;
>
>         /* Current backref iterator only supports iteration in commit root */
> -       ret->path->search_commit_root = 1;
> -       ret->path->skip_locking = 1;
> -       ret->fs_info = fs_info;
> +       iter->path->search_commit_root = 1;
> +       iter->path->skip_locking = 1;
> +       iter->fs_info = fs_info;
>
> -       return ret;
> +       return 0;
>  }
>
>  int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
> index 83a9a34e948e..24fabbd2a80a 100644
> --- a/fs/btrfs/backref.h
> +++ b/fs/btrfs/backref.h
> @@ -269,15 +269,8 @@ struct btrfs_backref_iter {
>         u32 end_ptr;
>  };
>
> -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
> -
> -static inline void btrfs_backref_iter_free(struct btrfs_backref_iter *iter)
> -{
> -       if (!iter)
> -               return;
> -       btrfs_free_path(iter->path);
> -       kfree(iter);
> -}
> +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> +                           struct btrfs_backref_iter *iter);
>
>  static inline struct extent_buffer *btrfs_backref_get_eb(
>                 struct btrfs_backref_iter *iter)
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index d1dcbb15baa7..6a31e73c3df7 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -464,7 +464,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>                         struct reloc_control *rc, struct btrfs_key *node_key,
>                         int level, u64 bytenr)
>  {
> -       struct btrfs_backref_iter *iter;
> +       struct btrfs_backref_iter iter;
>         struct btrfs_backref_cache *cache = &rc->backref_cache;
>         /* For searching parent of TREE_BLOCK_REF */
>         struct btrfs_path *path;
> @@ -474,9 +474,9 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>         int ret;
>         int err = 0;
>
> -       iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
> -       if (!iter)
> -               return ERR_PTR(-ENOMEM);
> +       ret = btrfs_backref_iter_init(rc->extent_root->fs_info, &iter);
> +       if (ret < 0)
> +               return ERR_PTR(ret);
>         path = btrfs_alloc_path();
>         if (!path) {
>                 err = -ENOMEM;
> @@ -494,7 +494,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>
>         /* Breadth-first search to build backref cache */
>         do {
> -               ret = btrfs_backref_add_tree_node(cache, path, iter, node_key,
> +               ret = btrfs_backref_add_tree_node(cache, path, &iter, node_key,
>                                                   cur);
>                 if (ret < 0) {
>                         err = ret;
> @@ -522,7 +522,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>         if (handle_useless_nodes(rc, node))
>                 node = NULL;
>  out:
> -       btrfs_backref_iter_free(iter);
> +       btrfs_backref_iter_release(&iter);
>         btrfs_free_path(path);
>         if (err) {
>                 btrfs_backref_error_cleanup(cache, node);
> --
> 2.41.0
>

Re: [PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[David Sterba]

On Wed, Oct 04, 2023 at 01:35:26PM +0100, Filipe Manana wrote:
> On Fri, Sep 22, 2023 at 2:26 PM David Sterba <dsterba@suse.com> wrote:
> >
> > build_backref_tree() is called in a loop by relocate_tree_blocks()
> > for each relocated block. The iterator is allocated and freed repeatedly
> > while we could simply use an on-stack variable to avoid the allocation
> > and remove one more failure case. The stack grows by 48 bytes.
> >
> > This was the only use of btrfs_backref_iter_alloc() so it's changed to
> > be an initializer and btrfs_backref_iter_free() can be removed
> > completely.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/backref.c    | 26 ++++++++++----------------
> >  fs/btrfs/backref.h    | 11 ++---------
> >  fs/btrfs/relocation.c | 12 ++++++------
> >  3 files changed, 18 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index 0dc91bf654b5..691b20b47065 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -2828,26 +2828,20 @@ void free_ipath(struct inode_fs_paths *ipath)
> >         kfree(ipath);
> >  }
> >
> > -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
> > +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> > +                           struct btrfs_backref_iter *iter)
> >  {
> > -       struct btrfs_backref_iter *ret;
> > -
> > -       ret = kzalloc(sizeof(*ret), GFP_NOFS);
> > -       if (!ret)
> > -               return NULL;
> > -
> > -       ret->path = btrfs_alloc_path();
> > -       if (!ret->path) {
> > -               kfree(ret);
> > -               return NULL;
> > -       }
> > +       memset(iter, 0, sizeof(struct btrfs_backref_iter));
> > +       iter->path = btrfs_alloc_path();
> 
> So this is breaking misc-next, as paths are leaking here, easily
> visible after "rmmod btrfs":
> 
> [ 2265.115295] =============================================================================
> [ 2265.115938] BUG btrfs_path (Not tainted): Objects remaining in
> btrfs_path on __kmem_cache_shutdown()
> [ 2265.116615] -----------------------------------------------------------------------------
> 
> [ 2265.117614] Slab 0x00000000dbb6fd30 objects=36 used=3
> fp=0x000000001768ab21
> flags=0x17fffc000000800(slab|node=0|zone=2|lastcpupid=0x1ffff)
> [ 2265.118423] CPU: 1 PID: 402761 Comm: rmmod Not tainted
> 6.6.0-rc3-btrfs-next-139+ #1
> [ 2265.118440] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
> [ 2265.118457] Call Trace:
> [ 2265.118483]  <TASK>
> [ 2265.118491]  dump_stack_lvl+0x44/0x60
> [ 2265.118521]  slab_err+0xb6/0xf0
> [ 2265.118547]  __kmem_cache_shutdown+0x15f/0x2f0
> [ 2265.118565]  kmem_cache_destroy+0x4c/0x170
> [ 2265.118588]  exit_btrfs_fs+0x24/0x40 [btrfs]
> [ 2265.119121]  __x64_sys_delete_module+0x193/0x290
> [ 2265.119137]  ? exit_to_user_mode_prepare+0x3d/0x170
> [ 2265.119154]  do_syscall_64+0x38/0x90
> [ 2265.119169]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> [ 2265.119185] RIP: 0033:0x7f55ec127997
> [ 2265.119199] Code: 73 01 c3 48 8b 0d 81 94 0c 00 f7 d8 64 89 01 48
> 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00
> 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 51 94 0c 00 f7 d8 64 89
> 01 48
> [ 2265.119210] RSP: 002b:00007ffe06412d98 EFLAGS: 00000206 ORIG_RAX:
> 00000000000000b0
> [ 2265.119225] RAX: ffffffffffffffda RBX: 00005589627f26f0 RCX: 00007f55ec127997
> [ 2265.119234] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 00005589627f2758
> [ 2265.119241] RBP: 0000000000000000 R08: 1999999999999999 R09: 0000000000000000
> [ 2265.119249] R10: 00007f55ec19aac0 R11: 0000000000000206 R12: 00007ffe06412fe0
> [ 2265.119257] R13: 00007ffe064133da R14: 00005589627f22a0 R15: 00007ffe06412fe8
> [ 2265.119276]  </TASK>
> [ 2265.119281] Disabling lock debugging due to kernel taint
> [ 2265.119289] Object 0x0000000062d6ea78 @offset=784
> [ 2265.120073] Object 0x0000000042bd66e6 @offset=1904
> [ 2265.120712] Object 0x00000000603962f0 @offset=2240
> [ 2265.121397] =============================================================================
> [ 2265.122021] BUG btrfs_path (Tainted: G    B             ): Objects
> remaining in btrfs_path on __kmem_cache_shutdown()
> (...)
> 
> I get thousands and thousands of these messages after running fstests
> and doing "rmmod btrfs".

Thanks, for catching it, I don't seem to have the rmmod test in my
setups.

> The problem here is the code is reusing the iterator, and every time
> allocating a new path without freeing the previous one.
> It could simply avoid path allocation and reuse it.

I'll remove the patch for now and revisit using this suggestion, thanks.