Btrfs: Batched discard support for btrfs

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Signed-off-by Li Dongyang <[email protected]>

---
 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81 ++++++++++++++++++++++++++++++++++++-------
 fs/btrfs/free-space-cache.c |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

fs/btrfs/ctree.h +2 -1

index 2c98b3a..4486349 100644
@@ -2217,7 +2217,8 @@ u64 btrfs_account_ro_block_groups_free_space(struct btrfs_space_info *sinfo);
 int btrfs_error_unpin_extent_range(struct btrfs_root *root,
 				   u64 start, u64 end);
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
-			       u64 num_bytes);
+			       u64 num_bytes, u64 *actual_bytes);
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range);
 
 /* ctree.c */
 int btrfs_bin_search(struct extent_buffer *eb, struct btrfs_key *key,

fs/btrfs/disk-io.c +4 -1

index e1aa8d6..bcb9451 100644
@@ -2947,7 +2947,10 @@ static int btrfs_destroy_pinned_extent(struct btrfs_root *root,
 			break;
 
 		/* opt_discard */
-		ret = btrfs_error_discard_extent(root, start, end + 1 - start);
+		if (btrfs_test_opt(root, DISCARD))
+			ret = btrfs_error_discard_extent(root, start,
+							 end + 1 - start,
+							 NULL);
 
 		clear_extent_dirty(unpin, start, end, GFP_NOFS);
 		btrfs_error_unpin_extent_range(root, start, end);

fs/btrfs/extent-tree.c +68 -13

index f3c96fc..7bed32a 100644
@@ -1740,22 +1740,20 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static void btrfs_issue_discard(struct block_device *bdev,
+static int btrfs_issue_discard(struct block_device *bdev,
 				u64 start, u64 len)
 {
-	blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
+	return blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
 }
 
 static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
-				u64 num_bytes)
+				u64 num_bytes, u64 *actual_bytes)
 {
 	int ret;
 	u64 map_length = num_bytes;
+	u64 discarded_bytes = 0;
 	struct btrfs_multi_bio *multi = NULL;
 
-	if (!btrfs_test_opt(root, DISCARD))
-		return 0;
-
 	/* Tell the block device(s) that the sectors can be discarded */
 	ret = btrfs_map_block(&root->fs_info->mapping_tree, READ,
 			      bytenr, &map_length, &multi, 0);

@@ -1767,13 +1765,25 @@ static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
 			map_length = num_bytes;
 
 		for (i = 0; i < multi->num_stripes; i++, stripe++) {
-			btrfs_issue_discard(stripe->dev->bdev,
-					    stripe->physical,
-					    map_length);
+			ret = btrfs_issue_discard(stripe->dev->bdev,
+						  stripe->physical,
+						  map_length);
+			if (!ret)
+				discarded_bytes += map_length;
+			else if (ret == -EOPNOTSUPP)
+				continue;
+			else
+				break;
 		}
 		kfree(multi);
 	}
 
+	if (discarded_bytes)
+		ret = 0;
+
+	if (actual_bytes)
+		*actual_bytes = discarded_bytes;
+
 	return ret;
 }
 

@@ -4353,7 +4363,8 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans,
 		if (ret)
 			break;
 
-		ret = btrfs_discard_extent(root, start, end + 1 - start);
+		if (btrfs_test_opt(root, DISCARD))
+			ret = btrfs_discard_extent(root, start, end + 1 - start, NULL);
 
 		clear_extent_dirty(unpin, start, end, GFP_NOFS);
 		unpin_extent_range(root, start, end);

@@ -5401,7 +5412,8 @@ int btrfs_free_reserved_extent(struct btrfs_root *root, u64 start, u64 len)
 		return -ENOSPC;
 	}
 
-	ret = btrfs_discard_extent(root, start, len);
+	if (btrfs_test_opt(root, DISCARD))
+		ret = btrfs_discard_extent(root, start, len, NULL);
 
 	btrfs_add_free_space(cache, start, len);
 	update_reserved_bytes(cache, len, 0, 1);

@@ -8712,7 +8724,50 @@ int btrfs_error_unpin_extent_range(struct btrfs_root *root, u64 start, u64 end)
 }
 
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
-			       u64 num_bytes)
+			       u64 num_bytes, u64 *actual_bytes)
+{
+	return btrfs_discard_extent(root, bytenr, num_bytes, actual_bytes);
+}
+
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
 {
-	return btrfs_discard_extent(root, bytenr, num_bytes);
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_block_group_cache *cache = NULL;
+	u64 cnt;
+	u64 start;
+	u64 end;
+	u64 trimmed = 0;
+	int ret = 0;
+
+	cache = btrfs_lookup_block_group(fs_info, range->start);
+
+	while (cache) {
+		if (cache->key.objectid >= (range->start + range->len)) {
+			btrfs_put_block_group(cache);
+			break;
+		}
+
+		start = max(range->start, cache->key.objectid);
+		end = min(range->start + range->len,
+				cache->key.objectid + cache->key.offset);
+
+		if (end - start >= range->minlen) {
+			ret = btrfs_trim_block_group(cache,
+						     &cnt,
+						     start,
+						     end,
+						     range->minlen);
+
+			trimmed += cnt;
+			if (ret < 0) {
+				btrfs_put_block_group(cache);
+				break;
+			}
+		}
+
+		cache = next_block_group(fs_info->tree_root, cache);
+	}
+
+	range->len = trimmed;
+	return ret;
 }

fs/btrfs/free-space-cache.c +79 -0

