thr3ads.net - Btrfs devel - [PATCH 0/3] bootstrapping btrfs_find_item

If this information is useful, please help other people find it:
Share via:

Kelley Nielsen

2013-Nov-01 06:52 UTC

[PATCH 0/3] bootstrapping btrfs_find_item_interface

There are many btrfs functions that manually search the tree for an
item. They all reimplement the same mechanism and differ in the
conditions that they use to find the item. It has been proposed that a
new core interface, btrfs_find_item, be created to take the place of 
these functions, and standardize the search functionality.

This patchset takes the first steps toward the implementation of this
core interface. The first patch creates a starting point for the
interface by moving one of the search functions, __inode_item, to
ctree.c and renaming it with the core function name. The next two
patches eliminate one similar helper function each by replacing their
callers with calls to the new core function, and modifying the new core
function to ensure it fulfills the purpose of the function being
replaced.

Kelley Nielsen (3):
  Bootstrap generic btrfs_find_item interface
  btrfs_find_item expanded to include find_root_ref funcionality
  btrfs_find_item expanded to include find_orphan_item functionality

 fs/btrfs/backref.c   | 40 ++++----------------------------------
 fs/btrfs/ctree.c     | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ctree.h     |  2 ++
 fs/btrfs/disk-io.c   |  3 ++-
 fs/btrfs/inode.c     |  6 +++---
 fs/btrfs/orphan.c    | 20 -------------------
 fs/btrfs/root-tree.c | 15 --------------
 fs/btrfs/tree-log.c  |  3 ++-
 8 files changed, 68 insertions(+), 76 deletions(-)

-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Kelley Nielsen

2013-Nov-01 06:53 UTC

head link

[PATCH 1/3] Bootstrap generic btrfs_find_item interface

There are many btrfs functions that manually search the tree for an
item. They all reimplement the same mechanism and differ in the
conditions that they use to find the item.__inode_info() is one such
example. It has been proposed that a new interface be created to
take the place of these functions.

This patch is the first step to creating the interface. A new function,
btrfs_find_item, has been added to ctree.c and prototyped in ctree.h.
It is identical to __inode_info, except that the order of the parameters
has been rearranged to more closely those of similar functions elsewhere
in the code (now, root and path come first, then the objectid, offset
and type, and the key to be filled in last). __inode_info''s callers
have
been set to call this new function instead, and __inode_info itself has
been removed.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Zach Brown <zab@redhat.com>
---
 fs/btrfs/backref.c | 40 ++++------------------------------------
 fs/btrfs/ctree.c   | 37 +++++++++++++++++++++++++++++++++++++
 fs/btrfs/ctree.h   |  2 ++
 3 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 3775947..595bd1f 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1107,38 +1107,6 @@ int btrfs_find_all_roots(struct btrfs_trans_handle
*trans,
 	return 0;
 }
 
-
-static int __inode_info(u64 inum, u64 ioff, u8 key_type,
-			struct btrfs_root *fs_root, struct btrfs_path *path,
-			struct btrfs_key *found_key)
-{
-	int ret;
-	struct btrfs_key key;
-	struct extent_buffer *eb;
-
-	key.type = key_type;
-	key.objectid = inum;
-	key.offset = ioff;
-
-	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
-	if (ret < 0)
-		return ret;
-
-	eb = path->nodes[0];
-	if (ret && path->slots[0] >= btrfs_header_nritems(eb)) {
-		ret = btrfs_next_leaf(fs_root, path);
-		if (ret)
-			return ret;
-		eb = path->nodes[0];
-	}
-
-	btrfs_item_key_to_cpu(eb, found_key, path->slots[0]);
-	if (found_key->type != key.type || found_key->objectid != key.objectid)
-		return 1;
-
-	return 0;
-}
-
 /*
  * this makes the path point to (inum INODE_ITEM ioff)
  */
