[PATCH RFC] btrfs: Introduce btrfs_for_each_slot

[PATCH RFC] 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>
---

 I've being testing this approach in the last few weeks, and using this macro
 all over the btrfs codebase, and not issues found yet. This is just a RFC
 showing how the xattr code would benefit using the macro.

 The only part that I didn't like was using the ret variable as a macro
 argument, but I couldn't find a better way to do it...

 That's why this is an RFC, so please comment :)

 fs/btrfs/ctree.c | 23 +++++++++++++++++++++++
 fs/btrfs/ctree.h | 12 ++++++++++++
 fs/btrfs/xattr.c | 37 +++++++++----------------------------
 3 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 4f8d8fa1e085..a49b256d78f7 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2123,6 +2123,29 @@ int btrfs_search_backwards(struct btrfs_root *root,
 	return ret;
 }
 
+int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
+		     struct btrfs_path *path)
+{
+	struct extent_buffer *leaf;
+	int slot;
+	int 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)
+				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 0a971e98f5f9..cff2a94700b2 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2912,6 +2912,18 @@ 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 *f,
+		     struct btrfs_path *path);
+
+#define btrfs_for_each_slot(root, key, found_key, path, ret)		\
+	for (ret = btrfs_search_slot(NULL, root, key, path, 0, 0);		\
+		({								\
+			ret >= 0 &&						\
+			(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..0562a17ff3b1 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -274,6 +274,7 @@ 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;
@@ -295,44 +296,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, 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 +344,11 @@ 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 (ret < 0)
+		goto err;
+
 	ret = total_size;
 
 err:
-- 
2.26.2

Re: [PATCH RFC] btrfs: Introduce btrfs_for_each_slot

[Marcos Paulo de Souza]

On Mon, 2021-08-02 at 09:57 -0300, 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>
> ---
> 
>  I've being testing this approach in the last few weeks, and using
> this macro
>  all over the btrfs codebase, and not issues found yet. This is just
> a RFC
>  showing how the xattr code would benefit using the macro.
> 
>  The only part that I didn't like was using the ret variable as a
> macro
>  argument, but I couldn't find a better way to do it...
> 
>  That's why this is an RFC, so please comment :)

Gentle ping :)

> 
>  fs/btrfs/ctree.c | 23 +++++++++++++++++++++++
>  fs/btrfs/ctree.h | 12 ++++++++++++
>  fs/btrfs/xattr.c | 37 +++++++++----------------------------
>  3 files changed, 44 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 4f8d8fa1e085..a49b256d78f7 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2123,6 +2123,29 @@ int btrfs_search_backwards(struct btrfs_root
> *root,
>  	return ret;
>  }
>  
> +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
> +		     struct btrfs_path *path)
> +{
> +	struct extent_buffer *leaf;
> +	int slot;
> +	int 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)
> +				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 0a971e98f5f9..cff2a94700b2 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2912,6 +2912,18 @@ 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 *f,
> +		     struct btrfs_path *path);
> +
> +#define btrfs_for_each_slot(root, key, found_key, path, ret)		
> \
> +	for (ret = btrfs_search_slot(NULL, root, key, path, 0, 0);	
> 	\
> +		({							
> 	\
> +			ret >= 0 &&					
> 	\
> +			(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..0562a17ff3b1 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -274,6 +274,7 @@ 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;
> @@ -295,44 +296,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, 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 +344,11 @@ 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 (ret < 0)
> +		goto err;
> +
>  	ret = total_size;
>  
>  err:

Re: [PATCH RFC] btrfs: Introduce btrfs_for_each_slot

[Nikolay Borisov]

On 16.08.21 г. 15:21, Marcos Paulo de Souza wrote:
> On Mon, 2021-08-02 at 09:57 -0300, 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>
>> ---
>>
>>  I've being testing this approach in the last few weeks, and using
>> this macro
>>  all over the btrfs codebase, and not issues found yet. This is just
>> a RFC
>>  showing how the xattr code would benefit using the macro.
>>
>>  The only part that I didn't like was using the ret variable as a
>> macro
>>  argument, but I couldn't find a better way to do it...
>>
>>  That's why this is an RFC, so please comment :)
> 
> Gentle ping :)
> 


I didn't give a RB because the patch is too small in the sense that it
shows just a single caller. In order to be able to better ascertain how
useful is I was expecting you'd submit a larger series containing all
the necessary changes. Additionally, does this patch relate to the
changes in 'btrfs: Use btrfs_find_item whenever possible' or are those 2
independent?

Re: [PATCH RFC] btrfs: Introduce btrfs_for_each_slot

[David Sterba]

On Mon, Aug 16, 2021 at 09:21:30AM -0300, Marcos Paulo de Souza wrote:
> On Mon, 2021-08-02 at 09:57 -0300, 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>
> > ---
> > 
> >  I've being testing this approach in the last few weeks, and using
> > this macro
> >  all over the btrfs codebase, and not issues found yet. This is just
> > a RFC
> >  showing how the xattr code would benefit using the macro.
> > 
> >  The only part that I didn't like was using the ret variable as a
> > macro
> >  argument, but I couldn't find a better way to do it...
> > 
> >  That's why this is an RFC, so please comment :)
> 
> Gentle ping :)

We're just before code freeze for the next merge window so non-critical
patches are on hold.

Re: [PATCH RFC] btrfs: Introduce btrfs_for_each_slot

[David Sterba]

On Mon, Aug 02, 2021 at 09:57:38AM -0300, 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>
> ---
> 
>  I've being testing this approach in the last few weeks, and using this macro
>  all over the btrfs codebase, and not issues found yet. This is just a RFC
>  showing how the xattr code would benefit using the macro.
> 
>  The only part that I didn't like was using the ret variable as a macro
>  argument, but I couldn't find a better way to do it...
> 
>  That's why this is an RFC, so please comment :)
> 
>  fs/btrfs/ctree.c | 23 +++++++++++++++++++++++
>  fs/btrfs/ctree.h | 12 ++++++++++++
>  fs/btrfs/xattr.c | 37 +++++++++----------------------------
>  3 files changed, 44 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 4f8d8fa1e085..a49b256d78f7 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2123,6 +2123,29 @@ int btrfs_search_backwards(struct btrfs_root *root,
>  	return ret;
>  }
>  
> +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
> +		     struct btrfs_path *path)
> +{
> +	struct extent_buffer *leaf;
> +	int slot;
> +	int 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)
> +				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 0a971e98f5f9..cff2a94700b2 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2912,6 +2912,18 @@ 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 *f,
> +		     struct btrfs_path *path);
> +
> +#define btrfs_for_each_slot(root, key, found_key, path, ret)		\
> +	for (ret = btrfs_search_slot(NULL, root, key, path, 0, 0);		\
> +		({								\
> +			ret >= 0 &&						\
> +			(ret = btrfs_valid_slot(root, found_key, path)) == 0;	\
> +		});								\

Why is this using the ({ }) block?

> +		path->slots[0]++						\
> +	)

Re: [PATCH RFC] btrfs: Introduce btrfs_for_each_slot

[David Sterba]

On Mon, Aug 02, 2021 at 09:57:38AM -0300, 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>
> ---
> 
>  I've being testing this approach in the last few weeks, and using this macro
>  all over the btrfs codebase, and not issues found yet. This is just a RFC
>  showing how the xattr code would benefit using the macro.
> 
>  The only part that I didn't like was using the ret variable as a macro
>  argument, but I couldn't find a better way to do it...

The extra variable for ret is acceptable but it must be a separate from
the one that can be used inside the loop, otherwise the value could get
overwritten and affect the for loop. Ie. you'd have give an extra one
jsut for the loop and then it also deserves a proper documentation of
the whole btrfs_for_each_slot macro.

>  fs/btrfs/ctree.c | 23 +++++++++++++++++++++++
>  fs/btrfs/ctree.h | 12 ++++++++++++
>  fs/btrfs/xattr.c | 37 +++++++++----------------------------
>  3 files changed, 44 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 4f8d8fa1e085..a49b256d78f7 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2123,6 +2123,29 @@ int btrfs_search_backwards(struct btrfs_root *root,
>  	return ret;
>  }
>  
> +int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
> +		     struct btrfs_path *path)

Interface function should be documented and parameters should be const
where applicable.

> +{
> +	struct extent_buffer *leaf;
> +	int slot;
> +	int ret;
> +
> +	while (1) {
> +		slot = path->slots[0];
> +		leaf = path->nodes[0];

The variables should be defined in the inner-most scope

> +		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 0a971e98f5f9..cff2a94700b2 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2912,6 +2912,18 @@ 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 *f,

The parameter names don't match the definition, especially 'f' is a
misnomer and a single letter.

> +		     struct btrfs_path *path);
> +
> +#define btrfs_for_each_slot(root, key, found_key, path, ret)		\
> +	for (ret = btrfs_search_slot(NULL, root, key, path, 0, 0);		\
> +		({								\
> +			ret >= 0 &&						\
> +			(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..0562a17ff3b1 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -274,6 +274,7 @@ 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;
> @@ -295,44 +296,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, 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 +344,11 @@ 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 (ret < 0)
> +		goto err;
> +
>  	ret = total_size;
>  
>  err:
> -- 
> 2.26.2