index a039065..a274df5 100644
@@ -2154,3 +2154,82 @@ 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)
+{
+	struct btrfs_free_space *entry = NULL;
+	struct btrfs_fs_info *fs_info = block_group->fs_info;
+	u64 bytes = 0;
+	u64 actually_trimmed;
+	int ret = 0;
+
+	*trimmed = 0;
+
+	while (start < end) {
+		spin_lock(&block_group->tree_lock);
+		if (block_group->free_space < minlen) {
+			spin_unlock(&block_group->tree_lock);
+			break;
+		}
+
+		entry = tree_search_offset(block_group, start, 0, 1);
+		if (!entry)
+			entry = tree_search_offset(block_group,
+						   offset_to_bitmap(block_group,
+								    start),
+						   1, 1);
+
+		if (!entry || entry->offset >= end) {
+			spin_unlock(&block_group->tree_lock);
+			break;
+		}
+
+		if (entry->bitmap) {
+			ret = search_bitmap(block_group, entry, &start, &bytes);
+			if (!ret) {
+				if (start >= end ) {
+					spin_unlock(&block_group->tree_lock);
+					break;
+				}
+				bytes = min(bytes, end - start);
+			} else {
+				start = entry->offset + BITS_PER_BITMAP *
+					block_group->sectorsize;
+				spin_unlock(&block_group->tree_lock);
+				continue;
+			}
+		} else {
+			start = entry->offset;
+			bytes = min(entry->bytes, end - start);
+		}
+
+		spin_unlock(&block_group->tree_lock);
+
+		if (bytes >= minlen && !btrfs_remove_free_space(block_group,
+								start,
+								bytes)) {
+			ret = btrfs_error_discard_extent(fs_info->extent_root,
+							 start,
+							 bytes,
+							 &actually_trimmed);
+
+			btrfs_add_free_space(block_group,
+					     start, bytes);
+			if (ret)
+				break;
+			*trimmed += actually_trimmed;
+		}
+		start += bytes;
+		bytes = 0;
+
+		if (fatal_signal_pending(current)) {
+			ret = -ERESTARTSYS;
+			break;
+		}
+
+		if (need_resched())
+			cond_resched();
+	}
+
+	return ret;
+}

fs/btrfs/free-space-cache.h +2 -0

index e49ca5c..65c3b93 100644
@@ -68,4 +68,6 @@ u64 btrfs_alloc_from_cluster(struct btrfs_block_group_cache *block_group,
 int btrfs_return_cluster_to_free_space(
 			       struct btrfs_block_group_cache *block_group,
 			       struct btrfs_free_cluster *cluster);
+int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
+			   u64 *trimmed, u64 start, u64 end, u64 minlen);
 #endif

fs/btrfs/ioctl.c +24 -0

index be2d4f6..ecd3982 100644
@@ -225,6 +225,28 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
 	return put_user(inode->i_generation, arg);
 }
 
