[PATCH 0/7] btrfs: Create macro to iterate slots

[PATCH 0/7] btrfs: Create macro to iterate slots

[Marcos Paulo de Souza]

There is a common pattern when search for a key in btrfs:

 * Call btrfs_search_slot
 * Endless loop
	 * If the found slot is bigger than the current items in the leaf, check the
	   next one
	 * If still not found in the next leaf, return 1
	 * Do something with the code
	 * Increment current slot, and continue

This pattern can be improved by creating an iterator macro, similar to
those for_each_X already existing in the linux kernel. using this
approach means to reduce significantly boilerplate code, along making it
easier to newcomers to understand how to code works.

This patchset survived a complete fstest run.

Changes from RFC:
 * Add documentation to btrfs_for_each_slot macro and btrfs_valid_slot function
   (David)
 * Add documentation about the ret variable used as a macro argument (David)
 * Match function argument from prototype and implementation (David)
 * Changed ({ }) block to only () in btrfs_for_each_slot macro (David)
 * Add more patches to show the code being reduced by using this approach
   (Nikolay)

Marcos Paulo de Souza (7):
  fs: btrfs: Introduce btrfs_for_each_slot
  btrfs: block-group: use btrfs_for_each_slot in find_first_block_group
  btrfs: dev-replace: Use btrfs_for_each_slot in
    mark_block_group_to_copy
  btrfs: dir-item: use btrfs_for_each_slot in
    btrfs_search_dir_index_item
  btrfs: inode: use btrfs_for_each_slot in btrfs_read_readdir
  btrfs: send: Use btrfs_for_each_slot macro
  btrfs: volumes: use btrfs_for_each_slot in btrfs_read_chunk_tree

 fs/btrfs/block-group.c |  33 +-----
 fs/btrfs/ctree.c       |  28 ++++++
 fs/btrfs/ctree.h       |  25 +++++
 fs/btrfs/dev-replace.c |  51 ++--------
 fs/btrfs/dir-item.c    |  27 +----
 fs/btrfs/inode.c       |  46 ++++-----
 fs/btrfs/send.c        | 222 +++++++++++------------------------------
 fs/btrfs/volumes.c     |  23 ++---
 fs/btrfs/xattr.c       |  40 +++-----
 9 files changed, 169 insertions(+), 326 deletions(-)

-- 
2.31.1

[PATCH 1/7] fs: btrfs: Introduce btrfs_for_each_slot

[Marcos Paulo de Souza]

There is a common pattern when search for a key in btrfs:

 * Call btrfs_search_slot
 * Endless loop
	 * If the found slot is bigger than the current items in the leaf, check the
	   next one
	 * If still not found in the next leaf, return 1
	 * Do something with the code
	 * Increment current slot, and continue

This pattern can be improved by creating an iterator macro, similar to
those for_each_X already existing in the linux kernel. Using this
approach means to reduce significantly boilerplate code, along making it
easier to newcomers to understand how to code works.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/ctree.c | 28 ++++++++++++++++++++++++++++
 fs/btrfs/ctree.h | 25 +++++++++++++++++++++++++
 fs/btrfs/xattr.c | 40 ++++++++++++----------------------------
 3 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 84627cbd5b5b..b1aa6e3987d0 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2122,6 +2122,34 @@ int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
 	return ret;
 }
 
