On 2019/12/5 3:38 PM, Qu Wenruo wrote:
On 2019/12/5 下午12:29, damenly.su@xxxxxxxxx wrote:
From: Su Yue <Damenly_Su@xxxxxxx>
Add the new value 2 of @contains in block_group_cache_tree_search().
The new values means the function will return the block group that
contains bytenr, otherwise return the next one that starts after
@bytenr. Will be used in later commit.
Signed-off-by: Su Yue <Damenly_Su@xxxxxxx>
---
extent-tree.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/extent-tree.c b/extent-tree.c
index ab576f8732a2..1d8535049eaf 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -196,13 +196,16 @@ static int btrfs_add_block_group_cache(struct btrfs_fs_info *info,
}
/*
- * This will return the block group at or after bytenr if contains is 0, else
- * it will return the block group that contains the bytenr
+ * @contains:
+ * if 0, return the block group at or after bytenr if contains is 0.
+ * if 1, return the block group that contains the bytenr.
+ * if 2, return the block group that contains bytenr, otherwise return the
+ * next one that starts after @bytenr.
Thats a creative solution, good job on that.
However since contains is no longer just simple 1 or 0, it's better to
enum to define the behavior, other than using the immediate numbers.
Nice suggestion, will do.
*/
static struct btrfs_block_group_cache *block_group_cache_tree_search(
struct btrfs_fs_info *info, u64 bytenr, int contains)
{
- struct btrfs_block_group_cache *cache, *ret = NULL;
+ struct btrfs_block_group_cache *cache, *ret = NULL, *tmp = NULL;
struct rb_node *n;
u64 end, start;
@@ -215,8 +218,8 @@ static struct btrfs_block_group_cache *block_group_cache_tree_search(
start = cache->key.objectid;
if (bytenr < start) {
- if (!contains && (!ret || start < ret->key.objectid))
- ret = cache;
+ if (!tmp || start < tmp->key.objectid)
+ tmp = cache;
This doesn't look correct.
I was expecting something based on last found node, other than doing
something strange in the rb-tree iteration code.
At least this breaks readability. It would be much better to handle this
after the rb tree while loop.
Spent much brain power on this trick, this line means we always keep the
next block block to @bytenr. The original code stores the block group
cache in the @ret, which here I do is to store it into @tmp.
This method doesn't change efficiency only little memory copies.
If put the logic after the whole loop, I'm afraid it will require more
code lines and lower the search efficiency.
Thanks
Thanks,
Qu
n = n->rb_left;
} else if (bytenr > start) {
if (contains && bytenr <= end) {
@@ -229,6 +232,13 @@ static struct btrfs_block_group_cache *block_group_cache_tree_search(
break;
}
}
+
+ /*
+ * If ret is NULL, means not found any block group cotanins @bytenr.
+ * So just except the case that cotanins equals 1.
+ */
+ if (!ret && contains != 1)
+ ret = tmp;
return ret;
}