+static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg)
+{
+	struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
+	struct fstrim_range range;
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(&range, arg, sizeof(range)))
+		return -EFAULT;
+
+	ret = btrfs_trim_fs(root, &range);
+	if (ret < 0)
+		return ret;
+
+	if (copy_to_user(arg, &range, sizeof(range)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static noinline int create_subvol(struct btrfs_root *root,
 				  struct dentry *dentry,
 				  char *name, int namelen,

@@ -2385,6 +2407,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_setflags(file, argp);
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
+	case FITRIM:
+		return btrfs_ioctl_fitrim(file, argp);
 	case BTRFS_IOC_SNAP_CREATE:
 		return btrfs_ioctl_snap_create(file, argp, 0);
 	case BTRFS_IOC_SNAP_CREATE_V2:

Signature · 6 lines

1.7.1

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

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Signed-off-by Li Dongyang <[email protected]>

---
 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81 ++++++++++++++++++++++++++++++++++++-------
 fs/btrfs/free-space-cache.c |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

fs/btrfs/free-space-cache.h +2 -0

index e49ca5c..65c3b93 100644
@@ -68,4 +68,6 @@ u64 btrfs_alloc_from_cluster(struct btrfs_block_group_cache *block_group,
 int btrfs_return_cluster_to_free_space(
 			       struct btrfs_block_group_cache *block_group,
 			       struct btrfs_free_cluster *cluster);
+int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
+			   u64 *trimmed, u64 start, u64 end, u64 minlen);
 #endif

fs/btrfs/ioctl.c +0 -0

index be2d4f6..ecd3982 100644
@@ -225,6 +225,28 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
 	return put_user(inode->i_generation, arg);

fs/btrfs/ioctl.c +16 -0

+
+	if (copy_from_user(&range, arg, sizeof(range)))
+		return -EFAULT;
+
+	ret = btrfs_trim_fs(root, &range);
+	if (ret < 0)
+		return ret;
+
+	if (copy_to_user(arg, &range, sizeof(range)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static noinline int create_subvol(struct btrfs_root *root,
 				  struct dentry *dentry,
 				  char *name, int namelen,

@@ -2385,6 +2407,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_setflags(file, argp);
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
+	case FITRIM:
+		return btrfs_ioctl_fitrim(file, argp);
 	case BTRFS_IOC_SNAP_CREATE:
 		return btrfs_ioctl_snap_create(file, argp, 0);
 	case BTRFS_IOC_SNAP_CREATE_V2:

Quoted 10 lines · Li Dongyang

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

Quoted 3 lines

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Hello,

First of all thanks for you effort:). I can not really comment on the btrfs specific code, however I have couple of comments bellow.

Btw, how did you test it ?

Thanks!
-Lukas

Quoted 11 lines

Signed-off-by Li Dongyang <[email protected]>

---

 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81
 ++++++++++++++++++++++++++++++++++++------- fs/btrfs/free-space-cache.c
 |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

..snip..

Quoted 30 lines

fs/btrfs/free-space-cache.h +2 -0

index e49ca5c..65c3b93 100644
@@ -68,4 +68,6 @@ u64 btrfs_alloc_from_cluster(struct
 btrfs_block_group_cache *block_group,
 
 int btrfs_return_cluster_to_free_space(
 
 			       struct btrfs_block_group_cache *block_group,
 			       struct btrfs_free_cluster *cluster);
 
+int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
+			   u64 *trimmed, u64 start, u64 end, u64 minlen);
 
 #endif

fs/btrfs/ioctl.c +1 -0

index be2d4f6..ecd3982 100644
@@ -225,6 +225,28 @@ static int btrfs_ioctl_getversion(struct file *file,
 int __user *arg)
 
 	return put_user(inode->i_generation, arg);
 
 }
 
+static noinline int btrfs_ioctl_fitrim(struct file *file, void __user

*arg) +{

fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 235

Quoted 6 lines

fs/btrfs/ioctl.c +6 -0

+	struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
+	struct fstrim_range range;
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;

You might want to check whether any of the underlying device does actually support trim and also adjust the minlen according to the discard_granularity.

Quoted 34 lines

fs/btrfs/ioctl.c +16 -0

+
+	if (copy_from_user(&range, arg, sizeof(range)))
+		return -EFAULT;
+
+	ret = btrfs_trim_fs(root, &range);
+	if (ret < 0)
+		return ret;
+
+	if (copy_to_user(arg, &range, sizeof(range)))
+		return -EFAULT;
+
+	return 0;
+}
+
 
 static noinline int create_subvol(struct btrfs_root *root,
 
 				  struct dentry *dentry,
 				  char *name, int namelen,

@@ -2385,6 +2407,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 
 		return btrfs_ioctl_setflags(file, argp);
 
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
 
+	case FITRIM:
+		return btrfs_ioctl_fitrim(file, argp);

fs/btrfs/ioctl.c +0 -0

 	case BTRFS_IOC_SNAP_CREATE:
 		return btrfs_ioctl_snap_create(file, argp, 0);
 
 	case BTRFS_IOC_SNAP_CREATE_V2:

Quoted 15 lines · Lukas Czerner

Quoted 13 lines · Li Dongyang

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Quoted 6 lines

Hello,

First of all thanks for you effort:). I can not really comment on the btrfs specific code, however I have couple of comments bellow.

Btw, how did you test it ?

Thanks for mentioning trim support checking and discard_granularity, they are added to V2, I fire fstrim from util-linux, and watch the trim command by blktrace.

Quoted 93 lines

Thanks!
-Lukas

Quoted 11 lines

Signed-off-by Li Dongyang <[email protected]>

---

 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81
 ++++++++++++++++++++++++++++++++++++------- fs/btrfs/free-space-cache.c
 |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

..snip..

Quoted 30 lines

fs/btrfs/free-space-cache.h +2 -0

index e49ca5c..65c3b93 100644
@@ -68,4 +68,6 @@ u64 btrfs_alloc_from_cluster(struct
 btrfs_block_group_cache *block_group,
 
 int btrfs_return_cluster_to_free_space(
 
 			       struct btrfs_block_group_cache *block_group,
 			       struct btrfs_free_cluster *cluster);
 
+int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
+			   u64 *trimmed, u64 start, u64 end, u64 minlen);
 
 #endif

fs/btrfs/ioctl.c +1 -0

index be2d4f6..ecd3982 100644
@@ -225,6 +225,28 @@ static int btrfs_ioctl_getversion(struct file *file,
 int __user *arg)
 
 	return put_user(inode->i_generation, arg);
 
 }
 
+static noinline int btrfs_ioctl_fitrim(struct file *file, void __user

*arg) +{

fs/btrfs/ioctl.c static int btrfs_ioctl_getversion(struct file *file, int __user *arg) · line 235

Quoted 6 lines

fs/btrfs/ioctl.c +6 -0

+	struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
+	struct fstrim_range range;
+	int ret;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;

You might want to check whether any of the underlying device does actually support trim and also adjust the minlen according to the discard_granularity.

Quoted 34 lines

fs/btrfs/ioctl.c +16 -0

+
+	if (copy_from_user(&range, arg, sizeof(range)))
+		return -EFAULT;
+
+	ret = btrfs_trim_fs(root, &range);
+	if (ret < 0)
+		return ret;
+
+	if (copy_to_user(arg, &range, sizeof(range)))
+		return -EFAULT;
+
+	return 0;
+}
+
 
 static noinline int create_subvol(struct btrfs_root *root,
 
 				  struct dentry *dentry,
 				  char *name, int namelen,

@@ -2385,6 +2407,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 
 		return btrfs_ioctl_setflags(file, argp);
 
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
 
+	case FITRIM:
+		return btrfs_ioctl_fitrim(file, argp);

fs/btrfs/ioctl.c +0 -0

 	case BTRFS_IOC_SNAP_CREATE:
 		return btrfs_ioctl_snap_create(file, argp, 0);
 
 	case BTRFS_IOC_SNAP_CREATE_V2:

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Signed-off-by Li Dongyang <[email protected]>

---
 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81 ++++++++++++++++++++++++++++++++++++-------
 fs/btrfs/free-space-cache.c |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

fs/btrfs/ctree.h +2 -1

index 2c98b3a..4486349 100644
@@ -2217,7 +2217,8 @@ u64 btrfs_account_ro_block_groups_free_space(struct btrfs_space_info *sinfo);
 int btrfs_error_unpin_extent_range(struct btrfs_root *root,
                    u64 start, u64 end);
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
-                   u64 num_bytes);
+                   u64 num_bytes, u64 *actual_bytes);
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range);
 
 /* ctree.c */
 int btrfs_bin_search(struct extent_buffer *eb, struct btrfs_key *key,

fs/btrfs/disk-io.c +4 -1

index e1aa8d6..bcb9451 100644
@@ -2947,7 +2947,10 @@ static int btrfs_destroy_pinned_extent(struct btrfs_root *root,
             break;
 
         /* opt_discard */
-        ret = btrfs_error_discard_extent(root, start, end + 1 - start);
+        if (btrfs_test_opt(root, DISCARD))
+            ret = btrfs_error_discard_extent(root, start,
+                             end + 1 - start,
+                             NULL);
 
         clear_extent_dirty(unpin, start, end, GFP_NOFS);
         btrfs_error_unpin_extent_range(root, start, end);

fs/btrfs/extent-tree.c +68 -13

index f3c96fc..7bed32a 100644
@@ -1740,22 +1740,20 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
     return ret;
 }
 
-static void btrfs_issue_discard(struct block_device *bdev,
+static int btrfs_issue_discard(struct block_device *bdev,
                 u64 start, u64 len)
 {
-    blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
+    return blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
 }
 
 static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
-                u64 num_bytes)
+                u64 num_bytes, u64 *actual_bytes)
 {
     int ret;
     u64 map_length = num_bytes;
+    u64 discarded_bytes = 0;
     struct btrfs_multi_bio *multi = NULL;
 
-    if (!btrfs_test_opt(root, DISCARD))
-        return 0;
-
     /* Tell the block device(s) that the sectors can be discarded */
     ret = btrfs_map_block(&root->fs_info->mapping_tree, READ,
                   bytenr, &map_length, &multi, 0);

@@ -1767,13 +1765,25 @@ static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
             map_length = num_bytes;
 
         for (i = 0; i < multi->num_stripes; i++, stripe++) {
-            btrfs_issue_discard(stripe->dev->bdev,
-                        stripe->physical,
-                        map_length);
+            ret = btrfs_issue_discard(stripe->dev->bdev,
+                          stripe->physical,
+                          map_length);
+            if (!ret)
+                discarded_bytes += map_length;
+            else if (ret == -EOPNOTSUPP)
+                continue;
+            else
+                break;
         }
         kfree(multi);
     }
 
+    if (discarded_bytes)
+        ret = 0;
+
+    if (actual_bytes)
+        *actual_bytes = discarded_bytes;
+
     return ret;
 }
 

