* [PATCH 0/6] btrfs: some bug fixes for free space cache
@ 2010-08-23 1:22 Li Zefan
2010-08-23 1:22 ` [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB Li Zefan
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:22 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
Those bugs are not critical, which means they won't crash the
filesystem, so they're hardly noticeable and no one noticed
them.
---
fs/btrfs/free-space-cache.c | 161 ++++++++++++++++++++++++++-----------------
1 files changed, 99 insertions(+), 62 deletions(-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
@ 2010-08-23 1:22 ` Li Zefan
2010-08-23 13:02 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 2/6] btrfs: add helper function free_bitmap() Li Zefan
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:22 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
If a block group is smaller than 1GB, the extent entry threadhold
calculation will always set the threshold to 0.
So as free space gets fragmented, btrfs will switch to use bitmap
to manage free space, but then will never switch back to extents
due to this bug.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/btrfs/free-space-cache.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index f488fac..7edbef6 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -263,14 +263,18 @@ static void recalculate_thresholds(struct btrfs_block_group_cache *block_group)
u64 max_bytes;
u64 bitmap_bytes;
u64 extent_bytes;
+ u64 size = block_group->key.offset;
/*
* The goal is to keep the total amount of memory used per 1gb of space
* at or below 32k, so we need to adjust how much memory we allow to be
* used by extent based free space tracking
*/
- max_bytes = MAX_CACHE_BYTES_PER_GIG *
- (div64_u64(block_group->key.offset, 1024 * 1024 * 1024));
+ if (size < 1024 * 1024 * 1024)
+ max_bytes = MAX_CACHE_BYTES_PER_GIG;
+ else
+ max_bytes = MAX_CACHE_BYTES_PER_GIG *
+ div64_u64(size, 1024 * 1024 * 1024);
/*
* we want to account for 1 more bitmap than what we have so we can make
--
1.7.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] btrfs: add helper function free_bitmap()
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
2010-08-23 1:22 ` [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB Li Zefan
@ 2010-08-23 1:23 ` Li Zefan
2010-08-23 13:03 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 3/6] btrfs: free fully occupied bitmap in cluster Li Zefan
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:23 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
Remove some duplicated code.
This prepares for the next patch.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/btrfs/free-space-cache.c | 37 ++++++++++++++++---------------------
1 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 7edbef6..017fd55 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -422,6 +422,16 @@ static void add_new_bitmap(struct btrfs_block_group_cache *block_group,
recalculate_thresholds(block_group);
}
+static void free_bitmap(struct btrfs_block_group_cache *block_group,
+ struct btrfs_free_space *bitmap_info)
+{
+ unlink_free_space(block_group, bitmap_info);
+ kfree(bitmap_info->bitmap);
+ kfree(bitmap_info);
+ block_group->total_bitmaps--;
+ recalculate_thresholds(block_group);
+}
+
static noinline int remove_from_bitmap(struct btrfs_block_group_cache *block_group,
struct btrfs_free_space *bitmap_info,
u64 *offset, u64 *bytes)
@@ -462,13 +472,8 @@ again:
if (*bytes) {
struct rb_node *next = rb_next(&bitmap_info->offset_index);
- if (!bitmap_info->bytes) {
- unlink_free_space(block_group, bitmap_info);
- kfree(bitmap_info->bitmap);
- kfree(bitmap_info);
- block_group->total_bitmaps--;
- recalculate_thresholds(block_group);
- }
+ if (!bitmap_info->bytes)
+ free_bitmap(block_group, bitmap_info);
/*
* no entry after this bitmap, but we still have bytes to
@@ -501,13 +506,8 @@ again:
return -EAGAIN;
goto again;
- } else if (!bitmap_info->bytes) {
- unlink_free_space(block_group, bitmap_info);
- kfree(bitmap_info->bitmap);
- kfree(bitmap_info);
- block_group->total_bitmaps--;
- recalculate_thresholds(block_group);
- }
+ } else if (!bitmap_info->bytes)
+ free_bitmap(block_group, bitmap_info);
return 0;
}
@@ -936,13 +936,8 @@ u64 btrfs_find_space_for_alloc(struct btrfs_block_group_cache *block_group,
ret = offset;
if (entry->bitmap) {
bitmap_clear_bits(block_group, entry, offset, bytes);
- if (!entry->bytes) {
- unlink_free_space(block_group, entry);
- kfree(entry->bitmap);
- kfree(entry);
- block_group->total_bitmaps--;
- recalculate_thresholds(block_group);
- }
+ if (!entry->bytes)
+ free_bitmap(block_group, entry);
} else {
unlink_free_space(block_group, entry);
entry->offset += bytes;
--
1.7.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] btrfs: free fully occupied bitmap in cluster
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
2010-08-23 1:22 ` [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB Li Zefan
2010-08-23 1:23 ` [PATCH 2/6] btrfs: add helper function free_bitmap() Li Zefan
@ 2010-08-23 1:23 ` Li Zefan
2010-08-23 13:04 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 4/6] btrfs: update stats when allocating from a cluster Li Zefan
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:23 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
If there's no more free space in a bitmap, we should free it,
otherwise seems it won't get freed until we free the block group.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/btrfs/free-space-cache.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 017fd55..631e14f 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -1035,6 +1035,8 @@ static u64 btrfs_alloc_from_bitmap(struct btrfs_block_group_cache *block_group,
ret = search_start;
bitmap_clear_bits(block_group, entry, ret, bytes);
+ if (entry->bytes == 0)
+ free_bitmap(block_group, entry);
out:
spin_unlock(&cluster->lock);
spin_unlock(&block_group->tree_lock);
--
1.7.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] btrfs: update stats when allocating from a cluster
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
` (2 preceding siblings ...)
2010-08-23 1:23 ` [PATCH 3/6] btrfs: free fully occupied bitmap in cluster Li Zefan
@ 2010-08-23 1:23 ` Li Zefan
2010-08-23 13:09 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 5/6] btrfs: add a helper try_merge_free_space() Li Zefan
2010-08-23 1:24 ` [PATCH 6/6] btrfs: check mergeable free space when removing a cluster Li Zefan
5 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:23 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
When allocating extent entry from a cluster, we should update
the free_space and free_extents fields of the block group.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/btrfs/free-space-cache.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 631e14f..20f3141 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -1090,15 +1090,26 @@ u64 btrfs_alloc_from_cluster(struct btrfs_block_group_cache *block_group,
entry->offset += bytes;
entry->bytes -= bytes;
- if (entry->bytes == 0) {
+ if (entry->bytes == 0)
rb_erase(&entry->offset_index, &cluster->root);
- kfree(entry);
- }
break;
}
out:
spin_unlock(&cluster->lock);
+ if (!ret)
+ return 0;
+
+ spin_lock(&block_group->tree_lock);
+
+ block_group->free_space -= bytes;
+ if (entry->bytes == 0) {
+ block_group->free_extents--;
+ kfree(entry);
+ }
+
+ spin_unlock(&block_group->tree_lock);
+
return ret;
}
--
1.7.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] btrfs: add a helper try_merge_free_space()
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
` (3 preceding siblings ...)
2010-08-23 1:23 ` [PATCH 4/6] btrfs: update stats when allocating from a cluster Li Zefan
@ 2010-08-23 1:23 ` Li Zefan
2010-08-23 13:15 ` Josef Bacik
2010-08-23 1:24 ` [PATCH 6/6] btrfs: check mergeable free space when removing a cluster Li Zefan
5 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:23 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
When adding a new extent, we'll firstly see if we can merge
this extent to the left or/and right extent. Extract this as
a helper try_merge_free_space().
As a side effect, we fix a small bug that if the new extent
has non-bitmap left entry but is unmergeble, we'll directly
link the extent without trying to drop it into bitmap.
This also prepares for the next patch.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/btrfs/free-space-cache.c | 75 ++++++++++++++++++++++++------------------
1 files changed, 43 insertions(+), 32 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 20f3141..faeec8f 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -610,22 +610,14 @@ out:
return ret;
}
-int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
- u64 offset, u64 bytes)
+bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
+ struct btrfs_free_space *info)
{
- struct btrfs_free_space *right_info = NULL;
- struct btrfs_free_space *left_info = NULL;
- struct btrfs_free_space *info = NULL;
- int ret = 0;
-
- info = kzalloc(sizeof(struct btrfs_free_space), GFP_NOFS);
- if (!info)
- return -ENOMEM;
-
- info->offset = offset;
- info->bytes = bytes;
-
- spin_lock(&block_group->tree_lock);
+ struct btrfs_free_space *left_info;
+ struct btrfs_free_space *right_info;
+ bool merged = false;
+ u64 offset = info->offset;
+ u64 bytes = info->bytes;
/*
* first we want to see if there is free space adjacent to the range we
@@ -639,27 +631,11 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
else
left_info = tree_search_offset(block_group, offset - 1, 0, 0);
- /*
- * If there was no extent directly to the left or right of this new
- * extent then we know we're going to have to allocate a new extent, so
- * before we do that see if we need to drop this into a bitmap
- */
- if ((!left_info || left_info->bitmap) &&
- (!right_info || right_info->bitmap)) {
- ret = insert_into_bitmap(block_group, info);
-
- if (ret < 0) {
- goto out;
- } else if (ret) {
- ret = 0;
- goto out;
- }
- }
-
if (right_info && !right_info->bitmap) {
unlink_free_space(block_group, right_info);
info->bytes += right_info->bytes;
kfree(right_info);
+ merged = true;
}
if (left_info && !left_info->bitmap &&
@@ -668,8 +644,43 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
info->offset = left_info->offset;
info->bytes += left_info->bytes;
kfree(left_info);
+ merged = true;
}
+ return merged;
+}
+
+int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
+ u64 offset, u64 bytes)
+{
+ struct btrfs_free_space *info;
+ int ret = 0;
+
+ info = kzalloc(sizeof(struct btrfs_free_space), GFP_NOFS);
+ if (!info)
+ return -ENOMEM;
+
+ info->offset = offset;
+ info->bytes = bytes;
+
+ spin_lock(&block_group->tree_lock);
+
+ if (try_merge_free_space(block_group, info))
+ goto link;
+
+ /*
+ * There was no extent directly to the left or right of this new
+ * extent then we know we're going to have to allocate a new extent, so
+ * before we do that see if we need to drop this into a bitmap
+ */
+ ret = insert_into_bitmap(block_group, info);
+ if (ret < 0) {
+ goto out;
+ } else if (ret) {
+ ret = 0;
+ goto out;
+ }
+link:
ret = link_free_space(block_group, info);
if (ret)
kfree(info);
--
1.7.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] btrfs: check mergeable free space when removing a cluster
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
` (4 preceding siblings ...)
2010-08-23 1:23 ` [PATCH 5/6] btrfs: add a helper try_merge_free_space() Li Zefan
@ 2010-08-23 1:24 ` Li Zefan
2010-08-23 13:15 ` Josef Bacik
5 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-23 1:24 UTC (permalink / raw)
To: Chris Mason; +Cc: Josef Bacik, linux-btrfs, LKML
After returing extents from a cluster to the block group, some
extents in the block group may be mergeable.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/btrfs/free-space-cache.c | 26 ++++++++++++++++++++------
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index faeec8f..c11f4f7 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -234,11 +234,18 @@ tree_search_offset(struct btrfs_block_group_cache *block_group,
return entry;
}
-static void unlink_free_space(struct btrfs_block_group_cache *block_group,
- struct btrfs_free_space *info)
+static inline void
+__unlink_free_space(struct btrfs_block_group_cache *block_group,
+ struct btrfs_free_space *info)
{
rb_erase(&info->offset_index, &block_group->free_space_offset);
block_group->free_extents--;
+}
+
+static void unlink_free_space(struct btrfs_block_group_cache *block_group,
+ struct btrfs_free_space *info)
+{
+ __unlink_free_space(block_group, info);
block_group->free_space -= info->bytes;
}
@@ -611,7 +618,7 @@ out:
}
bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
- struct btrfs_free_space *info)
+ struct btrfs_free_space *info, bool update_stat)
{
struct btrfs_free_space *left_info;
struct btrfs_free_space *right_info;
@@ -632,7 +639,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
left_info = tree_search_offset(block_group, offset - 1, 0, 0);
if (right_info && !right_info->bitmap) {
- unlink_free_space(block_group, right_info);
+ if (update_stat)
+ unlink_free_space(block_group, right_info);
+ else
+ __unlink_free_space(block_group, right_info);
info->bytes += right_info->bytes;
kfree(right_info);
merged = true;
@@ -640,7 +650,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
if (left_info && !left_info->bitmap &&
left_info->offset + left_info->bytes == offset) {
- unlink_free_space(block_group, left_info);
+ if (update_stat)
+ unlink_free_space(block_group, left_info);
+ else
+ __unlink_free_space(block_group, left_info);
info->offset = left_info->offset;
info->bytes += left_info->bytes;
kfree(left_info);
@@ -665,7 +678,7 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
spin_lock(&block_group->tree_lock);
- if (try_merge_free_space(block_group, info))
+ if (try_merge_free_space(block_group, info, true))
goto link;
/*
@@ -883,6 +896,7 @@ __btrfs_return_cluster_to_free_space(
node = rb_next(&entry->offset_index);
rb_erase(&entry->offset_index, &cluster->root);
BUG_ON(entry->bitmap);
+ try_merge_free_space(block_group, entry, false);
tree_insert_offset(&block_group->free_space_offset,
entry->offset, &entry->offset_index, 0);
}
--
1.7.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB
2010-08-23 1:22 ` [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB Li Zefan
@ 2010-08-23 13:02 ` Josef Bacik
0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2010-08-23 13:02 UTC (permalink / raw)
To: Li Zefan; +Cc: Chris Mason, Josef Bacik, linux-btrfs, LKML
On Mon, Aug 23, 2010 at 09:22:40AM +0800, Li Zefan wrote:
> If a block group is smaller than 1GB, the extent entry threadhold
> calculation will always set the threshold to 0.
>
> So as free space gets fragmented, btrfs will switch to use bitmap
> to manage free space, but then will never switch back to extents
> due to this bug.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> fs/btrfs/free-space-cache.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index f488fac..7edbef6 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -263,14 +263,18 @@ static void recalculate_thresholds(struct btrfs_block_group_cache *block_group)
> u64 max_bytes;
> u64 bitmap_bytes;
> u64 extent_bytes;
> + u64 size = block_group->key.offset;
>
> /*
> * The goal is to keep the total amount of memory used per 1gb of space
> * at or below 32k, so we need to adjust how much memory we allow to be
> * used by extent based free space tracking
> */
> - max_bytes = MAX_CACHE_BYTES_PER_GIG *
> - (div64_u64(block_group->key.offset, 1024 * 1024 * 1024));
> + if (size < 1024 * 1024 * 1024)
> + max_bytes = MAX_CACHE_BYTES_PER_GIG;
> + else
> + max_bytes = MAX_CACHE_BYTES_PER_GIG *
> + div64_u64(size, 1024 * 1024 * 1024);
>
> /*
> * we want to account for 1 more bitmap than what we have so we can make
> --
> 1.7.0.1
>
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] btrfs: add helper function free_bitmap()
2010-08-23 1:23 ` [PATCH 2/6] btrfs: add helper function free_bitmap() Li Zefan
@ 2010-08-23 13:03 ` Josef Bacik
0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2010-08-23 13:03 UTC (permalink / raw)
To: Li Zefan; +Cc: Chris Mason, Josef Bacik, linux-btrfs, LKML
On Mon, Aug 23, 2010 at 09:23:01AM +0800, Li Zefan wrote:
> Remove some duplicated code.
>
> This prepares for the next patch.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> fs/btrfs/free-space-cache.c | 37 ++++++++++++++++---------------------
> 1 files changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 7edbef6..017fd55 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -422,6 +422,16 @@ static void add_new_bitmap(struct btrfs_block_group_cache *block_group,
> recalculate_thresholds(block_group);
> }
>
> +static void free_bitmap(struct btrfs_block_group_cache *block_group,
> + struct btrfs_free_space *bitmap_info)
> +{
> + unlink_free_space(block_group, bitmap_info);
> + kfree(bitmap_info->bitmap);
> + kfree(bitmap_info);
> + block_group->total_bitmaps--;
> + recalculate_thresholds(block_group);
> +}
> +
> static noinline int remove_from_bitmap(struct btrfs_block_group_cache *block_group,
> struct btrfs_free_space *bitmap_info,
> u64 *offset, u64 *bytes)
> @@ -462,13 +472,8 @@ again:
>
> if (*bytes) {
> struct rb_node *next = rb_next(&bitmap_info->offset_index);
> - if (!bitmap_info->bytes) {
> - unlink_free_space(block_group, bitmap_info);
> - kfree(bitmap_info->bitmap);
> - kfree(bitmap_info);
> - block_group->total_bitmaps--;
> - recalculate_thresholds(block_group);
> - }
> + if (!bitmap_info->bytes)
> + free_bitmap(block_group, bitmap_info);
>
> /*
> * no entry after this bitmap, but we still have bytes to
> @@ -501,13 +506,8 @@ again:
> return -EAGAIN;
>
> goto again;
> - } else if (!bitmap_info->bytes) {
> - unlink_free_space(block_group, bitmap_info);
> - kfree(bitmap_info->bitmap);
> - kfree(bitmap_info);
> - block_group->total_bitmaps--;
> - recalculate_thresholds(block_group);
> - }
> + } else if (!bitmap_info->bytes)
> + free_bitmap(block_group, bitmap_info);
>
> return 0;
> }
> @@ -936,13 +936,8 @@ u64 btrfs_find_space_for_alloc(struct btrfs_block_group_cache *block_group,
> ret = offset;
> if (entry->bitmap) {
> bitmap_clear_bits(block_group, entry, offset, bytes);
> - if (!entry->bytes) {
> - unlink_free_space(block_group, entry);
> - kfree(entry->bitmap);
> - kfree(entry);
> - block_group->total_bitmaps--;
> - recalculate_thresholds(block_group);
> - }
> + if (!entry->bytes)
> + free_bitmap(block_group, entry);
> } else {
> unlink_free_space(block_group, entry);
> entry->offset += bytes;
> --
> 1.7.0.1
>
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] btrfs: free fully occupied bitmap in cluster
2010-08-23 1:23 ` [PATCH 3/6] btrfs: free fully occupied bitmap in cluster Li Zefan
@ 2010-08-23 13:04 ` Josef Bacik
0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2010-08-23 13:04 UTC (permalink / raw)
To: Li Zefan; +Cc: Chris Mason, Josef Bacik, linux-btrfs, LKML
On Mon, Aug 23, 2010 at 09:23:16AM +0800, Li Zefan wrote:
> If there's no more free space in a bitmap, we should free it,
> otherwise seems it won't get freed until we free the block group.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> fs/btrfs/free-space-cache.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 017fd55..631e14f 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -1035,6 +1035,8 @@ static u64 btrfs_alloc_from_bitmap(struct btrfs_block_group_cache *block_group,
>
> ret = search_start;
> bitmap_clear_bits(block_group, entry, ret, bytes);
> + if (entry->bytes == 0)
> + free_bitmap(block_group, entry);
> out:
> spin_unlock(&cluster->lock);
> spin_unlock(&block_group->tree_lock);
> --
> 1.7.0.1
>
Oops, good catch
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] btrfs: update stats when allocating from a cluster
2010-08-23 1:23 ` [PATCH 4/6] btrfs: update stats when allocating from a cluster Li Zefan
@ 2010-08-23 13:09 ` Josef Bacik
2010-08-24 0:50 ` Li Zefan
0 siblings, 1 reply; 17+ messages in thread
From: Josef Bacik @ 2010-08-23 13:09 UTC (permalink / raw)
To: Li Zefan; +Cc: Chris Mason, Josef Bacik, linux-btrfs, LKML
On Mon, Aug 23, 2010 at 09:23:29AM +0800, Li Zefan wrote:
> When allocating extent entry from a cluster, we should update
> the free_space and free_extents fields of the block group.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> fs/btrfs/free-space-cache.c | 17 ++++++++++++++---
> 1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 631e14f..20f3141 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -1090,15 +1090,26 @@ u64 btrfs_alloc_from_cluster(struct btrfs_block_group_cache *block_group,
> entry->offset += bytes;
> entry->bytes -= bytes;
>
> - if (entry->bytes == 0) {
> + if (entry->bytes == 0)
> rb_erase(&entry->offset_index, &cluster->root);
> - kfree(entry);
> - }
> break;
> }
> out:
> spin_unlock(&cluster->lock);
>
> + if (!ret)
> + return 0;
> +
> + spin_lock(&block_group->tree_lock);
> +
> + block_group->free_space -= bytes;
> + if (entry->bytes == 0) {
> + block_group->free_extents--;
> + kfree(entry);
> + }
> +
> + spin_unlock(&block_group->tree_lock);
> +
Move this part up so we don't drop the spinlock and then re-grab it. Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] btrfs: check mergeable free space when removing a cluster
2010-08-23 1:24 ` [PATCH 6/6] btrfs: check mergeable free space when removing a cluster Li Zefan
@ 2010-08-23 13:15 ` Josef Bacik
2010-08-24 1:04 ` Li Zefan
0 siblings, 1 reply; 17+ messages in thread
From: Josef Bacik @ 2010-08-23 13:15 UTC (permalink / raw)
To: Li Zefan; +Cc: Chris Mason, Josef Bacik, linux-btrfs, LKML
On Mon, Aug 23, 2010 at 09:24:07AM +0800, Li Zefan wrote:
> After returing extents from a cluster to the block group, some
> extents in the block group may be mergeable.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> fs/btrfs/free-space-cache.c | 26 ++++++++++++++++++++------
> 1 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index faeec8f..c11f4f7 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -234,11 +234,18 @@ tree_search_offset(struct btrfs_block_group_cache *block_group,
> return entry;
> }
>
> -static void unlink_free_space(struct btrfs_block_group_cache *block_group,
> - struct btrfs_free_space *info)
> +static inline void
> +__unlink_free_space(struct btrfs_block_group_cache *block_group,
> + struct btrfs_free_space *info)
> {
> rb_erase(&info->offset_index, &block_group->free_space_offset);
> block_group->free_extents--;
> +}
> +
> +static void unlink_free_space(struct btrfs_block_group_cache *block_group,
> + struct btrfs_free_space *info)
> +{
> + __unlink_free_space(block_group, info);
> block_group->free_space -= info->bytes;
> }
>
> @@ -611,7 +618,7 @@ out:
> }
>
> bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
> - struct btrfs_free_space *info)
> + struct btrfs_free_space *info, bool update_stat)
> {
> struct btrfs_free_space *left_info;
> struct btrfs_free_space *right_info;
> @@ -632,7 +639,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
> left_info = tree_search_offset(block_group, offset - 1, 0, 0);
>
> if (right_info && !right_info->bitmap) {
> - unlink_free_space(block_group, right_info);
> + if (update_stat)
> + unlink_free_space(block_group, right_info);
> + else
> + __unlink_free_space(block_group, right_info);
> info->bytes += right_info->bytes;
> kfree(right_info);
> merged = true;
> @@ -640,7 +650,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
>
> if (left_info && !left_info->bitmap &&
> left_info->offset + left_info->bytes == offset) {
> - unlink_free_space(block_group, left_info);
> + if (update_stat)
> + unlink_free_space(block_group, left_info);
> + else
> + __unlink_free_space(block_group, left_info);
> info->offset = left_info->offset;
> info->bytes += left_info->bytes;
> kfree(left_info);
> @@ -665,7 +678,7 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
>
> spin_lock(&block_group->tree_lock);
>
> - if (try_merge_free_space(block_group, info))
> + if (try_merge_free_space(block_group, info, true))
> goto link;
>
> /*
> @@ -883,6 +896,7 @@ __btrfs_return_cluster_to_free_space(
> node = rb_next(&entry->offset_index);
> rb_erase(&entry->offset_index, &cluster->root);
> BUG_ON(entry->bitmap);
> + try_merge_free_space(block_group, entry, false);
> tree_insert_offset(&block_group->free_space_offset,
> entry->offset, &entry->offset_index, 0);
It's early in the morning here, so forgive me if I'm missing something obvious,
but if try_merge_free_space() succeeds here, then won't tree_insert_offset()
then add an entry that overlaps the same spot? Should we do something like
merged = try_merge_free_space();
if (!merged)
tree_insert_offset();
? Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/6] btrfs: add a helper try_merge_free_space()
2010-08-23 1:23 ` [PATCH 5/6] btrfs: add a helper try_merge_free_space() Li Zefan
@ 2010-08-23 13:15 ` Josef Bacik
0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2010-08-23 13:15 UTC (permalink / raw)
To: Li Zefan; +Cc: Chris Mason, Josef Bacik, linux-btrfs, LKML
On Mon, Aug 23, 2010 at 09:23:44AM +0800, Li Zefan wrote:
> When adding a new extent, we'll firstly see if we can merge
> this extent to the left or/and right extent. Extract this as
> a helper try_merge_free_space().
>
> As a side effect, we fix a small bug that if the new extent
> has non-bitmap left entry but is unmergeble, we'll directly
> link the extent without trying to drop it into bitmap.
>
> This also prepares for the next patch.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> fs/btrfs/free-space-cache.c | 75 ++++++++++++++++++++++++------------------
> 1 files changed, 43 insertions(+), 32 deletions(-)
>
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 20f3141..faeec8f 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -610,22 +610,14 @@ out:
> return ret;
> }
>
> -int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
> - u64 offset, u64 bytes)
> +bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
> + struct btrfs_free_space *info)
> {
> - struct btrfs_free_space *right_info = NULL;
> - struct btrfs_free_space *left_info = NULL;
> - struct btrfs_free_space *info = NULL;
> - int ret = 0;
> -
> - info = kzalloc(sizeof(struct btrfs_free_space), GFP_NOFS);
> - if (!info)
> - return -ENOMEM;
> -
> - info->offset = offset;
> - info->bytes = bytes;
> -
> - spin_lock(&block_group->tree_lock);
> + struct btrfs_free_space *left_info;
> + struct btrfs_free_space *right_info;
> + bool merged = false;
> + u64 offset = info->offset;
> + u64 bytes = info->bytes;
>
> /*
> * first we want to see if there is free space adjacent to the range we
> @@ -639,27 +631,11 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
> else
> left_info = tree_search_offset(block_group, offset - 1, 0, 0);
>
> - /*
> - * If there was no extent directly to the left or right of this new
> - * extent then we know we're going to have to allocate a new extent, so
> - * before we do that see if we need to drop this into a bitmap
> - */
> - if ((!left_info || left_info->bitmap) &&
> - (!right_info || right_info->bitmap)) {
> - ret = insert_into_bitmap(block_group, info);
> -
> - if (ret < 0) {
> - goto out;
> - } else if (ret) {
> - ret = 0;
> - goto out;
> - }
> - }
> -
> if (right_info && !right_info->bitmap) {
> unlink_free_space(block_group, right_info);
> info->bytes += right_info->bytes;
> kfree(right_info);
> + merged = true;
> }
>
> if (left_info && !left_info->bitmap &&
> @@ -668,8 +644,43 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
> info->offset = left_info->offset;
> info->bytes += left_info->bytes;
> kfree(left_info);
> + merged = true;
> }
>
> + return merged;
> +}
> +
> +int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
> + u64 offset, u64 bytes)
> +{
> + struct btrfs_free_space *info;
> + int ret = 0;
> +
> + info = kzalloc(sizeof(struct btrfs_free_space), GFP_NOFS);
> + if (!info)
> + return -ENOMEM;
> +
> + info->offset = offset;
> + info->bytes = bytes;
> +
> + spin_lock(&block_group->tree_lock);
> +
> + if (try_merge_free_space(block_group, info))
> + goto link;
> +
> + /*
> + * There was no extent directly to the left or right of this new
> + * extent then we know we're going to have to allocate a new extent, so
> + * before we do that see if we need to drop this into a bitmap
> + */
> + ret = insert_into_bitmap(block_group, info);
> + if (ret < 0) {
> + goto out;
> + } else if (ret) {
> + ret = 0;
> + goto out;
> + }
> +link:
> ret = link_free_space(block_group, info);
> if (ret)
> kfree(info);
> --
> 1.7.0.1
>
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] btrfs: update stats when allocating from a cluster
2010-08-23 13:09 ` Josef Bacik
@ 2010-08-24 0:50 ` Li Zefan
2010-08-24 2:50 ` Josef Bacik
0 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-24 0:50 UTC (permalink / raw)
To: Josef Bacik; +Cc: Chris Mason, linux-btrfs, LKML
>> spin_unlock(&cluster->lock);
>>
>> + if (!ret)
>> + return 0;
>> +
>> + spin_lock(&block_group->tree_lock);
>> +
>> + block_group->free_space -= bytes;
>> + if (entry->bytes == 0) {
>> + block_group->free_extents--;
>> + kfree(entry);
>> + }
>> +
>> + spin_unlock(&block_group->tree_lock);
>> +
>
> Move this part up so we don't drop the spinlock and then re-grab it. Thanks,
>
But they are 2 different locks. ;)
And we can't grab block_group->tree_lock when holding cluster->lock.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] btrfs: check mergeable free space when removing a cluster
2010-08-23 13:15 ` Josef Bacik
@ 2010-08-24 1:04 ` Li Zefan
2010-08-24 2:58 ` Josef Bacik
0 siblings, 1 reply; 17+ messages in thread
From: Li Zefan @ 2010-08-24 1:04 UTC (permalink / raw)
To: Josef Bacik; +Cc: Chris Mason, linux-btrfs, LKML
Josef Bacik wrote:
> On Mon, Aug 23, 2010 at 09:24:07AM +0800, Li Zefan wrote:
>> After returing extents from a cluster to the block group, some
>> extents in the block group may be mergeable.
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>> ---
>> fs/btrfs/free-space-cache.c | 26 ++++++++++++++++++++------
>> 1 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
>> index faeec8f..c11f4f7 100644
>> --- a/fs/btrfs/free-space-cache.c
>> +++ b/fs/btrfs/free-space-cache.c
>> @@ -234,11 +234,18 @@ tree_search_offset(struct btrfs_block_group_cache *block_group,
>> return entry;
>> }
>>
>> -static void unlink_free_space(struct btrfs_block_group_cache *block_group,
>> - struct btrfs_free_space *info)
>> +static inline void
>> +__unlink_free_space(struct btrfs_block_group_cache *block_group,
>> + struct btrfs_free_space *info)
>> {
>> rb_erase(&info->offset_index, &block_group->free_space_offset);
>> block_group->free_extents--;
>> +}
>> +
>> +static void unlink_free_space(struct btrfs_block_group_cache *block_group,
>> + struct btrfs_free_space *info)
>> +{
>> + __unlink_free_space(block_group, info);
>> block_group->free_space -= info->bytes;
>> }
>>
>> @@ -611,7 +618,7 @@ out:
>> }
>>
>> bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
>> - struct btrfs_free_space *info)
>> + struct btrfs_free_space *info, bool update_stat)
>> {
>> struct btrfs_free_space *left_info;
>> struct btrfs_free_space *right_info;
>> @@ -632,7 +639,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
>> left_info = tree_search_offset(block_group, offset - 1, 0, 0);
>>
>> if (right_info && !right_info->bitmap) {
>> - unlink_free_space(block_group, right_info);
>> + if (update_stat)
>> + unlink_free_space(block_group, right_info);
>> + else
>> + __unlink_free_space(block_group, right_info);
>> info->bytes += right_info->bytes;
>> kfree(right_info);
>> merged = true;
>> @@ -640,7 +650,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
>>
>> if (left_info && !left_info->bitmap &&
>> left_info->offset + left_info->bytes == offset) {
>> - unlink_free_space(block_group, left_info);
>> + if (update_stat)
>> + unlink_free_space(block_group, left_info);
>> + else
>> + __unlink_free_space(block_group, left_info);
>> info->offset = left_info->offset;
>> info->bytes += left_info->bytes;
>> kfree(left_info);
>> @@ -665,7 +678,7 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
>>
>> spin_lock(&block_group->tree_lock);
>>
>> - if (try_merge_free_space(block_group, info))
>> + if (try_merge_free_space(block_group, info, true))
>> goto link;
>>
>> /*
>> @@ -883,6 +896,7 @@ __btrfs_return_cluster_to_free_space(
>> node = rb_next(&entry->offset_index);
>> rb_erase(&entry->offset_index, &cluster->root);
>> BUG_ON(entry->bitmap);
>> + try_merge_free_space(block_group, entry, false);
>> tree_insert_offset(&block_group->free_space_offset,
>> entry->offset, &entry->offset_index, 0);
>
> It's early in the morning here, so forgive me if I'm missing something obvious,
> but if try_merge_free_space() succeeds here, then won't tree_insert_offset()
> then add an entry that overlaps the same spot? Should we do something like
>
> merged = try_merge_free_space();
> if (!merged)
> tree_insert_offset();
>
> ? Thanks,
>
In try_merge_free_space() we try to merge the left and right entry into
@entry, and then the caller has to insert the @entry into the tree by
itself.
Similarly, In btrfs_add_free_space() we call link_free_space() no matter
try_merge_free_space() returns success or not.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] btrfs: update stats when allocating from a cluster
2010-08-24 0:50 ` Li Zefan
@ 2010-08-24 2:50 ` Josef Bacik
0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2010-08-24 2:50 UTC (permalink / raw)
To: Li Zefan; +Cc: Josef Bacik, Chris Mason, linux-btrfs, LKML
On Tue, Aug 24, 2010 at 08:50:57AM +0800, Li Zefan wrote:
> >> spin_unlock(&cluster->lock);
> >>
> >> + if (!ret)
> >> + return 0;
> >> +
> >> + spin_lock(&block_group->tree_lock);
> >> +
> >> + block_group->free_space -= bytes;
> >> + if (entry->bytes == 0) {
> >> + block_group->free_extents--;
> >> + kfree(entry);
> >> + }
> >> +
> >> + spin_unlock(&block_group->tree_lock);
> >> +
> >
> > Move this part up so we don't drop the spinlock and then re-grab it. Thanks,
> >
>
> But they are 2 different locks. ;)
>
> And we can't grab block_group->tree_lock when holding cluster->lock.
Ugh Jesus sorry, ignore me.
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] btrfs: check mergeable free space when removing a cluster
2010-08-24 1:04 ` Li Zefan
@ 2010-08-24 2:58 ` Josef Bacik
0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2010-08-24 2:58 UTC (permalink / raw)
To: Li Zefan; +Cc: Josef Bacik, Chris Mason, linux-btrfs, LKML
On Tue, Aug 24, 2010 at 09:04:13AM +0800, Li Zefan wrote:
> Josef Bacik wrote:
> > On Mon, Aug 23, 2010 at 09:24:07AM +0800, Li Zefan wrote:
> >> After returing extents from a cluster to the block group, some
> >> extents in the block group may be mergeable.
> >>
> >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> >> ---
> >> fs/btrfs/free-space-cache.c | 26 ++++++++++++++++++++------
> >> 1 files changed, 20 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> >> index faeec8f..c11f4f7 100644
> >> --- a/fs/btrfs/free-space-cache.c
> >> +++ b/fs/btrfs/free-space-cache.c
> >> @@ -234,11 +234,18 @@ tree_search_offset(struct btrfs_block_group_cache *block_group,
> >> return entry;
> >> }
> >>
> >> -static void unlink_free_space(struct btrfs_block_group_cache *block_group,
> >> - struct btrfs_free_space *info)
> >> +static inline void
> >> +__unlink_free_space(struct btrfs_block_group_cache *block_group,
> >> + struct btrfs_free_space *info)
> >> {
> >> rb_erase(&info->offset_index, &block_group->free_space_offset);
> >> block_group->free_extents--;
> >> +}
> >> +
> >> +static void unlink_free_space(struct btrfs_block_group_cache *block_group,
> >> + struct btrfs_free_space *info)
> >> +{
> >> + __unlink_free_space(block_group, info);
> >> block_group->free_space -= info->bytes;
> >> }
> >>
> >> @@ -611,7 +618,7 @@ out:
> >> }
> >>
> >> bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
> >> - struct btrfs_free_space *info)
> >> + struct btrfs_free_space *info, bool update_stat)
> >> {
> >> struct btrfs_free_space *left_info;
> >> struct btrfs_free_space *right_info;
> >> @@ -632,7 +639,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
> >> left_info = tree_search_offset(block_group, offset - 1, 0, 0);
> >>
> >> if (right_info && !right_info->bitmap) {
> >> - unlink_free_space(block_group, right_info);
> >> + if (update_stat)
> >> + unlink_free_space(block_group, right_info);
> >> + else
> >> + __unlink_free_space(block_group, right_info);
> >> info->bytes += right_info->bytes;
> >> kfree(right_info);
> >> merged = true;
> >> @@ -640,7 +650,10 @@ bool try_merge_free_space(struct btrfs_block_group_cache *block_group,
> >>
> >> if (left_info && !left_info->bitmap &&
> >> left_info->offset + left_info->bytes == offset) {
> >> - unlink_free_space(block_group, left_info);
> >> + if (update_stat)
> >> + unlink_free_space(block_group, left_info);
> >> + else
> >> + __unlink_free_space(block_group, left_info);
> >> info->offset = left_info->offset;
> >> info->bytes += left_info->bytes;
> >> kfree(left_info);
> >> @@ -665,7 +678,7 @@ int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
> >>
> >> spin_lock(&block_group->tree_lock);
> >>
> >> - if (try_merge_free_space(block_group, info))
> >> + if (try_merge_free_space(block_group, info, true))
> >> goto link;
> >>
> >> /*
> >> @@ -883,6 +896,7 @@ __btrfs_return_cluster_to_free_space(
> >> node = rb_next(&entry->offset_index);
> >> rb_erase(&entry->offset_index, &cluster->root);
> >> BUG_ON(entry->bitmap);
> >> + try_merge_free_space(block_group, entry, false);
> >> tree_insert_offset(&block_group->free_space_offset,
> >> entry->offset, &entry->offset_index, 0);
> >
> > It's early in the morning here, so forgive me if I'm missing something obvious,
> > but if try_merge_free_space() succeeds here, then won't tree_insert_offset()
> > then add an entry that overlaps the same spot? Should we do something like
> >
> > merged = try_merge_free_space();
> > if (!merged)
> > tree_insert_offset();
> >
> > ? Thanks,
> >
>
> In try_merge_free_space() we try to merge the left and right entry into
> @entry, and then the caller has to insert the @entry into the tree by
> itself.
>
> Similarly, In btrfs_add_free_space() we call link_free_space() no matter
> try_merge_free_space() returns success or not.
Ah ok I see now, thanks.
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2010-08-24 2:58 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-23 1:22 [PATCH 0/6] btrfs: some bug fixes for free space cache Li Zefan
2010-08-23 1:22 ` [PATCH 1/6] btrfs: fix threshold calculation for block groups smaller than 1GB Li Zefan
2010-08-23 13:02 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 2/6] btrfs: add helper function free_bitmap() Li Zefan
2010-08-23 13:03 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 3/6] btrfs: free fully occupied bitmap in cluster Li Zefan
2010-08-23 13:04 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 4/6] btrfs: update stats when allocating from a cluster Li Zefan
2010-08-23 13:09 ` Josef Bacik
2010-08-24 0:50 ` Li Zefan
2010-08-24 2:50 ` Josef Bacik
2010-08-23 1:23 ` [PATCH 5/6] btrfs: add a helper try_merge_free_space() Li Zefan
2010-08-23 13:15 ` Josef Bacik
2010-08-23 1:24 ` [PATCH 6/6] btrfs: check mergeable free space when removing a cluster Li Zefan
2010-08-23 13:15 ` Josef Bacik
2010-08-24 1:04 ` Li Zefan
2010-08-24 2:58 ` Josef Bacik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.