@@ -1146,16 +1114,16 @@ int inode_item_info(u64 inum, u64 ioff, struct
btrfs_root *fs_root,
 			struct btrfs_path *path)
 {
 	struct btrfs_key key;
-	return __inode_info(inum, ioff, BTRFS_INODE_ITEM_KEY, fs_root, path,
-				&key);
+	return btrfs_find_item(fs_root, path, inum, ioff,
+			BTRFS_INODE_ITEM_KEY, &key);
 }
 
 static int inode_ref_info(u64 inum, u64 ioff, struct btrfs_root *fs_root,
 				struct btrfs_path *path,
 				struct btrfs_key *found_key)
 {
-	return __inode_info(inum, ioff, BTRFS_INODE_REF_KEY, fs_root, path,
-				found_key);
+	return btrfs_find_item(fs_root, path, inum, ioff,
+			BTRFS_INODE_REF_KEY, found_key);
 }
 
 int btrfs_find_one_extref(struct btrfs_root *root, u64 inode_objectid,
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 316136b..3828352 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2462,6 +2462,43 @@ static int key_search(struct extent_buffer *b, struct
btrfs_key *key,
 	return 0;
 }
 
+/* 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.
+*/
+int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
+		u64 inum, u64 ioff, u8 key_type,
+		struct btrfs_key *found_key)
+{
+	int ret;
+	struct btrfs_key key;
+	struct extent_buffer *eb;
+
+	key.type = key_type;
+	key.objectid = inum;
+	key.offset = ioff;
+
+	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
+	if (ret < 0)
+		return ret;
+
+	eb = path->nodes[0];
+	if (ret && path->slots[0] >= btrfs_header_nritems(eb)) {
+		ret = btrfs_next_leaf(fs_root, path);
+		if (ret)
+			return ret;
+		eb = path->nodes[0];
+	}
+
+	btrfs_item_key_to_cpu(eb, found_key, path->slots[0]);
+	if (found_key->type != key.type ||
+			found_key->objectid != key.objectid)
+		return 1;
+
+	return 0;
+}
+
 /*
  * look for key in the tree.  path is filled in with nodes along the way
  * if key is found, we return zero and you can find the item in the leaf
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 941019d..aa9993c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3357,6 +3357,8 @@ int btrfs_duplicate_item(struct btrfs_trans_handle *trans,
 			 struct btrfs_root *root,
 			 struct btrfs_path *path,
 			 struct btrfs_key *new_key);
+int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
+		u64 inum, u64 ioff, u8 key_type, struct btrfs_key *found_key);
 int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root
 		      *root, struct btrfs_key *key, struct btrfs_path *p, int
 		      ins_len, int cow);
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Josh Triplett

2013-Nov-02 17:02 UTC

head link

Re: [OPW kernel] [PATCH 0/3] bootstrapping btrfs_find_item_interface

On Thu, Oct 31, 2013 at 11:52:45PM -0700, Kelley Nielsen
wrote:> There are many btrfs functions that manually search the tree for an
> item. They all reimplement the same mechanism and differ in the
> conditions that they use to find the item. It has been proposed that a
> new core interface, btrfs_find_item, be created to take the place of 
s/It has been proposed that ... be created/Zach Brown proposed creating .../
, judging by the Suggested-by in the patches.
> these functions, and standardize the search functionality.
> 
> This patchset takes the first steps toward the implementation of this
> core interface. The first patch creates a starting point for the
> interface by moving one of the search functions, __inode_item, to
> ctree.c and renaming it with the core function name. The next two
> patches eliminate one similar helper function each by replacing their
> callers with calls to the new core function, and modifying the new core
> function to ensure it fulfills the purpose of the function being
> replaced.
> 
> Kelley Nielsen (3):
>   Bootstrap generic btrfs_find_item interface
>   btrfs_find_item expanded to include find_root_ref funcionality
>   btrfs_find_item expanded to include find_orphan_item functionality
> 
>  fs/btrfs/backref.c   | 40 ++++----------------------------------
>  fs/btrfs/ctree.c     | 55
++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/ctree.h     |  2 ++
>  fs/btrfs/disk-io.c   |  3 ++-
>  fs/btrfs/inode.c     |  6 +++---
>  fs/btrfs/orphan.c    | 20 -------------------
>  fs/btrfs/root-tree.c | 15 --------------
>  fs/btrfs/tree-log.c  |  3 ++-
>  8 files changed, 68 insertions(+), 76 deletions(-)
> 
> -- 
> 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.--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Josh Triplett

2013-Nov-02 17:05 UTC

head link

Re: [OPW kernel] [PATCH 1/3] Bootstrap generic btrfs_find_item interface

On Thu, Oct 31, 2013 at 11:53:41PM -0700, Kelley Nielsen
wrote:> There are many btrfs functions that manually search the tree for an
> item. They all reimplement the same mechanism and differ in the
> conditions that they use to find the item.__inode_info() is one such
You need a space after the ''.'' here; otherwise, this looks
like a call
to "item.__inode_info()".
> example. It has been proposed that a new interface be created to
> take the place of these functions.
Same suggestion as in the cover letter: "It has been proposed that a new
interface be created" -> "Zach Brown proposed creating a new
interface"
> This patch is the first step to creating the interface. A new function,
> btrfs_find_item, has been added to ctree.c and prototyped in ctree.h.
> It is identical to __inode_info, except that the order of the parameters
> has been rearranged to more closely those of similar functions elsewhere
> in the code (now, root and path come first, then the objectid, offset
> and type, and the key to be filled in last). __inode_info''s
callers have
> been set to call this new function instead, and __inode_info itself has
> been removed.
> 
> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
> Suggested-by: Zach Brown <zab@redhat.com>
With the fix above,
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  fs/btrfs/backref.c | 40 ++++------------------------------------
>  fs/btrfs/ctree.c   | 37 +++++++++++++++++++++++++++++++++++++
>  fs/btrfs/ctree.h   |  2 ++
>  3 files changed, 43 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 3775947..595bd1f 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1107,38 +1107,6 @@ int btrfs_find_all_roots(struct btrfs_trans_handle
*trans,
>  	return 0;
>  }
>  
> -
> -static int __inode_info(u64 inum, u64 ioff, u8 key_type,
> -			struct btrfs_root *fs_root, struct btrfs_path *path,
> -			struct btrfs_key *found_key)
> -{
> -	int ret;
> -	struct btrfs_key key;
> -	struct extent_buffer *eb;
> -
> -	key.type = key_type;
> -	key.objectid = inum;
> -	key.offset = ioff;
> -
> -	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
> -	if (ret < 0)
> -		return ret;
> -
> -	eb = path->nodes[0];
> -	if (ret && path->slots[0] >= btrfs_header_nritems(eb)) {
> -		ret = btrfs_next_leaf(fs_root, path);
> -		if (ret)
> -			return ret;
> -		eb = path->nodes[0];
> -	}
> -
> -	btrfs_item_key_to_cpu(eb, found_key, path->slots[0]);
> -	if (found_key->type != key.type || found_key->objectid !=
key.objectid)
> -		return 1;
> -
> -	return 0;
> -}
> -
>  /*
>   * this makes the path point to (inum INODE_ITEM ioff)
>   */
> @@ -1146,16 +1114,16 @@ int inode_item_info(u64 inum, u64 ioff, struct
btrfs_root *fs_root,
>  			struct btrfs_path *path)
>  {
>  	struct btrfs_key key;
> -	return __inode_info(inum, ioff, BTRFS_INODE_ITEM_KEY, fs_root, path,
> -				&key);
> +	return btrfs_find_item(fs_root, path, inum, ioff,
> +			BTRFS_INODE_ITEM_KEY, &key);
>  }
>  
>  static int inode_ref_info(u64 inum, u64 ioff, struct btrfs_root *fs_root,
>  				struct btrfs_path *path,
>  				struct btrfs_key *found_key)
>  {
> -	return __inode_info(inum, ioff, BTRFS_INODE_REF_KEY, fs_root, path,
> -				found_key);
> +	return btrfs_find_item(fs_root, path, inum, ioff,
> +			BTRFS_INODE_REF_KEY, found_key);
>  }
>  
>  int btrfs_find_one_extref(struct btrfs_root *root, u64 inode_objectid,
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 316136b..3828352 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2462,6 +2462,43 @@ static int key_search(struct extent_buffer *b,
struct btrfs_key *key,
>  	return 0;
>  }
>  
> +/* 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.
> +*/
Very nice work; even your comments are bisectable. :)
> +int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
> +		u64 inum, u64 ioff, u8 key_type,
> +		struct btrfs_key *found_key)
> +{
> +	int ret;
> +	struct btrfs_key key;
> +	struct extent_buffer *eb;
> +
> +	key.type = key_type;
> +	key.objectid = inum;
> +	key.offset = ioff;
> +
> +	ret = btrfs_search_slot(NULL, fs_root, &key, path, 0, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	eb = path->nodes[0];
> +	if (ret && path->slots[0] >= btrfs_header_nritems(eb)) {
> +		ret = btrfs_next_leaf(fs_root, path);
> +		if (ret)
> +			return ret;
> +		eb = path->nodes[0];
> +	}
> +
> +	btrfs_item_key_to_cpu(eb, found_key, path->slots[0]);
> +	if (found_key->type != key.type ||
> +			found_key->objectid != key.objectid)
> +		return 1;
> +
> +	return 0;
> +}
> +
>  /*
>   * look for key in the tree.  path is filled in with nodes along the way
>   * if key is found, we return zero and you can find the item in the leaf
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 941019d..aa9993c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3357,6 +3357,8 @@ int btrfs_duplicate_item(struct btrfs_trans_handle
*trans,
>  			 struct btrfs_root *root,
>  			 struct btrfs_path *path,
>  			 struct btrfs_key *new_key);
> +int btrfs_find_item(struct btrfs_root *fs_root, struct btrfs_path *path,
> +		u64 inum, u64 ioff, u8 key_type, struct btrfs_key *found_key);
>  int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root
>  		      *root, struct btrfs_key *key, struct btrfs_path *p, int
>  		      ins_len, int cow);
> -- 
> 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.--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Btrfs devel - Nov 2013 - [PATCH 0/3] bootstrapping btrfs_find_item_interface

[PATCH 0/3] bootstrapping btrfs_find_item_interface

[PATCH 1/3] Bootstrap generic btrfs_find_item interface

Re: [OPW kernel] [PATCH 0/3] bootstrapping btrfs_find_item_interface

Re: [OPW kernel] [PATCH 1/3] Bootstrap generic btrfs_find_item interface