+/* Search for a valid slot for the given path.
+ * @root: The root node of the tree.
+ * @key: Will contain a valid item if found.
+ * @path: The start point to validate the slot.
+ *
+ * Return 0 if the item is valid, 1 if not found and < 0 if error.
+ */
+int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
+		     struct btrfs_path *path)
+{
+	while (1) {
+		int ret;
+		const int slot = path->slots[0];
+		const struct extent_buffer *leaf = path->nodes[0];
+
+		if (slot >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(root, path);
+			if (ret)
+				return ret;
+			continue;
+		}
+		btrfs_item_key_to_cpu(leaf, key, slot);
+		break;
+	}
+
+	return 0;
+}
+
 /*
  * adjust the pointers going up the tree, starting at level
  * making sure the right key of each node is points to 'key'.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index f07c82fafa04..1e3c4a7741ca 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2912,6 +2912,31 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
 int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
 			   struct btrfs_path *path);
 
+int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
+		     struct btrfs_path *path);
+
+/* Search in @root for a given @key, and store the slot found in @found_key.
+ * @root: The root node of the tree.
+ * @key: The key we are looking for.
+ * @found_key: Will hold the found item.
+ * @path: Holds the current slot/leaf.
+ * @iter_ret: Contains the returned value from btrfs_search_slot and
+ *            btrfs_valid_slot, whatever is executed later.
+ *
+ * The iter_ret is an output variable that will contain the result of the
+ * btrfs_search_slot if it returns an error, or the value returned from
+ * btrfs_valid_slot otherwise. The return value can be 0 if the something was
+ * found, 1 if there weren't bigger leaves, and <0 if error.
+ */
+#define btrfs_for_each_slot(root, key, found_key, path, iter_ret)		\
+	for (iter_ret = btrfs_search_slot(NULL, root, key, path, 0, 0);		\
+		(								\
+			iter_ret >= 0 &&					\
+			(iter_ret = btrfs_valid_slot(root, found_key, path)) == 0 \
+		);								  \
+		path->slots[0]++						  \
+	)
+
 static inline int btrfs_next_old_item(struct btrfs_root *root,
 				      struct btrfs_path *p, u64 time_seq)
 {
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 8a4514283a4b..f85febba1891 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -274,10 +274,12 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name,
 ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
 {
 	struct btrfs_key key;
+	struct btrfs_key found_key;
 	struct inode *inode = d_inode(dentry);
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct btrfs_path *path;
 	int ret = 0;
+	int iter_ret = 0;
 	size_t total_size = 0, size_left = size;
 
 	/*
@@ -295,44 +297,22 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	path->reada = READA_FORWARD;
 
 	/* search for our xattrs */
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto err;
-
-	while (1) {
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		struct extent_buffer *leaf;
 		int slot;
 		struct btrfs_dir_item *di;
-		struct btrfs_key found_key;
 		u32 item_size;
 		u32 cur;
 
 		leaf = path->nodes[0];
 		slot = path->slots[0];
 
-		/* this is where we start walking through the path */
-		if (slot >= btrfs_header_nritems(leaf)) {
-			/*
-			 * if we've reached the last slot in this leaf we need
-			 * to go to the next leaf and reset everything
-			 */
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				goto err;
-			else if (ret > 0)
-				break;
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(leaf, &found_key, slot);
-
 		/* check to make sure this item is what we want */
-		if (found_key.objectid != key.objectid)
-			break;
-		if (found_key.type > BTRFS_XATTR_ITEM_KEY)
+		if (found_key.objectid != key.objectid ||
+		    found_key.type > BTRFS_XATTR_ITEM_KEY)
 			break;
 		if (found_key.type < BTRFS_XATTR_ITEM_KEY)
-			goto next_item;
+			continue;
 
 		di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
 		item_size = btrfs_item_size_nr(leaf, slot);
@@ -365,9 +345,13 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
 			cur += this_len;
 			di = (struct btrfs_dir_item *)((char *)di + this_len);
 		}
-next_item:
-		path->slots[0]++;
 	}
+
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto err;
+	}
+
 	ret = total_size;
 
 err:
-- 
2.31.1

[PATCH 2/7] btrfs: block-group: use btrfs_for_each_slot in find_first_block_group

[Marcos Paulo de Souza]

The function can be simplified by using the iterator like macro.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/block-group.c | 33 +++++----------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 1f8b06afbd03..0d999a3bfc84 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1654,38 +1654,15 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
 				  struct btrfs_path *path,
 				  struct btrfs_key *key)
 {
-	struct btrfs_root *root = fs_info->extent_root;
-	int ret;
 	struct btrfs_key found_key;
-	struct extent_buffer *leaf;
-	int slot;
-
-	ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
-	if (ret < 0)
-		return ret;
-
-	while (1) {
-		slot = path->slots[0];
-		leaf = path->nodes[0];
-		if (slot >= btrfs_header_nritems(leaf)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret == 0)
-				continue;
-			if (ret < 0)
-				goto out;
-			break;
-		}
-		btrfs_item_key_to_cpu(leaf, &found_key, slot);
+	int ret;
 
+	btrfs_for_each_slot(fs_info->extent_root, key, &found_key, path, ret) {
 		if (found_key.objectid >= key->objectid &&
-		    found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
-			ret = read_bg_from_eb(fs_info, &found_key, path);
-			break;
-		}
-
-		path->slots[0]++;
+		    found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY)
+			return read_bg_from_eb(fs_info, &found_key, path);
 	}
-out:
+
 	return ret;
 }
 
-- 
2.31.1

[PATCH 3/7] btrfs: dev-replace: Use btrfs_for_each_slot in mark_block_group_to_copy

[Marcos Paulo de Souza]

The function can be simplified by using the macro.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/dev-replace.c | 51 +++++++++---------------------------------
 1 file changed, 11 insertions(+), 40 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index d029be40ea6f..6124f2e8e9f1 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -523,63 +523,34 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
 	key.type = BTRFS_DEV_EXTENT_KEY;
 	key.offset = 0;
 
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto free_path;
-	if (ret > 0) {
-		if (path->slots[0] >=
-		    btrfs_header_nritems(path->nodes[0])) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				goto free_path;
-			if (ret > 0) {
-				ret = 0;
-				goto free_path;
-			}
-		} else {
-			ret = 0;
-		}
-	}
-
-	while (1) {
+	btrfs_for_each_slot(root, &key, &found_key, path, ret) {
 		struct extent_buffer *leaf = path->nodes[0];
-		int slot = path->slots[0];
-
-		btrfs_item_key_to_cpu(leaf, &found_key, slot);
-
-		if (found_key.objectid != src_dev->devid)
-			break;
 
-		if (found_key.type != BTRFS_DEV_EXTENT_KEY)
+		if (found_key.objectid != src_dev->devid ||
+		    found_key.type != BTRFS_DEV_EXTENT_KEY ||
+		    found_key.offset < key.offset)
 			break;
 
-		if (found_key.offset < key.offset)
-			break;
-
-		dev_extent = btrfs_item_ptr(leaf, slot, struct btrfs_dev_extent);
+		dev_extent = btrfs_item_ptr(leaf, path->slots[0],
+				struct btrfs_dev_extent);
 
 		chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dev_extent);
 
 		cache = btrfs_lookup_block_group(fs_info, chunk_offset);
 		if (!cache)
-			goto skip;
+			continue;
 
 		spin_lock(&cache->lock);
 		cache->to_copy = 1;
 		spin_unlock(&cache->lock);
 
 		btrfs_put_block_group(cache);
-
-skip:
-		ret = btrfs_next_item(root, path);
-		if (ret != 0) {
-			if (ret > 0)
-				ret = 0;
-			break;
-		}
 	}
 