@@ -4353,7 +4363,8 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans,
         if (ret)
             break;
 
-        ret = btrfs_discard_extent(root, start, end + 1 - start);
+        if (btrfs_test_opt(root, DISCARD))
+            ret = btrfs_discard_extent(root, start, end + 1 - start, NULL);
 
         clear_extent_dirty(unpin, start, end, GFP_NOFS);
         unpin_extent_range(root, start, end);

@@ -5401,7 +5412,8 @@ int btrfs_free_reserved_extent(struct btrfs_root *root, u64 start, u64 len)
         return -ENOSPC;
     }
 
-    ret = btrfs_discard_extent(root, start, len);
+    if (btrfs_test_opt(root, DISCARD))
+        ret = btrfs_discard_extent(root, start, len, NULL);
 
     btrfs_add_free_space(cache, start, len);
     update_reserved_bytes(cache, len, 0, 1);

@@ -8712,7 +8724,50 @@ int btrfs_error_unpin_extent_range(struct btrfs_root *root, u64 start, u64 end)
 }
 
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
-                   u64 num_bytes)
+                   u64 num_bytes, u64 *actual_bytes)
+{
+    return btrfs_discard_extent(root, bytenr, num_bytes, actual_bytes);
+}
+
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
 {
-    return btrfs_discard_extent(root, bytenr, num_bytes);
+    struct btrfs_fs_info *fs_info = root->fs_info;
+    struct btrfs_block_group_cache *cache = NULL;
+    u64 cnt;
+    u64 start;
+    u64 end;
+    u64 trimmed = 0;
+    int ret = 0;
+
+    cache = btrfs_lookup_block_group(fs_info, range->start);
+
+    while (cache) {
+        if (cache->key.objectid >= (range->start + range->len)) {
+            btrfs_put_block_group(cache);
+            break;
+        }
+
+        start = max(range->start, cache->key.objectid);
+        end = min(range->start + range->len,
+                cache->key.objectid + cache->key.offset);
+
+        if (end - start >= range->minlen) {
+            ret = btrfs_trim_block_group(cache,
+                             &cnt,
+                             start,
+                             end,
+                             range->minlen);
+
+            trimmed += cnt;
+            if (ret < 0) {
+                btrfs_put_block_group(cache);
+                break;
+            }
+        }
+
+        cache = next_block_group(fs_info->tree_root, cache);
+    }
+
+    range->len = trimmed;
+    return ret;
 }

fs/btrfs/free-space-cache.c +79 -0

