[PATCH 2/3] btrfs_find_item expanded to include find_root_ref functionality

[PATCH 2/3] btrfs_find_item expanded to include find_root_ref functionality

[Kelley Nielsen]

This patch is the second step in bootstrapping the btrfs_find_item
interface. The function btrfs_find_root_ref is similar to the former
__inode_info; it accepts four of its parameters, and duplicates the
first half of its functionality.

The one former call to btrfs_find_root_ref has been replaced with a
call to btrfs_find_item, along with the defined key type that was used
internally by btrfs_find_root ref, and a null found key. In
btrfs_find_item, a test for the null key has been added at the place
where the functionality of btrfs_find_root_ref ends; btrfs_find_item
then returns if the test passes. Finally, btrfs_find_root_ref has been
removed.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Zach Brown <zab@redhat.com>
---
 fs/btrfs/ctree.c     | 10 ++++++++--
 fs/btrfs/inode.c     |  6 +++---
 fs/btrfs/root-tree.c | 15 ---------------
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 3828352..498b19d 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2465,7 +2465,13 @@ static int key_search(struct extent_buffer *b, struct btrfs_key *key,
 /* Proposed generic search function, meant to take the place of the
 * various small search helper functions throughout the code and standardize
 * the search interface. Right now, it only replaces the former __inode_info
-* in backref.c.
+* in backref.c, and the former btrfs_find_root_ref in root-tree.c.
+*
+* If a null key is passed, it returns immediately after running
+* btrfs_search_slot, leaving the path filled as it is and passing its
+* return value upward. If a real key is passed, it will set the caller's
+* path to point to the first item in the tree after its specified
+* objectid, type, and offset for which objectid and type match the input.
 */
 int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
 		u64 inum, u64 ioff, u8 key_type,
@@ -2480,7 +2486,7 @@ int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
 	key.offset = ioff;
 
 	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
-	if (ret < 0)
+	if ((ret < 0) || (found_key == NULL))
 		return ret;
 
 	eb = path->nodes[0];
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3911fca..c07afdf 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4672,9 +4672,9 @@ static int fixup_tree_root_location(struct btrfs_root *root,
 	}
 
 	err = -ENOENT;
-	ret = btrfs_find_root_ref(root->fs_info->tree_root, path,
-				  BTRFS_I(dir)->root->root_key.objectid,
-				  location->objectid);
+	ret = btrfs_find_item(root->fs_info->tree_root, path,
+				BTRFS_I(dir)->root->root_key.objectid,
+				location->objectid, BTRFS_ROOT_REF_KEY, NULL);
 	if (ret) {
 		if (ret < 0)
 			err = ret;
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index ec71ea4..fcc10eb 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -400,21 +400,6 @@ out:
 	return err;
 }
 
-int btrfs_find_root_ref(struct btrfs_root *tree_root,
-		   struct btrfs_path *path,
-		   u64 root_id, u64 ref_id)
-{
-	struct btrfs_key key;
-	int ret;
-
-	key.objectid = root_id;
-	key.type = BTRFS_ROOT_REF_KEY;
-	key.offset = ref_id;
-
-	ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
-	return ret;
-}
-
 /*
  * add a btrfs_root_ref item.  type is either BTRFS_ROOT_REF_KEY
  * or BTRFS_ROOT_BACKREF_KEY.
-- 
1.8.1.2

Re: [OPW kernel] [PATCH 2/3] btrfs_find_item expanded to include find_root_ref functionality

[Josh Triplett]

On Fri, Nov 01, 2013 at 12:00:10AM -0700, Kelley Nielsen wrote:
> This patch is the second step in bootstrapping the btrfs_find_item
> interface. The function btrfs_find_root_ref is similar to the former
> __inode_info; it accepts four of its parameters, and duplicates the
> first half of its functionality.
> 
> The one former call to btrfs_find_root_ref has been replaced with a
> call to btrfs_find_item, along with the defined key type that was used
> internally by btrfs_find_root ref, and a null found key. In
> btrfs_find_item, a test for the null key has been added at the place
> where the functionality of btrfs_find_root_ref ends; btrfs_find_item
> then returns if the test passes. Finally, btrfs_find_root_ref has been
> removed.

Write your commit messages in imperative form, not passive voice:

Replace the one former call to btrfs_find_root_ref with a call to
btrfs_find_item, along with the defined key type used internally by
btrfs_find_root ref, and a null found key. In btrfs_find_item, add a
test for the null key at the place where the functionality of
btrfs_find_root_ref ends; btrfs_find_item then returns if the test
passes. Finally, remove btrfs_find_root_ref.

> 
> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
> Suggested-by: Zach Brown <zab@redhat.com>

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> ---
>  fs/btrfs/ctree.c     | 10 ++++++++--
>  fs/btrfs/inode.c     |  6 +++---
>  fs/btrfs/root-tree.c | 15 ---------------
>  3 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 3828352..498b19d 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2465,7 +2465,13 @@ static int key_search(struct extent_buffer *b, struct btrfs_key *key,
>  /* Proposed generic search function, meant to take the place of the
>  * various small search helper functions throughout the code and standardize
>  * the search interface. Right now, it only replaces the former __inode_info
> -* in backref.c.
> +* in backref.c, and the former btrfs_find_root_ref in root-tree.c.
> +*
> +* If a null key is passed, it returns immediately after running
> +* btrfs_search_slot, leaving the path filled as it is and passing its
> +* return value upward. If a real key is passed, it will set the caller's
> +* path to point to the first item in the tree after its specified
> +* objectid, type, and offset for which objectid and type match the input.
>  */
>  int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
>  		u64 inum, u64 ioff, u8 key_type,
> @@ -2480,7 +2486,7 @@ int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
>  	key.offset = ioff;
>  
>  	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
> -	if (ret < 0)
> +	if ((ret < 0) || (found_key == NULL))
>  		return ret;
>  
>  	eb = path->nodes[0];
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3911fca..c07afdf 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4672,9 +4672,9 @@ static int fixup_tree_root_location(struct btrfs_root *root,
>  	}
>  
>  	err = -ENOENT;
> -	ret = btrfs_find_root_ref(root->fs_info->tree_root, path,
> -				  BTRFS_I(dir)->root->root_key.objectid,
> -				  location->objectid);
> +	ret = btrfs_find_item(root->fs_info->tree_root, path,
> +				BTRFS_I(dir)->root->root_key.objectid,
> +				location->objectid, BTRFS_ROOT_REF_KEY, NULL);
>  	if (ret) {
>  		if (ret < 0)
>  			err = ret;
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index ec71ea4..fcc10eb 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -400,21 +400,6 @@ out:
>  	return err;
>  }
>  
> -int btrfs_find_root_ref(struct btrfs_root *tree_root,
> -		   struct btrfs_path *path,
> -		   u64 root_id, u64 ref_id)
> -{
> -	struct btrfs_key key;
> -	int ret;
> -
> -	key.objectid = root_id;
> -	key.type = BTRFS_ROOT_REF_KEY;
> -	key.offset = ref_id;
> -
> -	ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
> -	return ret;
> -}
> -
>  /*
>   * add a btrfs_root_ref item.  type is either BTRFS_ROOT_REF_KEY
>   * or BTRFS_ROOT_BACKREF_KEY.
> -- 
> 1.8.1.2
> 
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

[PATCH 3/3] btrfs_find_item expanded to include find_orphan_item functionality

[Kelley Nielsen]

This is the third step in bootstrapping the btrfs_find_item interface.
The function find_orphan_item, in orphan.c, is similar to the two
functions already replaced by the new interface. It uses two parameters,
which are already present in the interface, and is nearly identical to
the function brought in in the previous patch.

The two calls to find_orphan_item have been replaced by calls to
btrfs_find_item, with the defined object id and type that was used
internally by find_orphan_item, a null path, and a null key. A test for
a null path has been added to btrfs_find_item, and if it passes, a path
is allocated and freed. Finally, find_orphan_item has been removed.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Zach Brown <zab@redhat.com>
---
 fs/btrfs/ctree.c    | 18 +++++++++++++++---
 fs/btrfs/disk-io.c  |  3 ++-
 fs/btrfs/orphan.c   | 20 --------------------
 fs/btrfs/tree-log.c |  3 ++-
 4 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 498b19d..83a5418 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2465,7 +2465,8 @@ static int key_search(struct extent_buffer *b, struct btrfs_key *key,
 /* Proposed generic search function, meant to take the place of the
 * various small search helper functions throughout the code and standardize
 * the search interface. Right now, it only replaces the former __inode_info
-* in backref.c, and the former btrfs_find_root_ref in root-tree.c.
+* in backref.c, the former btrfs_find_root_ref in root-tree.c, and the
+* former btrfs_find_orphan_item in orphan.c.
 *
 * If a null key is passed, it returns immediately after running
 * btrfs_search_slot, leaving the path filled as it is and passing its
@@ -2473,21 +2474,32 @@ static int key_search(struct extent_buffer *b, struct btrfs_key *key,
 * path to point to the first item in the tree after its specified
 * objectid, type, and offset for which objectid and type match the input.
 */
-int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
+int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *found_path,
 		u64 inum, u64 ioff, u8 key_type,
 		struct btrfs_key *found_key)
 {
 	int ret;
 	struct btrfs_key key;
 	struct extent_buffer *eb;
+	struct btrfs_path *path;
 
 	key.type = key_type;
 	key.objectid = inum;
 	key.offset = ioff;
 
+	if (found_path == NULL) {
+		path = btrfs_alloc_path();
+		if (!path)
+			return -ENOMEM;
+	} else
+		path = found_path;
+
 	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
-	if ((ret < 0) || (found_key == NULL))
+	if ((ret < 0) || (found_key == NULL)) {
+		if (path != found_path)
+			btrfs_free_path(path);
 		return ret;
+	}
 
 	eb = path->nodes[0];
 	if (ret && path->slots[0] >= btrfs_header_nritems(eb)) {
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4c4ed0b..bce90c9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1616,7 +1616,8 @@ again:
 	if (ret)
 		goto fail;
 
-	ret = btrfs_find_orphan_item(fs_info->tree_root, location->objectid);
+	ret = btrfs_find_item(fs_info->tree_root, NULL, BTRFS_ORPHAN_OBJECTID,
+			location->objectid, BTRFS_ORPHAN_ITEM_KEY, NULL);
 	if (ret < 0)
 		goto fail;
 	if (ret == 0)
diff --git a/fs/btrfs/orphan.c b/fs/btrfs/orphan.c
index 24cad16..65793ed 100644
--- a/fs/btrfs/orphan.c
+++ b/fs/btrfs/orphan.c
@@ -69,23 +69,3 @@ out:
 	btrfs_free_path(path);
 	return ret;
 }
-
-int btrfs_find_orphan_item(struct btrfs_root *root, u64 offset)
-{
-	struct btrfs_path *path;
-	struct btrfs_key key;
-	int ret;
-
-	key.objectid = BTRFS_ORPHAN_OBJECTID;
-	key.type = BTRFS_ORPHAN_ITEM_KEY;
-	key.offset = offset;
-
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-
-	btrfs_free_path(path);
-	return ret;
-}
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index a2c7b04..9972e2a 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1238,7 +1238,8 @@ static int insert_orphan_item(struct btrfs_trans_handle *trans,
 			      struct btrfs_root *root, u64 offset)
 {
 	int ret;
-	ret = btrfs_find_orphan_item(root, offset);
+	ret = btrfs_find_item(root, NULL, BTRFS_ORPHAN_OBJECTID,
+			offset, BTRFS_ORPHAN_ITEM_KEY, NULL);
 	if (ret > 0)
 		ret = btrfs_insert_orphan_item(trans, root, offset);
 	return ret;
-- 
1.8.1.2

Re: [OPW kernel] [PATCH 3/3] btrfs_find_item expanded to include find_orphan_item functionality

[Josh Triplett]

On Fri, Nov 01, 2013 at 12:00:32AM -0700, Kelley Nielsen wrote:
> This is the third step in bootstrapping the btrfs_find_item interface.
> The function find_orphan_item, in orphan.c, is similar to the two
> functions already replaced by the new interface. It uses two parameters,
> which are already present in the interface, and is nearly identical to
> the function brought in in the previous patch.
> 
> The two calls to find_orphan_item have been replaced by calls to
> btrfs_find_item, with the defined object id and type that was used
> internally by find_orphan_item, a null path, and a null key. A test for
> a null path has been added to btrfs_find_item, and if it passes, a path
> is allocated and freed. Finally, find_orphan_item has been removed.

Again, use imperative voice, not passive voice.  Your commit message
should instruct the kernel to improve. :)

Replace the two calls to find_orphan_item ...

> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
> Suggested-by: Zach Brown <zab@redhat.com>

One issue below.

>  fs/btrfs/ctree.c    | 18 +++++++++++++++---
>  fs/btrfs/disk-io.c  |  3 ++-
>  fs/btrfs/orphan.c   | 20 --------------------
>  fs/btrfs/tree-log.c |  3 ++-
>  4 files changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 498b19d..83a5418 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2465,7 +2465,8 @@ static int key_search(struct extent_buffer *b, struct btrfs_key *key,
>  /* Proposed generic search function, meant to take the place of the
>  * various small search helper functions throughout the code and standardize
>  * the search interface. Right now, it only replaces the former __inode_info
> -* in backref.c, and the former btrfs_find_root_ref in root-tree.c.
> +* in backref.c, the former btrfs_find_root_ref in root-tree.c, and the
> +* former btrfs_find_orphan_item in orphan.c.

Hmmm.  I'd expected this comment to have disappeared by the end of the
patch series, once all the cases were handled.

- Josh Triplett