diff mbox series

[4/7] btrfs: root-tree: Use btrfs_find_item in btrfs_find_orphan_roots

Message ID 20210804184854.10696-5-mpdesouza@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: Use btrfs_find_item whenever possible | expand

Commit Message

Marcos Paulo de Souza Aug. 4, 2021, 6:48 p.m. UTC
Prefer btrfs_find_item instead of btrfs_search_slot, since it calls
btrfs_next_leaf if needed and checks if the item found has the same
objectid and type passed in the search key.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/root-tree.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

Comments

Qu Wenruo Aug. 5, 2021, 6:42 a.m. UTC | #1
On 2021/8/5 上午2:48, Marcos Paulo de Souza wrote:
> Prefer btrfs_find_item instead of btrfs_search_slot, since it calls
> btrfs_next_leaf if needed and checks if the item found has the same
> objectid and type passed in the search key.

This is fine, but IMHO btrfs_find_item() would be preferred to be
utilized when we know exactly the search key.

For orphan iteration, I guess we would really prefer something like
"btrfs_iterate_key_range()" or things like that?

Yeah, using btrfs_find_item() here is no harm, but it's also not that
typical to me.

Or you're going to introduce specific helper for this usage later?
Then I prefer to modify this call site when the iteration interface is
introduced.

Thanks,
Qu
>
> No functional changes.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>   fs/btrfs/root-tree.c | 32 ++++++++------------------------
>   1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 702dc5441f03..4bb0ad192a2f 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -207,10 +207,10 @@ int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>   int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
>   {
>   	struct btrfs_root *tree_root = fs_info->tree_root;
> -	struct extent_buffer *leaf;
>   	struct btrfs_path *path;
>   	struct btrfs_key key;
>   	struct btrfs_root *root;
> +	u64 offset = 0;
>   	int err = 0;
>   	int ret;
>
> @@ -218,38 +218,22 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
>   	if (!path)
>   		return -ENOMEM;
>
> -	key.objectid = BTRFS_ORPHAN_OBJECTID;
> -	key.type = BTRFS_ORPHAN_ITEM_KEY;
> -	key.offset = 0;
> -
>   	while (1) {
>   		u64 root_objectid;
>
> -		ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
> +		ret = btrfs_find_item(tree_root, path, BTRFS_ORPHAN_OBJECTID,
> +				BTRFS_ORPHAN_ITEM_KEY, offset, &key);
> +
> +		btrfs_release_path(path);
>   		if (ret < 0) {
>   			err = ret;
>   			break;
> -		}
> -
> -		leaf = path->nodes[0];
> -		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> -			ret = btrfs_next_leaf(tree_root, path);
> -			if (ret < 0)
> -				err = ret;
> -			if (ret != 0)
> -				break;
> -			leaf = path->nodes[0];
> -		}
> -
> -		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
> -		btrfs_release_path(path);
> -
> -		if (key.objectid != BTRFS_ORPHAN_OBJECTID ||
> -		    key.type != BTRFS_ORPHAN_ITEM_KEY)
> +		} else if (ret > 0) {
>   			break;
> +		}
>
>   		root_objectid = key.offset;
> -		key.offset++;
> +		offset++;
>
>   		root = btrfs_get_fs_root(fs_info, root_objectid, false);
>   		err = PTR_ERR_OR_ZERO(root);
>
diff mbox series

Patch

diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 702dc5441f03..4bb0ad192a2f 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -207,10 +207,10 @@  int btrfs_insert_root(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_root *tree_root = fs_info->tree_root;
-	struct extent_buffer *leaf;
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct btrfs_root *root;
+	u64 offset = 0;
 	int err = 0;
 	int ret;
 
@@ -218,38 +218,22 @@  int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
 	if (!path)
 		return -ENOMEM;
 
-	key.objectid = BTRFS_ORPHAN_OBJECTID;
-	key.type = BTRFS_ORPHAN_ITEM_KEY;
-	key.offset = 0;
-
 	while (1) {
 		u64 root_objectid;
 
-		ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
+		ret = btrfs_find_item(tree_root, path, BTRFS_ORPHAN_OBJECTID,
+				BTRFS_ORPHAN_ITEM_KEY, offset, &key);
+
+		btrfs_release_path(path);
 		if (ret < 0) {
 			err = ret;
 			break;
-		}
-
-		leaf = path->nodes[0];
-		if (path->slots[0] >= btrfs_header_nritems(leaf)) {
-			ret = btrfs_next_leaf(tree_root, path);
-			if (ret < 0)
-				err = ret;
-			if (ret != 0)
-				break;
-			leaf = path->nodes[0];
-		}
-
-		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
-		btrfs_release_path(path);
-
-		if (key.objectid != BTRFS_ORPHAN_OBJECTID ||
-		    key.type != BTRFS_ORPHAN_ITEM_KEY)
+		} else if (ret > 0) {
 			break;
+		}
 
 		root_objectid = key.offset;
-		key.offset++;
+		offset++;
 
 		root = btrfs_get_fs_root(fs_info, root_objectid, false);
 		err = PTR_ERR_OR_ZERO(root);