index a039065..a274df5 100644
@@ -2154,3 +2154,82 @@ 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)
+{
+    struct btrfs_free_space *entry = NULL;
+    struct btrfs_fs_info *fs_info = block_group->fs_info;
+    u64 bytes = 0;
+    u64 actually_trimmed;
+    int ret = 0;
+
+    *trimmed = 0;
+
+    while (start < end) {
+        spin_lock(&block_group->tree_lock);
+        if (block_group->free_space < minlen) {
+            spin_unlock(&block_group->tree_lock);
+            break;
+        }
+
+        entry = tree_search_offset(block_group, start, 0, 1);
+        if (!entry)
+            entry = tree_search_offset(block_group,
+                           offset_to_bitmap(block_group,
+                                    start),
+                           1, 1);
+
+        if (!entry || entry->offset >= end) {
+            spin_unlock(&block_group->tree_lock);
+            break;
+        }
+
+        if (entry->bitmap) {
+            ret = search_bitmap(block_group, entry, &start, &bytes);
+            if (!ret) {
+                if (start >= end ) {
+                    spin_unlock(&block_group->tree_lock);
+                    break;
+                }
+                bytes = min(bytes, end - start);
+            } else {
+                start = entry->offset + BITS_PER_BITMAP *
+                    block_group->sectorsize;
+                spin_unlock(&block_group->tree_lock);
+                continue;
+            }
+        } else {
+            start = entry->offset;
+            bytes = min(entry->bytes, end - start);
+        }
+
+        spin_unlock(&block_group->tree_lock);
+
+        if (bytes >= minlen && !btrfs_remove_free_space(block_group,
+                                start,
+                                bytes)) {
+            ret = btrfs_error_discard_extent(fs_info->extent_root,
+                             start,
+                             bytes,
+                             &actually_trimmed);
+
+            btrfs_add_free_space(block_group,
+                         start, bytes);
+            if (ret)
+                break;
+            *trimmed += actually_trimmed;
+        }
+        start += bytes;
+        bytes = 0;
+
+        if (fatal_signal_pending(current)) {
+            ret = -ERESTARTSYS;
+            break;
+        }
+
+        if (need_resched())
+            cond_resched();
+    }
+
+    return ret;
+}

fs/btrfs/free-space-cache.h +2 -0

index e49ca5c..65c3b93 100644
@@ -68,4 +68,6 @@ u64 btrfs_alloc_from_cluster(struct btrfs_block_group_cache *block_group,
 int btrfs_return_cluster_to_free_space(
                    struct btrfs_block_group_cache *block_group,
                    struct btrfs_free_cluster *cluster);
+int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
+               u64 *trimmed, u64 start, u64 end, u64 minlen);
 #endif

fs/btrfs/ioctl.c +24 -0

index be2d4f6..ecd3982 100644
@@ -225,6 +225,28 @@ static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
     return put_user(inode->i_generation, arg);
 }
 
+static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg)
+{
+    struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
+    struct fstrim_range range;
+    int ret;
+
+    if (!capable(CAP_SYS_ADMIN))
+        return -EPERM;
+
+    if (copy_from_user(&range, arg, sizeof(range)))
+        return -EFAULT;
+
+    ret = btrfs_trim_fs(root, &range);
+    if (ret < 0)
+        return ret;
+
+    if (copy_to_user(arg, &range, sizeof(range)))
+        return -EFAULT;
+
+    return 0;
+}
+
 static noinline int create_subvol(struct btrfs_root *root,
                   struct dentry *dentry,
                   char *name, int namelen,

@@ -2385,6 +2407,8 @@ long btrfs_ioctl(struct file *file, unsigned int
         return btrfs_ioctl_setflags(file, argp);
     case FS_IOC_GETVERSION:
         return btrfs_ioctl_getversion(file, argp);
+    case FITRIM:
+        return btrfs_ioctl_fitrim(file, argp);
     case BTRFS_IOC_SNAP_CREATE:
         return btrfs_ioctl_snap_create(file, argp, 0);
     case BTRFS_IOC_SNAP_CREATE_V2:

Excerpts from Li Dongyang's message of 2011-02-21 03:52:27 -0500:

Quoted 9 lines

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

This is really cool, thanks for doing it.

Quoted 3 lines

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Nice catch, could you please send a tested patch for that as well?

In terms of allocator interaction, this looks pretty good to me. You're taking free space out of the allocator, trimming it and then putting it back in.

But, you're not updating the reserved extent calculations as you go, so the enospc code is going to get upset when space gets tight.

One other tiny tiny nit:

if (need_resched())
cond_resched()

I'm guessing there used to be more here ;) cond_resched() alone is enough.

In terms of future improvements on top of the patch, we have a flag on the device struct to say if it can support barriers, it makes sense to add one for devices supporting discard too.

You could add mount -o discard_lazy that runs through and trims active block groups every N transactions (100? 1000?). You wouldn't need to keep track of the last discard, just if (transid % 1000 == 0) do it. Or if you want to get fancy you can factor in a hash of the objectid of the block group so that we don't trim all the block groups at once.

The part I don't know from a hardware point of view is how big the penalty is for discarding a range that has already been discarded.

One thing btrfs is missing from a thin provisioning point of view is a balancing ioctl that just tries to reclaim partially empty block groups, and then trims the freed bytes.

-chris

Quoted 384 lines

Signed-off-by Li Dongyang <[email protected]>

---

 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81
 ++++++++++++++++++++++++++++++++++++------- fs/btrfs/free-space-cache.c
 |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

fs/btrfs/ctree.h +2 -1

index 2c98b3a..4486349 100644
@@ -2217,7 +2217,8 @@ u64 btrfs_account_ro_block_groups_free_space(struct
 btrfs_space_info *sinfo);
 
 int btrfs_error_unpin_extent_range(struct btrfs_root *root,
 
                    u64 start, u64 end);
 
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
 
-                   u64 num_bytes);
+                   u64 num_bytes, u64 *actual_bytes);
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range);
 
 /* ctree.c */
 int btrfs_bin_search(struct extent_buffer *eb, struct btrfs_key *key,

fs/btrfs/disk-io.c +4 -1

index e1aa8d6..bcb9451 100644
@@ -2947,7 +2947,10 @@ static int btrfs_destroy_pinned_extent(struct
 btrfs_root *root,
 
             break;
 
         /* opt_discard */
 
-        ret = btrfs_error_discard_extent(root, start, end + 1 - start);
+        if (btrfs_test_opt(root, DISCARD))
+            ret = btrfs_error_discard_extent(root, start,
+                             end + 1 - start,
+                             NULL);

         clear_extent_dirty(unpin, start, end, GFP_NOFS);
         btrfs_error_unpin_extent_range(root, start, end);

fs/btrfs/extent-tree.c +21 -10

index f3c96fc..7bed32a 100644
@@ -1740,22 +1740,20 @@ static int remove_extent_backref(struct
 btrfs_trans_handle *trans,
 
     return ret;
 
 }
 
-static void btrfs_issue_discard(struct block_device *bdev,
+static int btrfs_issue_discard(struct block_device *bdev,
 
                 u64 start, u64 len)
 
 {
 
-    blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
+    return blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL,
 0);
 
 }
 
 static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
 
-                u64 num_bytes)
+                u64 num_bytes, u64 *actual_bytes)
 
 {
 
     int ret;
     u64 map_length = num_bytes;
 
+    u64 discarded_bytes = 0;
 
     struct btrfs_multi_bio *multi = NULL;
 
-    if (!btrfs_test_opt(root, DISCARD))
-        return 0;
-
 
     /* Tell the block device(s) that the sectors can be discarded */
     ret = btrfs_map_block(&root->fs_info->mapping_tree, READ,
 
                   bytenr, &map_length, &multi, 0);

@@ -1767,13 +1765,25 @@ static int btrfs_discard_extent(struct btrfs_root
 *root, u64 bytenr,
 
             map_length = num_bytes;
 
         for (i = 0; i < multi->num_stripes; i++, stripe++) {
 
-            btrfs_issue_discard(stripe->dev->bdev,
-                        stripe->physical,
-                        map_length);
+            ret = btrfs_issue_discard(stripe->dev->bdev,
+                          stripe->physical,
+                          map_length);
+            if (!ret)
+                discarded_bytes += map_length;
+            else if (ret == -EOPNOTSUPP)
+                continue;
+            else
+                break;
 
         }
         kfree(multi);
 
     }
 
+    if (discarded_bytes)
+        ret = 0;
+
+    if (actual_bytes)
+        *actual_bytes = discarded_bytes;
+
 
     return ret;
 
 }

