Re: [PATCH] Btrfs: rewrite btrfs_trim_block_group()

2011-11-23 Thread Li Zefan
David Sterba wrote:
> On Thu, Nov 17, 2011 at 03:26:17PM +0800, Li Zefan wrote:
>> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
>> index 8c32434..89cc54e 100644
>> --- a/fs/btrfs/free-space-cache.c
>> +++ b/fs/btrfs/free-space-cache.c
>> +static int trim_no_bitmap(struct btrfs_block_group_cache *block_group,
>> +  u64 *total_trimmed, u64 start, u64 end, u64 minlen)
>> +{
>> +struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
>> +struct btrfs_free_space *entry;
>> +struct rb_node *node;
>> +int ret;
> int ret = 0;
> 
>   CC [M]  fs/btrfs/free-space-cache.o
> fs/btrfs/free-space-cache.c: In function trim_no_bitmap:
> fs/btrfs/free-space-cache.c:2636: warning: ret may be used uninitialized in 
> this function
> 
> Although it seems the uninitialized value could never be returned.
> 

Actually it's possible. The odd thing is my gcc doesn't complain about this.

Will fix. Thanks!

--
Li Zefan
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: rewrite btrfs_trim_block_group()

2011-11-23 Thread David Sterba
On Thu, Nov 17, 2011 at 03:26:17PM +0800, Li Zefan wrote:
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 8c32434..89cc54e 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> +static int trim_no_bitmap(struct btrfs_block_group_cache *block_group,
> +   u64 *total_trimmed, u64 start, u64 end, u64 minlen)
> +{
> + struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
> + struct btrfs_free_space *entry;
> + struct rb_node *node;
> + int ret;
int ret = 0;

  CC [M]  fs/btrfs/free-space-cache.o
fs/btrfs/free-space-cache.c: In function trim_no_bitmap:
fs/btrfs/free-space-cache.c:2636: warning: ret may be used uninitialized in 
this function

Although it seems the uninitialized value could never be returned.


david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Btrfs: rewrite btrfs_trim_block_group()

2011-11-16 Thread Li Zefan
There are various bugs in block group trimming:

- It may trim from offset smaller than user-specified offset.
- It may trim beyond user-specified range.
- It may leak free space for extents smaller than specified minlen.
- It may truncate the last trimmed extent thus leak free space.
- With mixed extents+bitmaps, some extents may not be trimmed.
- With mixed extents+bitmaps, some bitmaps may not be trimmed (even
none will be trimmed). Even for those trimmed, not all the free space
in the bitmaps will be trimmed.

I rewrite btrfs_trim_block_group() and break it into two functions.
One is to trim extents only, and the other is to trim bitmaps only.

Signed-off-by: Li Zefan 
---
 fs/btrfs/free-space-cache.c |  235 ++-
 1 files changed, 164 insertions(+), 71 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 8c32434..89cc54e 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -2575,17 +2575,57 @@ void btrfs_init_free_cluster(struct btrfs_free_cluster 
*cluster)
cluster->block_group = NULL;
 }
 
-int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
-  u64 *trimmed, u64 start, u64 end, u64 minlen)
+static int do_trimming(struct btrfs_block_group_cache *block_group,
+  u64 *total_trimmed, u64 start, u64 bytes,
+  u64 reserved_start, u64 reserved_bytes)
 {
-   struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
-   struct btrfs_free_space *entry = NULL;
+   struct btrfs_space_info *space_info = block_group->space_info;
struct btrfs_fs_info *fs_info = block_group->fs_info;
-   u64 bytes = 0;
-   u64 actually_trimmed;
-   int ret = 0;
+   int ret;
+   int update = 0;
+   u64 trimmed = 0;
 
-   *trimmed = 0;
+   spin_lock(&space_info->lock);
+   spin_lock(&block_group->lock);
+   if (!block_group->ro) {
+   block_group->reserved += reserved_bytes;
+   space_info->bytes_reserved += reserved_bytes;
+   update = 1;
+   }
+   spin_unlock(&block_group->lock);
+   spin_unlock(&space_info->lock);
+
+   ret = btrfs_error_discard_extent(fs_info->extent_root,
+start, bytes, &trimmed);
+   if (!ret)
+   *total_trimmed += trimmed;
+
+   btrfs_add_free_space(block_group, reserved_start, reserved_bytes);
+
+   if (update) {
+   spin_lock(&space_info->lock);
+   spin_lock(&block_group->lock);
+   if (block_group->ro)
+   space_info->bytes_readonly += reserved_bytes;
+   block_group->reserved -= reserved_bytes;
+   space_info->bytes_reserved -= reserved_bytes;
+   spin_unlock(&space_info->lock);
+   spin_unlock(&block_group->lock);
+   }
+
+   return ret;
+}
+
+static int trim_no_bitmap(struct btrfs_block_group_cache *block_group,
+ u64 *total_trimmed, u64 start, u64 end, u64 minlen)
+{
+   struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
+   struct btrfs_free_space *entry;
+   struct rb_node *node;
+   int ret;
+   u64 extent_start;
+   u64 extent_bytes;
+   u64 bytes;
 
while (start < end) {
spin_lock(&ctl->tree_lock);
@@ -2596,81 +2636,118 @@ int btrfs_trim_block_group(struct 
btrfs_block_group_cache *block_group,
}
 
entry = tree_search_offset(ctl, start, 0, 1);
-   if (!entry)
-   entry = tree_search_offset(ctl,
-  offset_to_bitmap(ctl, start),
-  1, 1);
-
-   if (!entry || entry->offset >= end) {
+   if (!entry) {
spin_unlock(&ctl->tree_lock);
break;
}
 
-   if (entry->bitmap) {
-   ret = search_bitmap(ctl, entry, &start, &bytes);
-   if (!ret) {
-   if (start >= end) {
-   spin_unlock(&ctl->tree_lock);
-   break;
-   }
-   bytes = min(bytes, end - start);
-   bitmap_clear_bits(ctl, entry, start, bytes);
-   if (entry->bytes == 0)
-   free_bitmap(ctl, entry);
-   } else {
-   start = entry->offset + BITS_PER_BITMAP *
-   block_group->sectorsize;
+   /* skip bitmaps */
+   while (entry->bitmap) {
+   node = rb_next(&entry->offset_index);
+   if (!node) {