Skip to contentSkip to conversation

Btrfs: fix space leak when skipping small extents during trimming

PATCH In review Li Zefan 2 participants 4 messages Jun 21, 2011 – Jul 5, 2011 Btrfs file system

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/2 Btrfs: fix space leak when skipping small extents during trimming

We're taking a free space extent out of the free space cache, trimming it and then putting it back into the cache.

However for an extent that is smaller than the specified minimum length, it's taken out but won't be put back, which causes space leak.

---

Unfortunately I have no trim-able device to test the patch.

---
 fs/btrfs/free-space-cache.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)
fs/btrfs/free-space-cache.c +17 -17
index 9f985a4..292c0d9 100644
@@ -2460,6 +2460,7 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 	u64 bytes = 0;
 	u64 actually_trimmed;
 	int ret = 0;
+	int update_ret;
 
 	*trimmed = 0;
 
@@ -2483,6 +2484,7 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 		}
 
 		if (entry->bitmap) {
+			bytes = 0;
 			ret = search_bitmap(ctl, entry, &start, &bytes);
 			if (!ret) {
 				if (start >= end) {
@@ -2490,6 +2492,8 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 					break;
 				}
 				bytes = min(bytes, end - start);
+				if (bytes < minlen)
+					goto next;
 				bitmap_clear_bits(ctl, entry, start, bytes);
 				if (entry->bytes == 0)
 					free_bitmap(ctl, entry);
@@ -2503,33 +2507,29 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 		} else {
 			start = entry->offset;
 			bytes = min(entry->bytes, end - start);
+			if (bytes < minlen)
+				goto next;
 			unlink_free_space(ctl, entry);
 			kmem_cache_free(btrfs_free_space_cachep, entry);
 		}
 
 		spin_unlock(&ctl->tree_lock);
 
-		if (bytes >= minlen) {
-			int update_ret;
-			update_ret = btrfs_update_reserved_bytes(block_group,
-								 bytes, 1, 1);
+		update_ret = btrfs_update_reserved_bytes(block_group,
+							 bytes, 1, 1);
 
-			ret = btrfs_error_discard_extent(fs_info->extent_root,
-							 start,
-							 bytes,
-							 &actually_trimmed);
+		ret = btrfs_error_discard_extent(fs_info->extent_root, start,
+						 bytes, &actually_trimmed);
 
-			btrfs_add_free_space(block_group, start, bytes);
-			if (!update_ret)
-				btrfs_update_reserved_bytes(block_group,
-							    bytes, 0, 1);
+		btrfs_add_free_space(block_group, start, bytes);
+		if (!update_ret)
+			btrfs_update_reserved_bytes(block_group, bytes, 0, 1);
 
-			if (ret)
-				break;
-			*trimmed += actually_trimmed;
-		}
+		if (ret)
+			break;
+		*trimmed += actually_trimmed;
+next:
 		start += bytes;
-		bytes = 0;
 
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
author
#1 lore raw

PATCH 2/2 Btrfs: fix space leak when trimming free extents

When the end of an extent exceeds the end of the specified range, the extent will be accidentally truncated.

---
 fs/btrfs/free-space-cache.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
fs/btrfs/free-space-cache.c +8 -1
index 292c0d9..185cf8e 100644
@@ -2509,8 +2509,15 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 			bytes = min(entry->bytes, end - start);
 			if (bytes < minlen)
 				goto next;
+
 			unlink_free_space(ctl, entry);
-			kmem_cache_free(btrfs_free_space_cachep, entry);
+			if (bytes < entry->bytes) {
+				entry->offset = entry->offset + bytes;
+				entry->bytes = entry->bytes - bytes;
+				link_free_space(ctl, entry);
+			} else {
+				kmem_cache_free(btrfs_free_space_cachep, entry);
+			}
 		}
 
 		spin_unlock(&ctl->tree_lock);

-- 1.7.3.1

#2 lore raw

Re: [PATCH 2/2] Btrfs: fix space leak when trimming free extents

Quoted 13 lines · Li Zefan