@@ -4353,7 +4363,8 @@ int btrfs_finish_extent_commit(struct
 btrfs_trans_handle *trans,
 
         if (ret)
 
             break;
 
-        ret = btrfs_discard_extent(root, start, end + 1 - start);
+        if (btrfs_test_opt(root, DISCARD))
+            ret = btrfs_discard_extent(root, start, end + 1 - start,

NULL);

         clear_extent_dirty(unpin, start, end, GFP_NOFS);
         unpin_extent_range(root, start, end);

fs/btrfs/extent-tree.c +2 -1

@@ -5401,7 +5412,8 @@ int btrfs_free_reserved_extent(struct btrfs_root
 *root, u64 start, u64 len)
 
         return -ENOSPC;
 
     }
 
-    ret = btrfs_discard_extent(root, start, len);
+    if (btrfs_test_opt(root, DISCARD))
+        ret = btrfs_discard_extent(root, start, len, NULL);

     btrfs_add_free_space(cache, start, len);
     update_reserved_bytes(cache, len, 0, 1);

fs/btrfs/extent-tree.c +45 -2

@@ -8712,7 +8724,50 @@ int btrfs_error_unpin_extent_range(struct
 btrfs_root *root, u64 start, u64 end)
 
 }
 
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
 
-                   u64 num_bytes)
+                   u64 num_bytes, u64 *actual_bytes)
+{
+    return btrfs_discard_extent(root, bytenr, num_bytes, actual_bytes);
+}
+
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
 
 {
 
-    return btrfs_discard_extent(root, bytenr, num_bytes);
+    struct btrfs_fs_info *fs_info = root->fs_info;
+    struct btrfs_block_group_cache *cache = NULL;
+    u64 cnt;
+    u64 start;
+    u64 end;
+    u64 trimmed = 0;
+    int ret = 0;
+
+    cache = btrfs_lookup_block_group(fs_info, range->start);
+
+    while (cache) {
+        if (cache->key.objectid >= (range->start + range->len)) {
+            btrfs_put_block_group(cache);
+            break;
+        }
+
+        start = max(range->start, cache->key.objectid);
+        end = min(range->start + range->len,
+                cache->key.objectid + cache->key.offset);
+
+        if (end - start >= range->minlen) {
+            ret = btrfs_trim_block_group(cache,
+                             &cnt,
+                             start,
+                             end,
+                             range->minlen);
+
+            trimmed += cnt;
+            if (ret < 0) {
+                btrfs_put_block_group(cache);
+                break;
+            }
+        }
+
+        cache = next_block_group(fs_info->tree_root, cache);
+    }
+
+    range->len = trimmed;
+    return ret;
 
 }

fs/btrfs/free-space-cache.c +79 -0

index a039065..a274df5 100644
@@ -2154,3 +2154,82 @@ 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)
+{
+    struct btrfs_free_space *entry = NULL;
+    struct btrfs_fs_info *fs_info = block_group->fs_info;
+    u64 bytes = 0;
+    u64 actually_trimmed;
+    int ret = 0;
+
+    *trimmed = 0;
+
+    while (start < end) {
+        spin_lock(&block_group->tree_lock);
+        if (block_group->free_space < minlen) {
+            spin_unlock(&block_group->tree_lock);
+            break;
+        }
+
+        entry = tree_search_offset(block_group, start, 0, 1);
+        if (!entry)
+            entry = tree_search_offset(block_group,
+                           offset_to_bitmap(block_group,
+                                    start),
+                           1, 1);
+
+        if (!entry || entry->offset >= end) {
+            spin_unlock(&block_group->tree_lock);
+            break;
+        }
+
+        if (entry->bitmap) {
+            ret = search_bitmap(block_group, entry, &start, &bytes);
+            if (!ret) {
+                if (start >= end ) {
+                    spin_unlock(&block_group->tree_lock);
+                    break;
+                }
+                bytes = min(bytes, end - start);
+            } else {
+                start = entry->offset + BITS_PER_BITMAP *
+                    block_group->sectorsize;
+                spin_unlock(&block_group->tree_lock);
+                continue;
+            }
+        } else {
+            start = entry->offset;
+            bytes = min(entry->bytes, end - start);
+        }
+
+        spin_unlock(&block_group->tree_lock);
+
+        if (bytes >= minlen && !btrfs_remove_free_space(block_group,
+                                start,
+                                bytes)) {
+            ret = btrfs_error_discard_extent(fs_info->extent_root,
+                             start,
+                             bytes,
+                             &actually_trimmed);
+
+            btrfs_add_free_space(block_group,
+                         start, bytes);
+            if (ret)
+                break;
+            *trimmed += actually_trimmed;
+        }
+        start += bytes;
+        bytes = 0;
+
+        if (fatal_signal_pending(current)) {
+            ret = -ERESTARTSYS;
+            break;
+        }
+
+        if (need_resched())
+            cond_resched();
+    }
+
+    return ret;
+}