-free_path:
+	/* Reset error if the key wasn't found. */
+	if (ret > 0)
+		ret = 0;
+
 	btrfs_free_path(path);
 unlock:
 	mutex_unlock(&fs_info->chunk_mutex);
-- 
2.31.1

[PATCH 4/7] btrfs: dir-item: use btrfs_for_each_slot in btrfs_search_dir_index_item

[Marcos Paulo de Souza]

The function can be simplified by using the iterator like macro.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/dir-item.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index f1274d5c3805..10560663dae1 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -301,36 +301,15 @@ btrfs_search_dir_index_item(struct btrfs_root *root,
 			    struct btrfs_path *path, u64 dirid,
 			    const char *name, int name_len)
 {
-	struct extent_buffer *leaf;
 	struct btrfs_dir_item *di;
 	struct btrfs_key key;
-	u32 nritems;
 	int ret;
 
 	key.objectid = dirid;
 	key.type = BTRFS_DIR_INDEX_KEY;
 	key.offset = 0;
 
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		return ERR_PTR(ret);
-
-	leaf = path->nodes[0];
-	nritems = btrfs_header_nritems(leaf);
-
-	while (1) {
-		if (path->slots[0] >= nritems) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				return ERR_PTR(ret);
-			if (ret > 0)
-				break;
-			leaf = path->nodes[0];
-			nritems = btrfs_header_nritems(leaf);
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+	btrfs_for_each_slot(root, &key, &key, path, ret) {
 		if (key.objectid != dirid || key.type != BTRFS_DIR_INDEX_KEY)
 			break;
 
@@ -338,9 +317,9 @@ btrfs_search_dir_index_item(struct btrfs_root *root,
 					       name, name_len);
 		if (di)
 			return di;
-
-		path->slots[0]++;
 	}
+	if (ret < 0)
+		return ERR_PTR(ret);
 	return NULL;
 }
 
-- 
2.31.1

[PATCH 5/7] btrfs: inode: use btrfs_for_each_slot in btrfs_read_readdir

[Marcos Paulo de Souza]

The function can be simplified by using the macro.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/inode.c | 46 +++++++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 29 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2aa9646bce56..12bee0107015 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6109,8 +6109,7 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 	struct list_head ins_list;
 	struct list_head del_list;
 	int ret;
-	struct extent_buffer *leaf;
-	int slot;
+	int iter_ret;
 	char *name_ptr;
 	int name_len;
 	int entries = 0;
@@ -6137,35 +6136,19 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 	key.offset = ctx->pos;
 	key.objectid = btrfs_ino(BTRFS_I(inode));
 
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto err;
-
-	while (1) {
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		struct dir_entry *entry;
+		struct extent_buffer *leaf = path->nodes[0];
 
-		leaf = path->nodes[0];
-		slot = path->slots[0];
-		if (slot >= btrfs_header_nritems(leaf)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				goto err;
-			else if (ret > 0)
-				break;
-			continue;
-		}
+		if (found_key.objectid != key.objectid ||
+		    found_key.type != BTRFS_DIR_INDEX_KEY)
+			break;
 
-		btrfs_item_key_to_cpu(leaf, &found_key, slot);
+		if (found_key.offset < ctx->pos ||
+		    btrfs_should_delete_dir_index(&del_list, found_key.offset))
+			continue;
 
-		if (found_key.objectid != key.objectid)
-			break;
-		if (found_key.type != BTRFS_DIR_INDEX_KEY)
-			break;
-		if (found_key.offset < ctx->pos)
-			goto next;
-		if (btrfs_should_delete_dir_index(&del_list, found_key.offset))
-			goto next;
-		di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
+		di = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_dir_item);
 		name_len = btrfs_dir_name_len(leaf, di);
 		if ((total_len + sizeof(struct dir_entry) + name_len) >=
 		    PAGE_SIZE) {
@@ -6192,9 +6175,14 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 		entries++;
 		addr += sizeof(struct dir_entry) + name_len;
 		total_len += sizeof(struct dir_entry) + name_len;
-next:
-		path->slots[0]++;
 	}
+
+	/* Error found while searching. */
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto err;
+	}
+
 	btrfs_release_path(path);
 
 	ret = btrfs_filldir(private->filldir_buf, entries, ctx);
-- 
2.31.1

[PATCH 6/7] btrfs: send: Use btrfs_for_each_slot macro

[Marcos Paulo de Souza]