When the end of an extent exceeds the end of the specified range, the extent will be accidentally truncated.

---
 fs/btrfs/free-space-cache.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
fs/btrfs/free-space-cache.c +0 -0
index 292c0d9..185cf8e 100644

fs/btrfs/free-space-cache.c int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group, · line 2517

Quoted 10 lines
fs/btrfs/free-space-cache.c +5 -1
@@ -2509,8 +2509,15 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 			bytes = min(entry->bytes, end - start);
 			if (bytes < minlen)
 				goto next;
+
 			unlink_free_space(ctl, entry);
-			kmem_cache_free(btrfs_free_space_cachep, entry);
+			if (bytes < entry->bytes) {
+				entry->offset = entry->offset + bytes;
+				entry->bytes = entry->bytes - bytes;
+				link_free_space(ctl, entry);

yes, I forgot to link the rest extent to the free space cache. Thanks for the fix!

Quoted 8 lines
fs/btrfs/free-space-cache.c +3 -1
+			} else {
+				kmem_cache_free(btrfs_free_space_cachep, entry);
+			}
 		}
 
 		spin_unlock(&ctl->tree_lock);
-- 1.7.3.1 
#3 lore raw
In reply to Li Zefan
Quoted 3 lines · Li Zefan

We're taking a free space extent out of the free space cache, trimming it and then putting it back into the cache.

sorry for the late reply, I can hardly to find time look at this.

Quoted 2 lines

However for an extent that is smaller than the specified minimum length, it's taken out but won't be put back, which causes space leak.

yes, you are correct, and the fix looks good to me, Thanks

Quoted 87 lines

---

Unfortunately I have no trim-able device to test the patch.

---
 fs/btrfs/free-space-cache.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)
fs/btrfs/free-space-cache.c +17 -17
index 9f985a4..292c0d9 100644
@@ -2460,6 +2460,7 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 	u64 bytes = 0;
 	u64 actually_trimmed;
 	int ret = 0;
+	int update_ret;
 
 	*trimmed = 0;
 
@@ -2483,6 +2484,7 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 		}
 
 		if (entry->bitmap) {
+			bytes = 0;
 			ret = search_bitmap(ctl, entry, &start, &bytes);
 			if (!ret) {
 				if (start >= end) {
@@ -2490,6 +2492,8 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 					break;
 				}
 				bytes = min(bytes, end - start);
+				if (bytes < minlen)
+					goto next;
 				bitmap_clear_bits(ctl, entry, start, bytes);
 				if (entry->bytes == 0)
 					free_bitmap(ctl, entry);
@@ -2503,33 +2507,29 @@ int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
 		} else {
 			start = entry->offset;
 			bytes = min(entry->bytes, end - start);
+			if (bytes < minlen)
+				goto next;
 			unlink_free_space(ctl, entry);
 			kmem_cache_free(btrfs_free_space_cachep, entry);
 		}
 
 		spin_unlock(&ctl->tree_lock);
 
-		if (bytes >= minlen) {
-			int update_ret;
-			update_ret = btrfs_update_reserved_bytes(block_group,
-								 bytes, 1, 1);
+		update_ret = btrfs_update_reserved_bytes(block_group,
+							 bytes, 1, 1);
 
-			ret = btrfs_error_discard_extent(fs_info->extent_root,
-							 start,
-							 bytes,
-							 &actually_trimmed);
+		ret = btrfs_error_discard_extent(fs_info->extent_root, start,
+						 bytes, &actually_trimmed);
 
-			btrfs_add_free_space(block_group, start, bytes);
-			if (!update_ret)
-				btrfs_update_reserved_bytes(block_group,
-							    bytes, 0, 1);
+		btrfs_add_free_space(block_group, start, bytes);
+		if (!update_ret)
+			btrfs_update_reserved_bytes(block_group, bytes, 0, 1);
 
-			if (ret)
-				break;
-			*trimmed += actually_trimmed;
-		}
+		if (ret)
+			break;
+		*trimmed += actually_trimmed;
+next:
 		start += bytes;
-		bytes = 0;
 
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;