fs/btrfs/free-space-cache.h +2 -0

index e49ca5c..65c3b93 100644
@@ -68,4 +68,6 @@ u64 btrfs_alloc_from_cluster(struct
 btrfs_block_group_cache *block_group,
 
 int btrfs_return_cluster_to_free_space(
 
                    struct btrfs_block_group_cache *block_group,
                    struct btrfs_free_cluster *cluster);
 
+int btrfs_trim_block_group(struct btrfs_block_group_cache *block_group,
+               u64 *trimmed, u64 start, u64 end, u64 minlen);
 
 #endif

fs/btrfs/ioctl.c +1 -0

index be2d4f6..ecd3982 100644
@@ -225,6 +225,28 @@ static int btrfs_ioctl_getversion(struct file *file,
 int __user *arg)
 
     return put_user(inode->i_generation, arg);
 
 }
 
+static noinline int btrfs_ioctl_fitrim(struct file *file, void __user

*arg) +{

fs/btrfs/ioctl.c +22 -0

+    struct btrfs_root *root = fdentry(file)->d_sb->s_fs_info;
+    struct fstrim_range range;
+    int ret;
+
+    if (!capable(CAP_SYS_ADMIN))
+        return -EPERM;
+
+    if (copy_from_user(&range, arg, sizeof(range)))
+        return -EFAULT;
+
+    ret = btrfs_trim_fs(root, &range);
+    if (ret < 0)
+        return ret;
+
+    if (copy_to_user(arg, &range, sizeof(range)))
+        return -EFAULT;
+
+    return 0;
+}
+
 
 static noinline int create_subvol(struct btrfs_root *root,
 
                   struct dentry *dentry,
                   char *name, int namelen,

@@ -2385,6 +2407,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 
         return btrfs_ioctl_setflags(file, argp);
 
     case FS_IOC_GETVERSION:
         return btrfs_ioctl_getversion(file, argp);
 
+    case FITRIM:
+        return btrfs_ioctl_fitrim(file, argp);

     case BTRFS_IOC_SNAP_CREATE:
         return btrfs_ioctl_snap_create(file, argp, 0);
     
     case BTRFS_IOC_SNAP_CREATE_V2:

Here is batched discard support for btrfs, several changes were made:

btrfs_test_opt(root, DISCARD) is moved from btrfs_discard_extent to callers, as we still want to trim the fs even it's not mounted with -o discard.
btrfs_discard_extent now reports errors and actual bytes trimmed to callers, for EOPNOTSUPP, we will try other stripes as an extent could span SSD and other drives, and we won't return error to callers unless we failed with all stripes.

And btrfs_discard_extent calls btrfs_map_block with READ, this means we won't get all stripes mapped for RAID1/DUP/RAID10, I think this should be fixed, Thanks.

Signed-off-by Li Dongyang <[email protected]>

---
 fs/btrfs/ctree.h            |    3 +-
 fs/btrfs/disk-io.c          |    5 ++-
 fs/btrfs/extent-tree.c      |   81 ++++++++++++++++++++++++++++++++++++-------
 fs/btrfs/free-space-cache.c |   79 +++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/free-space-cache.h |    2 +
 fs/btrfs/ioctl.c            |   24 +++++++++++++
 6 files changed, 179 insertions(+), 15 deletions(-)

fs/btrfs/ctree.h +2 -1

index 2c98b3a..4486349 100644
@@ -2217,7 +2217,8 @@ u64 btrfs_account_ro_block_groups_free_space(struct btrfs_space_info *sinfo);
 int btrfs_error_unpin_extent_range(struct btrfs_root *root,
 				   u64 start, u64 end);
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
-			       u64 num_bytes);
+			       u64 num_bytes, u64 *actual_bytes);
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range);
 
 /* ctree.c */
 int btrfs_bin_search(struct extent_buffer *eb, struct btrfs_key *key,

fs/btrfs/disk-io.c +4 -1

index e1aa8d6..bcb9451 100644
@@ -2947,7 +2947,10 @@ static int btrfs_destroy_pinned_extent(struct btrfs_root *root,
 			break;
 
 		/* opt_discard */
-		ret = btrfs_error_discard_extent(root, start, end + 1 - start);
+		if (btrfs_test_opt(root, DISCARD))
+			ret = btrfs_error_discard_extent(root, start,
+							 end + 1 - start,
+							 NULL);
 
 		clear_extent_dirty(unpin, start, end, GFP_NOFS);
 		btrfs_error_unpin_extent_range(root, start, end);

fs/btrfs/extent-tree.c +68 -13

index f3c96fc..7bed32a 100644
@@ -1740,22 +1740,20 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static void btrfs_issue_discard(struct block_device *bdev,
+static int btrfs_issue_discard(struct block_device *bdev,
 				u64 start, u64 len)
 {
-	blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
+	return blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
 }
 
 static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
-				u64 num_bytes)
+				u64 num_bytes, u64 *actual_bytes)
 {
 	int ret;
 	u64 map_length = num_bytes;
+	u64 discarded_bytes = 0;
 	struct btrfs_multi_bio *multi = NULL;
 
-	if (!btrfs_test_opt(root, DISCARD))
-		return 0;
-
 	/* Tell the block device(s) that the sectors can be discarded */
 	ret = btrfs_map_block(&root->fs_info->mapping_tree, READ,
 			      bytenr, &map_length, &multi, 0);

@@ -1767,13 +1765,25 @@ static int btrfs_discard_extent(struct btrfs_root *root, u64 bytenr,
 			map_length = num_bytes;
 
 		for (i = 0; i < multi->num_stripes; i++, stripe++) {
-			btrfs_issue_discard(stripe->dev->bdev,
-					    stripe->physical,
-					    map_length);
+			ret = btrfs_issue_discard(stripe->dev->bdev,
+						  stripe->physical,
+						  map_length);
+			if (!ret)
+				discarded_bytes += map_length;
+			else if (ret == -EOPNOTSUPP)
+				continue;
+			else
+				break;
 		}
 		kfree(multi);
 	}
 
+	if (discarded_bytes)
+		ret = 0;
+
+	if (actual_bytes)
+		*actual_bytes = discarded_bytes;
+
 	return ret;
 }
 

@@ -4353,7 +4363,8 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans,
 		if (ret)
 			break;
 
-		ret = btrfs_discard_extent(root, start, end + 1 - start);
+		if (btrfs_test_opt(root, DISCARD))
+			ret = btrfs_discard_extent(root, start, end + 1 - start, NULL);
 
 		clear_extent_dirty(unpin, start, end, GFP_NOFS);
 		unpin_extent_range(root, start, end);

@@ -5401,7 +5412,8 @@ int btrfs_free_reserved_extent(struct btrfs_root *root, u64 start, u64 len)
 		return -ENOSPC;
 	}
 