This makes the code much simpler and smaller.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/send.c | 222 +++++++++++++-----------------------------------
 1 file changed, 59 insertions(+), 163 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index afdcbe7844e0..3e32886e171e 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2647,6 +2647,7 @@ static int send_create_inode(struct send_ctx *sctx, u64 ino)
  */
 static int did_create_dir(struct send_ctx *sctx, u64 dir)
 {
+	int iter_ret;
 	int ret = 0;
 	struct btrfs_path *path = NULL;
 	struct btrfs_key key;
@@ -2654,43 +2655,23 @@ static int did_create_dir(struct send_ctx *sctx, u64 dir)
 	struct btrfs_key di_key;
 	struct extent_buffer *eb;
 	struct btrfs_dir_item *di;
-	int slot;
 
 	path = alloc_path_for_send();
-	if (!path) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!path)
+		return -ENOMEM;
 
 	key.objectid = dir;
 	key.type = BTRFS_DIR_INDEX_KEY;
 	key.offset = 0;
-	ret = btrfs_search_slot(NULL, sctx->send_root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
-	while (1) {
-		eb = path->nodes[0];
-		slot = path->slots[0];
-		if (slot >= btrfs_header_nritems(eb)) {
-			ret = btrfs_next_leaf(sctx->send_root, path);
-			if (ret < 0) {
-				goto out;
-			} else if (ret > 0) {
-				ret = 0;
-				break;
-			}
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(eb, &found_key, slot);
+	btrfs_for_each_slot(sctx->send_root, &key, &found_key, path, iter_ret) {
 		if (found_key.objectid != key.objectid ||
 		    found_key.type != key.type) {
 			ret = 0;
 			goto out;
 		}
 
-		di = btrfs_item_ptr(eb, slot, struct btrfs_dir_item);
+		eb = path->nodes[0];
+		di = btrfs_item_ptr(eb, path->slots[0], struct btrfs_dir_item);
 		btrfs_dir_item_key_to_cpu(eb, di, &di_key);
 
 		if (di_key.type != BTRFS_ROOT_ITEM_KEY &&
@@ -2698,10 +2679,12 @@ static int did_create_dir(struct send_ctx *sctx, u64 dir)
 			ret = 1;
 			goto out;
 		}
-
-		path->slots[0]++;
 	}
 
+	/* Only set ret if there was an error in the search. */
+	if (iter_ret < 0)
+		ret = iter_ret;
+
 out:
 	btrfs_free_path(path);
 	return ret;
@@ -2905,6 +2888,7 @@ static void free_orphan_dir_info(struct send_ctx *sctx,
 static int can_rmdir(struct send_ctx *sctx, u64 dir, u64 dir_gen,
 		     u64 send_progress)
 {
+	int iter_ret;
 	int ret = 0;
 	struct btrfs_root *root = sctx->parent_root;
 	struct btrfs_path *path;
@@ -2932,23 +2916,9 @@ static int can_rmdir(struct send_ctx *sctx, u64 dir, u64 dir_gen,
 	if (odi)
 		key.offset = odi->last_dir_index_offset;
 
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
-	while (1) {
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		struct waiting_dir_move *dm;
 
-		if (path->slots[0] >= btrfs_header_nritems(path->nodes[0])) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				goto out;
-			else if (ret > 0)
-				break;
-			continue;
-		}
-		btrfs_item_key_to_cpu(path->nodes[0], &found_key,
-				      path->slots[0]);
 		if (found_key.objectid != key.objectid ||
 		    found_key.type != key.type)
 			break;
@@ -2984,8 +2954,13 @@ static int can_rmdir(struct send_ctx *sctx, u64 dir, u64 dir_gen,
 			goto out;
 		}
 
-		path->slots[0]++;
 	}
+
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto out;
+	}
+
 	free_orphan_dir_info(sctx, odi);
 
 	ret = 1;
@@ -3563,6 +3538,7 @@ static int is_ancestor(struct btrfs_root *root,
 		       struct fs_path *fs_path)
 {
 	bool free_fs_path = false;
+	int iter_ret = 0;
 	int ret = 0;
 	struct btrfs_path *path = NULL;
 	struct btrfs_key key;
@@ -3584,26 +3560,12 @@ static int is_ancestor(struct btrfs_root *root,
 	key.type = BTRFS_INODE_REF_KEY;
 	key.offset = 0;
 
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
-	while (true) {
+	btrfs_for_each_slot(root, &key, &key, path, iter_ret) {
 		struct extent_buffer *leaf = path->nodes[0];
 		int slot = path->slots[0];
 		u32 cur_offset = 0;
 		u32 item_size;
 
-		if (slot >= btrfs_header_nritems(leaf)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				goto out;
-			if (ret > 0)
-				break;
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(leaf, &key, slot);
 		if (key.objectid != ino2)
 			break;
 		if (key.type != BTRFS_INODE_REF_KEY &&
@@ -3641,8 +3603,13 @@ static int is_ancestor(struct btrfs_root *root,
 			if (ret)
 				goto out;
 		}
-		path->slots[0]++;
 	}
+
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto out;
+	}
+
 	ret = 0;
  out:
 	btrfs_free_path(path);
@@ -4524,13 +4491,12 @@ static int record_changed_ref(struct send_ctx *sctx)
 static int process_all_refs(struct send_ctx *sctx,
 			    enum btrfs_compare_tree_result cmd)
 {
-	int ret;
+	int iter_ret;
+	int ret = 0;
 	struct btrfs_root *root;
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct btrfs_key found_key;
-	struct extent_buffer *eb;
-	int slot;
 	iterate_inode_ref_t cb;
 	int pending_move = 0;
 
@@ -4554,24 +4520,7 @@ static int process_all_refs(struct send_ctx *sctx,
 	key.objectid = sctx->cmp_key->objectid;
 	key.type = BTRFS_INODE_REF_KEY;
 	key.offset = 0;
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
-	while (1) {
-		eb = path->nodes[0];
-		slot = path->slots[0];
-		if (slot >= btrfs_header_nritems(eb)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				goto out;
-			else if (ret > 0)
-				break;
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(eb, &found_key, slot);
-
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		if (found_key.objectid != key.objectid ||
 		    (found_key.type != BTRFS_INODE_REF_KEY &&
 		     found_key.type != BTRFS_INODE_EXTREF_KEY))
@@ -4580,9 +4529,13 @@ static int process_all_refs(struct send_ctx *sctx,
 		ret = iterate_inode_ref(root, path, &found_key, 0, cb, sctx);
 		if (ret < 0)
 			goto out;
+	}
 
-		path->slots[0]++;
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto out;
 	}
+
 	btrfs_release_path(path);
 
 	/*
@@ -4847,13 +4800,12 @@ static int process_changed_xattr(struct send_ctx *sctx)
 
 static int process_all_new_xattrs(struct send_ctx *sctx)
 {
-	int ret;
+	int iter_ret;
+	int ret = 0;
 	struct btrfs_root *root;
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct btrfs_key found_key;
-	struct extent_buffer *eb;
-	int slot;
 
 	path = alloc_path_for_send();
 	if (!path)
@@ -4864,39 +4816,18 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
 	key.objectid = sctx->cmp_key->objectid;
 	key.type = BTRFS_XATTR_ITEM_KEY;
 	key.offset = 0;
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
-	while (1) {
-		eb = path->nodes[0];
-		slot = path->slots[0];
-		if (slot >= btrfs_header_nritems(eb)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0) {
-				goto out;
-			} else if (ret > 0) {
-				ret = 0;
-				break;
-			}
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(eb, &found_key, slot);
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		if (found_key.objectid != key.objectid ||
-		    found_key.type != key.type) {
-			ret = 0;
-			goto out;
-		}
+		    found_key.type != key.type)
+			break;
 
 		ret = iterate_dir_item(root, path, __process_new_xattr, sctx);
 		if (ret < 0)
-			goto out;
-
-		path->slots[0]++;
+			break;
 	}
+	if (iter_ret < 0)
+		ret = iter_ret;
 
-out:
 	btrfs_free_path(path);
 	return ret;
 }
@@ -5938,13 +5869,12 @@ static int process_extent(struct send_ctx *sctx,
 
 static int process_all_extents(struct send_ctx *sctx)
 {
-	int ret;
+	int iter_ret;
+	int ret = 0;
 	struct btrfs_root *root;
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct btrfs_key found_key;
-	struct extent_buffer *eb;
-	int slot;
 
 	root = sctx->send_root;
 	path = alloc_path_for_send();
@@ -5954,40 +5884,19 @@ static int process_all_extents(struct send_ctx *sctx)
 	key.objectid = sctx->cmp_key->objectid;
 	key.type = BTRFS_EXTENT_DATA_KEY;
 	key.offset = 0;
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
-	while (1) {
-		eb = path->nodes[0];
-		slot = path->slots[0];
-
-		if (slot >= btrfs_header_nritems(eb)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0) {
-				goto out;
-			} else if (ret > 0) {
-				ret = 0;
-				break;
-			}
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(eb, &found_key, slot);
-
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		if (found_key.objectid != key.objectid ||
-		    found_key.type != key.type) {
-			ret = 0;
+		    found_key.type != key.type)
 			goto out;
-		}
 
 		ret = process_extent(sctx, path, &found_key);
 		if (ret < 0)
 			goto out;
-
-		path->slots[0]++;
 	}
 
+	if (iter_ret < 0)
+		ret = iter_ret;
+
 out:
 	btrfs_free_path(path);
 	return ret;
@@ -6180,36 +6089,20 @@ static int btrfs_unlink_all_paths(struct send_ctx *sctx)
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct parent_paths_ctx ctx;
-	int ret;
+	int iter_ret;
+	int ret = 0;
 
 	path = alloc_path_for_send();
 	if (!path)
 		return -ENOMEM;
 
-	key.objectid = sctx->cur_ino;
-	key.type = BTRFS_INODE_REF_KEY;
-	key.offset = 0;
-	ret = btrfs_search_slot(NULL, sctx->parent_root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-
 	ctx.refs = &deleted_refs;
 	ctx.sctx = sctx;
 
-	while (true) {
-		struct extent_buffer *eb = path->nodes[0];
-		int slot = path->slots[0];
-
-		if (slot >= btrfs_header_nritems(eb)) {
-			ret = btrfs_next_leaf(sctx->parent_root, path);
-			if (ret < 0)
-				goto out;
-			else if (ret > 0)
-				break;
-			continue;
-		}
-
-		btrfs_item_key_to_cpu(eb, &key, slot);
+	key.objectid = sctx->cur_ino;
+	key.type = BTRFS_INODE_REF_KEY;
+	key.offset = 0;
+	btrfs_for_each_slot(sctx->parent_root, &key, &key, path, iter_ret) {
 		if (key.objectid != sctx->cur_ino)
 			break;
 		if (key.type != BTRFS_INODE_REF_KEY &&
@@ -6220,8 +6113,11 @@ static int btrfs_unlink_all_paths(struct send_ctx *sctx)
 					record_parent_ref, &ctx);
 		if (ret < 0)
 			goto out;
+	}
 
-		path->slots[0]++;
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto out;
 	}
 
 	while (!list_empty(&deleted_refs)) {
-- 
2.31.1

[PATCH 7/7] btrfs: volumes: use btrfs_for_each_slot in btrfs_read_chunk_tree

[Marcos Paulo de Souza]

The function can be simplified by using the iterator like macro.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/volumes.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d72e4e3e02b1..998838ac106c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7428,8 +7428,9 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
 	struct btrfs_key found_key;
-	int ret;
 	int slot;
+	int iter_ret;
+	int ret = 0;
 	u64 total_dev = 0;
 	u64 last_ra_node = 0;
 
@@ -7460,22 +7461,12 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
 	key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
 	key.offset = 0;
 	key.type = 0;
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto error;
-	while (1) {
+	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
 		struct extent_buffer *node;
 
 		leaf = path->nodes[0];
 		slot = path->slots[0];
-		if (slot >= btrfs_header_nritems(leaf)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret == 0)
-				continue;
-			if (ret < 0)
-				goto error;
-			break;
-		}
+
 		/*
 		 * The nodes on level 1 are not locked but we don't need to do
 		 * that during mount time as nothing else can access the tree
@@ -7513,7 +7504,11 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
 			if (ret)
 				goto error;
 		}
-		path->slots[0]++;
+	}
+
+	if (iter_ret < 0) {
+		ret = iter_ret;
+		goto error;
 	}
 
 	/*
-- 
2.31.1

Re: [PATCH 1/7] fs: btrfs: Introduce btrfs_for_each_slot

[Johannes Thumshirn]

On 24/08/2021 19:13, Marcos Paulo de Souza wrote:
> There is a common pattern when search for a key in btrfs:
> 
>  * Call btrfs_search_slot
>  * Endless loop
> 	 * If the found slot is bigger than the current items in the leaf, check the
> 	   next one
> 	 * If still not found in the next leaf, return 1
> 	 * Do something with the code
> 	 * Increment current slot, and continue
> 
> This pattern can be improved by creating an iterator macro, similar to
> those for_each_X already existing in the linux kernel. Using this
> approach means to reduce significantly boilerplate code, along making it
> easier to newcomers to understand how to code works.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  fs/btrfs/ctree.c | 28 ++++++++++++++++++++++++++++
>  fs/btrfs/ctree.h | 25 +++++++++++++++++++++++++
>  fs/btrfs/xattr.c | 40 ++++++++++++----------------------------
>  3 files changed, 65 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 84627cbd5b5b..b1aa6e3987d0 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2122,6 +2122,34 @@ int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
>  	return ret;
>  }
>  
> +/* Search for a valid slot for the given path.
> + * @root: The root node of the tree.
> + * @key: Will contain a valid item if found.
> + * @path: The start point to validate the slot.
> + *
> + * Return 0 if the item is valid, 1 if not found and < 0 if error.
> + */
> +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
> +		     struct btrfs_path *path)
> +{
> +	while (1) {
> +		int ret;
> +		const int slot = path->slots[0];
> +		const struct extent_buffer *leaf = path->nodes[0];
> +
> +		if (slot >= btrfs_header_nritems(leaf)) {
> +			ret = btrfs_next_leaf(root, path);
> +			if (ret)
> +				return ret;
> +			continue;
> +		}
> +		btrfs_item_key_to_cpu(leaf, key, slot);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * adjust the pointers going up the tree, starting at level
>   * making sure the right key of each node is points to 'key'.
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index f07c82fafa04..1e3c4a7741ca 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2912,6 +2912,31 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>  int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
>  			   struct btrfs_path *path);
>  
> +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
> +		     struct btrfs_path *path);
> +
> +/* Search in @root for a given @key, and store the slot found in @found_key.
> + * @root: The root node of the tree.
> + * @key: The key we are looking for.
> + * @found_key: Will hold the found item.
> + * @path: Holds the current slot/leaf.
> + * @iter_ret: Contains the returned value from btrfs_search_slot and
> + *            btrfs_valid_slot, whatever is executed later.
> + *
> + * The iter_ret is an output variable that will contain the result of the
> + * btrfs_search_slot if it returns an error, or the value returned from
> + * btrfs_valid_slot otherwise. The return value can be 0 if the something was
> + * found, 1 if there weren't bigger leaves, and <0 if error.
> + */
> +#define btrfs_for_each_slot(root, key, found_key, path, iter_ret)		\
> +	for (iter_ret = btrfs_search_slot(NULL, root, key, path, 0, 0);		\
> +		(								\
> +			iter_ret >= 0 &&					\
> +			(iter_ret = btrfs_valid_slot(root, found_key, path)) == 0 \
> +		);								  \
> +		path->slots[0]++						  \
> +	)
> +
>  static inline int btrfs_next_old_item(struct btrfs_root *root,
>  				      struct btrfs_path *p, u64 time_seq)
>  {

Shouldn't below xattr code be in a separate patch? Just like the block-group,
dev-replace, etc changes?

> diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> index 8a4514283a4b..f85febba1891 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -274,10 +274,12 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name,
>  ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
>  {
>  	struct btrfs_key key;
> +	struct btrfs_key found_key;
>  	struct inode *inode = d_inode(dentry);
>  	struct btrfs_root *root = BTRFS_I(inode)->root;
>  	struct btrfs_path *path;
>  	int ret = 0;
> +	int iter_ret = 0;
>  	size_t total_size = 0, size_left = size;
>  
>  	/*
> @@ -295,44 +297,22 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
>  	path->reada = READA_FORWARD;
>  
>  	/* search for our xattrs */
> -	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> -	if (ret < 0)
> -		goto err;
> -
> -	while (1) {
> +	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
>  		struct extent_buffer *leaf;
>  		int slot;
>  		struct btrfs_dir_item *di;
> -		struct btrfs_key found_key;
>  		u32 item_size;
>  		u32 cur;
>  
>  		leaf = path->nodes[0];
>  		slot = path->slots[0];
>  
> -		/* this is where we start walking through the path */
> -		if (slot >= btrfs_header_nritems(leaf)) {
> -			/*
> -			 * if we've reached the last slot in this leaf we need
> -			 * to go to the next leaf and reset everything
> -			 */
> -			ret = btrfs_next_leaf(root, path);
> -			if (ret < 0)
> -				goto err;
> -			else if (ret > 0)
> -				break;
> -			continue;
> -		}
> -
> -		btrfs_item_key_to_cpu(leaf, &found_key, slot);
> -
>  		/* check to make sure this item is what we want */
> -		if (found_key.objectid != key.objectid)
> -			break;
> -		if (found_key.type > BTRFS_XATTR_ITEM_KEY)
> +		if (found_key.objectid != key.objectid ||
> +		    found_key.type > BTRFS_XATTR_ITEM_KEY)
>  			break;
>  		if (found_key.type < BTRFS_XATTR_ITEM_KEY)
> -			goto next_item;
> +			continue;
>  
>  		di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
>  		item_size = btrfs_item_size_nr(leaf, slot);
> @@ -365,9 +345,13 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
>  			cur += this_len;
>  			di = (struct btrfs_dir_item *)((char *)di + this_len);
>  		}
> -next_item:
> -		path->slots[0]++;
>  	}
> +
> +	if (iter_ret < 0) {
> +		ret = iter_ret;
> +		goto err;
> +	}
> +
>  	ret = total_size;
>  
>  err:
> 

Re: [PATCH 1/7] fs: btrfs: Introduce btrfs_for_each_slot

[David Sterba]

On Wed, Aug 25, 2021 at 07:42:43AM +0000, Johannes Thumshirn wrote:
> On 24/08/2021 19:13, Marcos Paulo de Souza wrote:
> >  static inline int btrfs_next_old_item(struct btrfs_root *root,
> >  				      struct btrfs_path *p, u64 time_seq)
> >  {
> 
> Shouldn't below xattr code be in a separate patch? Just like the block-group,
> dev-replace, etc changes?

Yes.

Re: [PATCH 1/7] fs: btrfs: Introduce btrfs_for_each_slot

[Marcos Paulo de Souza]

On Wed, 2021-08-25 at 07:42 +0000, Johannes Thumshirn wrote:
> On 24/08/2021 19:13, Marcos Paulo de Souza wrote:
> > There is a common pattern when search for a key in btrfs:
> > 
> >  * Call btrfs_search_slot
> >  * Endless loop
> > 	 * If the found slot is bigger than the current items in the
> > leaf, check the
> > 	   next one
> > 	 * If still not found in the next leaf, return 1
> > 	 * Do something with the code
> > 	 * Increment current slot, and continue
> > 
> > This pattern can be improved by creating an iterator macro, similar
> > to
> > those for_each_X already existing in the linux kernel. Using this
> > approach means to reduce significantly boilerplate code, along
> > making it
> > easier to newcomers to understand how to code works.
> > 
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> >  fs/btrfs/ctree.c | 28 ++++++++++++++++++++++++++++
> >  fs/btrfs/ctree.h | 25 +++++++++++++++++++++++++
> >  fs/btrfs/xattr.c | 40 ++++++++++++----------------------------
> >  3 files changed, 65 insertions(+), 28 deletions(-)
> > 
> > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> > index 84627cbd5b5b..b1aa6e3987d0 100644
> > --- a/fs/btrfs/ctree.c
> > +++ b/fs/btrfs/ctree.c
> > @@ -2122,6 +2122,34 @@ int btrfs_search_backwards(struct btrfs_root
> > *root, struct btrfs_key *key,
> >  	return ret;
> >  }
> >  
> > +/* Search for a valid slot for the given path.
> > + * @root: The root node of the tree.
> > + * @key: Will contain a valid item if found.
> > + * @path: The start point to validate the slot.
> > + *
> > + * Return 0 if the item is valid, 1 if not found and < 0 if error.
> > + */
> > +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key
> > *key,
> > +		     struct btrfs_path *path)
> > +{
> > +	while (1) {
> > +		int ret;
> > +		const int slot = path->slots[0];
> > +		const struct extent_buffer *leaf = path->nodes[0];
> > +
> > +		if (slot >= btrfs_header_nritems(leaf)) {
> > +			ret = btrfs_next_leaf(root, path);
> > +			if (ret)
> > +				return ret;
> > +			continue;
> > +		}
> > +		btrfs_item_key_to_cpu(leaf, key, slot);
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * adjust the pointers going up the tree, starting at level
> >   * making sure the right key of each node is points to 'key'.
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index f07c82fafa04..1e3c4a7741ca 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -2912,6 +2912,31 @@ int btrfs_next_old_leaf(struct btrfs_root
> > *root, struct btrfs_path *path,
> >  int btrfs_search_backwards(struct btrfs_root *root, struct
> > btrfs_key *key,
> >  			   struct btrfs_path *path);
> >  
> > +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key
> > *key,
> > +		     struct btrfs_path *path);
> > +
> > +/* Search in @root for a given @key, and store the slot found in
> > @found_key.
> > + * @root: The root node of the tree.
> > + * @key: The key we are looking for.
> > + * @found_key: Will hold the found item.
> > + * @path: Holds the current slot/leaf.
> > + * @iter_ret: Contains the returned value from btrfs_search_slot
> > and
> > + *            btrfs_valid_slot, whatever is executed later.
> > + *
> > + * The iter_ret is an output variable that will contain the result
> > of the
> > + * btrfs_search_slot if it returns an error, or the value returned
> > from
> > + * btrfs_valid_slot otherwise. The return value can be 0 if the
> > something was
> > + * found, 1 if there weren't bigger leaves, and <0 if error.
> > + */
> > +#define btrfs_for_each_slot(root, key, found_key, path, iter_ret)	
> > 	\
> > +	for (iter_ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
> > 		\
> > +		(								
> > \
> > +			iter_ret >= 0 &&					
> > \
> > +			(iter_ret = btrfs_valid_slot(root, found_key,
> > path)) == 0 \
> > +		);								
> >   \
> > +		path->slots[0]++						
> >   \
> > +	)
> > +
> >  static inline int btrfs_next_old_item(struct btrfs_root *root,
> >  				      struct btrfs_path *p, u64
> > time_seq)
> >  {
> 
> Shouldn't below xattr code be in a separate patch? Just like the
> block-group,
> dev-replace, etc changes?

You're right, this was a leftover of my RFC patches. I'll send a new
version as soon.

> 
> > diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> > index 8a4514283a4b..f85febba1891 100644
> > --- a/fs/btrfs/xattr.c
> > +++ b/fs/btrfs/xattr.c
> > @@ -274,10 +274,12 @@ int btrfs_setxattr_trans(struct inode *inode,
> > const char *name,
> >  ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer,
> > size_t size)
> >  {
> >  	struct btrfs_key key;
> > +	struct btrfs_key found_key;
> >  	struct inode *inode = d_inode(dentry);
> >  	struct btrfs_root *root = BTRFS_I(inode)->root;
> >  	struct btrfs_path *path;
> >  	int ret = 0;
> > +	int iter_ret = 0;
> >  	size_t total_size = 0, size_left = size;
> >  
> >  	/*
> > @@ -295,44 +297,22 @@ ssize_t btrfs_listxattr(struct dentry
> > *dentry, char *buffer, size_t size)
> >  	path->reada = READA_FORWARD;
> >  
> >  	/* search for our xattrs */
> > -	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> > -	if (ret < 0)
> > -		goto err;
> > -
> > -	while (1) {
> > +	btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
> >  		struct extent_buffer *leaf;
> >  		int slot;
> >  		struct btrfs_dir_item *di;
> > -		struct btrfs_key found_key;
> >  		u32 item_size;
> >  		u32 cur;
> >  
> >  		leaf = path->nodes[0];
> >  		slot = path->slots[0];
> >  
> > -		/* this is where we start walking through the path */
> > -		if (slot >= btrfs_header_nritems(leaf)) {
> > -			/*
> > -			 * if we've reached the last slot in this leaf
> > we need
> > -			 * to go to the next leaf and reset everything
> > -			 */
> > -			ret = btrfs_next_leaf(root, path);
> > -			if (ret < 0)
> > -				goto err;
> > -			else if (ret > 0)
> > -				break;
> > -			continue;
> > -		}
> > -
> > -		btrfs_item_key_to_cpu(leaf, &found_key, slot);
> > -
> >  		/* check to make sure this item is what we want */
> > -		if (found_key.objectid != key.objectid)
> > -			break;
> > -		if (found_key.type > BTRFS_XATTR_ITEM_KEY)
> > +		if (found_key.objectid != key.objectid ||
> > +		    found_key.type > BTRFS_XATTR_ITEM_KEY)
> >  			break;
> >  		if (found_key.type < BTRFS_XATTR_ITEM_KEY)
> > -			goto next_item;
> > +			continue;
> >  
> >  		di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
> >  		item_size = btrfs_item_size_nr(leaf, slot);
> > @@ -365,9 +345,13 @@ ssize_t btrfs_listxattr(struct dentry *dentry,
> > char *buffer, size_t size)
> >  			cur += this_len;
> >  			di = (struct btrfs_dir_item *)((char *)di +
> > this_len);
> >  		}
> > -next_item:
> > -		path->slots[0]++;
> >  	}
> > +
> > +	if (iter_ret < 0) {
> > +		ret = iter_ret;
> > +		goto err;
> > +	}
> > +
> >  	ret = total_size;
> >  
> >  err:
> >