diff mbox series

[2/6] btrfs: edit btrfs_add_block_group_cache() to use rb helper

Message ID 2c3972c084ab074fd1b6a2e03ada8c20dde6d060.1733850317.git.beckerlee3@gmail.com (mailing list archive)
State New
Headers show
Series reduce boilerplate code within btrfs | expand

Commit Message

Lee Beckermeyer Dec. 10, 2024, 7:06 p.m. UTC
Edits fs/btrfs/block-group.c to use rb_find_add_cached(),
also adds a comparison function, btrfs_add_block_blkgrp_cmp(),
for use with rb_find_add_cached().

Suggested-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Roger L. Beckermeyer III <beckerlee3@gmail.com>
Signed-off-by: Roger L. Beckermeyer III <beckerlee3@gmail.com>
---
 fs/btrfs/block-group.c | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

Comments

David Sterba Dec. 11, 2024, 6:45 p.m. UTC | #1
On Tue, Dec 10, 2024 at 01:06:08PM -0600, Roger L. Beckermeyer III wrote:
> Edits fs/btrfs/block-group.c to use rb_find_add_cached(),

Please don't use 'edit' for changelogs, 'update to use' or 'use'
describes it better.

> also adds a comparison function, btrfs_add_block_blkgrp_cmp(),
> for use with rb_find_add_cached().
> 
> Suggested-by: Josef Bacik <josef@toxicpanda.com>
> Reviewed-by: Roger L. Beckermeyer III <beckerlee3@gmail.com>
> Signed-off-by: Roger L. Beckermeyer III <beckerlee3@gmail.com>
> ---
>  fs/btrfs/block-group.c | 40 +++++++++++++++++-----------------------
>  1 file changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 5be029734cfa..ccff051de43a 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -173,40 +173,34 @@ void btrfs_put_block_group(struct btrfs_block_group *cache)
>  	}
>  }
>  
> +static int btrfs_add_blkgrp_cmp(struct rb_node *new, const struct rb_node *exist)

This is a comparator so 'add' is confusing here, so 'btrfs_bg_start_cmp'
for exmample.

> +{
> +	struct btrfs_block_group *cmp1 = rb_entry(new, struct btrfs_block_group, cache_node);
> +	struct btrfs_block_group *cmp2 = rb_entry(exist, struct btrfs_block_group, cache_node);

The types don't match, 'exist' is const.

> +
> +	if (cmp1->start < cmp2->start)
> +		return -1;
> +	if (cmp1->start > cmp2->start)
> +		return 1;
> +	return 0;
> +}
David Sterba Dec. 11, 2024, 6:49 p.m. UTC | #2
On Tue, Dec 10, 2024 at 01:06:08PM -0600, Roger L. Beckermeyer III wrote:
> +	exist = rb_find_add_cached(&block_group->cache_node,
> +			&info->block_group_cache_tree, btrfs_add_blkgrp_cmp);
> +	if (exist != NULL)

For pointer checks you can use the shorter form 'if (exist)'

> +		return -EEXIST;
>  	write_unlock(&info->block_group_cache_lock);
>  
>  	return 0;
> -- 
> 2.45.2
>
diff mbox series

Patch

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 5be029734cfa..ccff051de43a 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -173,40 +173,34 @@  void btrfs_put_block_group(struct btrfs_block_group *cache)
 	}
 }
 
+static int btrfs_add_blkgrp_cmp(struct rb_node *new, const struct rb_node *exist)
+{
+	struct btrfs_block_group *cmp1 = rb_entry(new, struct btrfs_block_group, cache_node);
+	struct btrfs_block_group *cmp2 = rb_entry(exist, struct btrfs_block_group, cache_node);
+
+	if (cmp1->start < cmp2->start)
+		return -1;
+	if (cmp1->start > cmp2->start)
+		return 1;
+	return 0;
+}
+
 /*
  * This adds the block group to the fs_info rb tree for the block group cache
  */
 static int btrfs_add_block_group_cache(struct btrfs_fs_info *info,
 				       struct btrfs_block_group *block_group)
 {
-	struct rb_node **p;
-	struct rb_node *parent = NULL;
-	struct btrfs_block_group *cache;
-	bool leftmost = true;
+	struct rb_node *exist;
 
 	ASSERT(block_group->length != 0);
 
 	write_lock(&info->block_group_cache_lock);
-	p = &info->block_group_cache_tree.rb_root.rb_node;
-
-	while (*p) {
-		parent = *p;
-		cache = rb_entry(parent, struct btrfs_block_group, cache_node);
-		if (block_group->start < cache->start) {
-			p = &(*p)->rb_left;
-		} else if (block_group->start > cache->start) {
-			p = &(*p)->rb_right;
-			leftmost = false;
-		} else {
-			write_unlock(&info->block_group_cache_lock);
-			return -EEXIST;
-		}
-	}
-
-	rb_link_node(&block_group->cache_node, parent, p);
-	rb_insert_color_cached(&block_group->cache_node,
-			       &info->block_group_cache_tree, leftmost);
 
+	exist = rb_find_add_cached(&block_group->cache_node,
+			&info->block_group_cache_tree, btrfs_add_blkgrp_cmp);
+	if (exist != NULL)
+		return -EEXIST;
 	write_unlock(&info->block_group_cache_lock);
 
 	return 0;