-	ret = btrfs_discard_extent(root, start, len);
+	if (btrfs_test_opt(root, DISCARD))
+		ret = btrfs_discard_extent(root, start, len, NULL);
 
 	btrfs_add_free_space(cache, start, len);
 	update_reserved_bytes(cache, len, 0, 1);

@@ -8712,7 +8724,50 @@ int btrfs_error_unpin_extent_range(struct btrfs_root *root, u64 start, u64 end)
 }
 
 int btrfs_error_discard_extent(struct btrfs_root *root, u64 bytenr,
-			       u64 num_bytes)
+			       u64 num_bytes, u64 *actual_bytes)
+{
+	return btrfs_discard_extent(root, bytenr, num_bytes, actual_bytes);
+}
+
+int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
 {
-	return btrfs_discard_extent(root, bytenr, num_bytes);
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_block_group_cache *cache = NULL;
+	u64 cnt;
+	u64 start;
+	u64 end;
+	u64 trimmed = 0;
+	int ret = 0;
+
+	cache = btrfs_lookup_block_group(fs_info, range->start);
+
+	while (cache) {
+		if (cache->key.objectid >= (range->start + range->len)) {
+			btrfs_put_block_group(cache);
+			break;
+		}
+
+		start = max(range->start, cache->key.objectid);
+		end = min(range->start + range->len,
+				cache->key.objectid + cache->key.offset);
+
+		if (end - start >= range->minlen) {
+			ret = btrfs_trim_block_group(cache,
+						     &cnt,
+						     start,
+						     end,
+						     range->minlen);
+
+			trimmed += cnt;
+			if (ret < 0) {
+				btrfs_put_block_group(cache);
+				break;
+			}
+		}
+
+		cache = next_block_group(fs_info->tree_root, cache);
+	}
+
+	range->len = trimmed;
+	return ret;
 }

fs/btrfs/free-space-cache.c +44 -0

index a039065..a274df5 100644
@@ -2154,3 +2154,82 @@ 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)
+{
+	struct btrfs_free_space *entry = NULL;
+	struct btrfs_fs_info *fs_info = block_group->fs_info;
+	u64 bytes = 0;
+	u64 actually_trimmed;
+	int ret = 0;
+
+	*trimmed = 0;
+
+	while (start < end) {
+		spin_lock(&block_group->tree_lock);
+		if (block_group->free_space < minlen) {
+			spin_unlock(&block_group->tree_lock);
+			break;
+		}
+
+		entry = tree_search_offset(block_group, start, 0, 1);
+		if (!entry)
+			entry = tree_search_offset(block_group,
+						   offset_to_bitmap(block_group,
+								    start),
+						   1, 1);
+
+		if (!entry || entry->offset >= end) {
+			spin_unlock(&block_group->tree_lock);
+			break;
+		}
+
+		if (entry->bitmap) {
+			ret = search_bitmap(block_group, entry, &start, &bytes);
+			if (!ret) {
+				if (start >= end ) {
+					spin_unlock(&block_group->tree_lock);
+					break;
+				}
+				bytes = min(bytes, end - start);
+			} else {
+				start = entry->offset + BITS_PER_BITMAP *
+					block_group->sectorsize;
+				spin_unlock(&block_group->tree_lock);
+				continue;
+			}

So you have just done 2 searches for the same thing. Instead of doing btrfs_remove_free_space here, put

bitmap_clear_bits()

after the search_bitmap, and do a

unlink_free_space()

if it's not a bitmap, that way we don't race with the allocator (even though this isn't a problem, it just sucks) and we don't do the search twice. Other than that it looks great, thank you,

Josef