[01/15] btrfs: Honour FITRIM range constraints during free space trim
diff mbox series

Message ID 20190130145102.4708-2-nborisov@suse.com
State New
Headers show
Series
  • Improvements to fitrim
Related show

Commit Message

Nikolay Borisov Jan. 30, 2019, 2:50 p.m. UTC
Up until know trimming the freespace was done irrespective of what the
arguments of the FITRIM ioctl were. For example fstrim's -o/-l arguments
will be entirely ignored. Fix it by correctly handling those paramter.
This requires breaking if the found freespace extent is after the end
of the passed range as well as completing trim after trimming
fstrim_range::len bytes.

Fixes: 499f377f49f0 ("btrfs: iterate over unused chunk space in FITRIM")
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/extent-tree.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

Comments

David Sterba Jan. 31, 2019, 3:21 p.m. UTC | #1
On Wed, Jan 30, 2019 at 04:50:48PM +0200, Nikolay Borisov wrote:
> Up until know trimming the freespace was done irrespective of what the
> arguments of the FITRIM ioctl were. For example fstrim's -o/-l arguments
> will be entirely ignored. Fix it by correctly handling those paramter.
> This requires breaking if the found freespace extent is after the end
> of the passed range as well as completing trim after trimming
> fstrim_range::len bytes.

How does this work with with multipe-device filesystems? The fstrim
range would apply to all of them. Which does make some sense, though
might be unexpected as this does not happen for other filesystems.

The FITRIM range is in the physical coordinates, so eg. the taking the
maximum size of all devices and iterating over that by 1GiB steps would
go though the whole filesystem. Something to put to the changelog and
documentation.

> Fixes: 499f377f49f0 ("btrfs: iterate over unused chunk space in FITRIM")
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/extent-tree.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 1fdba38761f7..fc3f6acc3c9b 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -11190,9 +11190,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
>   * held back allocations.
>   */
>  static int btrfs_trim_free_extents(struct btrfs_device *device,
> -				   u64 minlen, u64 *trimmed)
> +				   struct fstrim_range *range, u64 *trimmed)
>  {
> -	u64 start = 0, len = 0;
> +	u64 start = range->start, len = 0;
>  	int ret;
>  
>  	*trimmed = 0;
> @@ -11235,8 +11235,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>  		if (!trans)
>  			up_read(&fs_info->commit_root_sem);
>  
> -		ret = find_free_dev_extent_start(trans, device, minlen, start,
> -						 &start, &len);
> +		ret = find_free_dev_extent_start(trans, device, range->minlen,
> +						 start, &start, &len);
>  		if (trans) {
>  			up_read(&fs_info->commit_root_sem);
>  			btrfs_put_transaction(trans);
> @@ -11249,6 +11249,16 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>  			break;
>  		}
>  
> +		/* If we are out of the passed range break */
> +		if (start > range->start + range->len - 1) {
> +			mutex_unlock(&fs_info->chunk_mutex);
> +			ret = 0;
> +			break;
> +		}
> +
> +		start = max(range->start, start);
> +		len = min(range->len, len);
> +
>  		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
>  		mutex_unlock(&fs_info->chunk_mutex);
>  
> @@ -11258,6 +11268,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>  		start += len;
>  		*trimmed += bytes;
>  
> +		/* We've trimmed enough */
> +		if (*trimmed >= range->len)
> +			break;
> +
>  		if (fatal_signal_pending(current)) {
>  			ret = -ERESTARTSYS;
>  			break;
> @@ -11341,8 +11355,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>  	mutex_lock(&fs_info->fs_devices->device_list_mutex);
>  	devices = &fs_info->fs_devices->devices;
>  	list_for_each_entry(device, devices, dev_list) {
> -		ret = btrfs_trim_free_extents(device, range->minlen,
> -					      &group_trimmed);
> +		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
>  		if (ret) {
>  			dev_failed++;
>  			dev_ret = ret;
> -- 
> 2.17.1
Nikolay Borisov Jan. 31, 2019, 3:35 p.m. UTC | #2
On 31.01.19 г. 17:21 ч., David Sterba wrote:
> On Wed, Jan 30, 2019 at 04:50:48PM +0200, Nikolay Borisov wrote:
>> Up until know trimming the freespace was done irrespective of what the
>> arguments of the FITRIM ioctl were. For example fstrim's -o/-l arguments
>> will be entirely ignored. Fix it by correctly handling those paramter.
>> This requires breaking if the found freespace extent is after the end
>> of the passed range as well as completing trim after trimming
>> fstrim_range::len bytes.
> 
> How does this work with with multipe-device filesystems? The fstrim
> range would apply to all of them. Which does make some sense, though
> might be unexpected as this does not happen for other filesystems.

Well FITRIM doesn't have support for multiple device so it's to the
discretion of the fs how exactly this is implemented. And this is indeed
the way things work currently.

> 
> The FITRIM range is in the physical coordinates, so eg. the taking the
> maximum size of all devices and iterating over that by 1GiB steps would
> go though the whole filesystem. Something to put to the changelog and
> documentation.

I don't follow, trimming would just trim the physical range as passed by
fstrim -o/-l options. That 1gb value is not hardcoded anywhere. If
someone wants to trim all of the freespace (which is what the majority
of the user do) then they can run FITRIM with 0 for offset and -1 for
length.

> 
>> Fixes: 499f377f49f0 ("btrfs: iterate over unused chunk space in FITRIM")
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  fs/btrfs/extent-tree.c | 25 +++++++++++++++++++------
>>  1 file changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 1fdba38761f7..fc3f6acc3c9b 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -11190,9 +11190,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
>>   * held back allocations.
>>   */
>>  static int btrfs_trim_free_extents(struct btrfs_device *device,
>> -				   u64 minlen, u64 *trimmed)
>> +				   struct fstrim_range *range, u64 *trimmed)
>>  {
>> -	u64 start = 0, len = 0;
>> +	u64 start = range->start, len = 0;
>>  	int ret;
>>  
>>  	*trimmed = 0;
>> @@ -11235,8 +11235,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>  		if (!trans)
>>  			up_read(&fs_info->commit_root_sem);
>>  
>> -		ret = find_free_dev_extent_start(trans, device, minlen, start,
>> -						 &start, &len);
>> +		ret = find_free_dev_extent_start(trans, device, range->minlen,
>> +						 start, &start, &len);
>>  		if (trans) {
>>  			up_read(&fs_info->commit_root_sem);
>>  			btrfs_put_transaction(trans);
>> @@ -11249,6 +11249,16 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>  			break;
>>  		}
>>  
>> +		/* If we are out of the passed range break */
>> +		if (start > range->start + range->len - 1) {
>> +			mutex_unlock(&fs_info->chunk_mutex);
>> +			ret = 0;
>> +			break;
>> +		}
>> +
>> +		start = max(range->start, start);
>> +		len = min(range->len, len);
>> +
>>  		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
>>  		mutex_unlock(&fs_info->chunk_mutex);
>>  
>> @@ -11258,6 +11268,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>  		start += len;
>>  		*trimmed += bytes;
>>  
>> +		/* We've trimmed enough */
>> +		if (*trimmed >= range->len)
>> +			break;
>> +
>>  		if (fatal_signal_pending(current)) {
>>  			ret = -ERESTARTSYS;
>>  			break;
>> @@ -11341,8 +11355,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>>  	mutex_lock(&fs_info->fs_devices->device_list_mutex);
>>  	devices = &fs_info->fs_devices->devices;
>>  	list_for_each_entry(device, devices, dev_list) {
>> -		ret = btrfs_trim_free_extents(device, range->minlen,
>> -					      &group_trimmed);
>> +		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
>>  		if (ret) {
>>  			dev_failed++;
>>  			dev_ret = ret;
>> -- 
>> 2.17.1
David Sterba Jan. 31, 2019, 3:48 p.m. UTC | #3
On Thu, Jan 31, 2019 at 05:35:10PM +0200, Nikolay Borisov wrote:
> On 31.01.19 г. 17:21 ч., David Sterba wrote:
> > On Wed, Jan 30, 2019 at 04:50:48PM +0200, Nikolay Borisov wrote:
> >> Up until know trimming the freespace was done irrespective of what the
> >> arguments of the FITRIM ioctl were. For example fstrim's -o/-l arguments
> >> will be entirely ignored. Fix it by correctly handling those paramter.
> >> This requires breaking if the found freespace extent is after the end
> >> of the passed range as well as completing trim after trimming
> >> fstrim_range::len bytes.
> > 
> > How does this work with with multipe-device filesystems? The fstrim
> > range would apply to all of them. Which does make some sense, though
> > might be unexpected as this does not happen for other filesystems.
> 
> Well FITRIM doesn't have support for multiple device so it's to the
> discretion of the fs how exactly this is implemented. And this is indeed
> the way things work currently.
> 
> > The FITRIM range is in the physical coordinates, so eg. the taking the
> > maximum size of all devices and iterating over that by 1GiB steps would
> > go though the whole filesystem. Something to put to the changelog and
> > documentation.
> 
> I don't follow, trimming would just trim the physical range as passed by
> fstrim -o/-l options. That 1gb value is not hardcoded anywhere. If
> someone wants to trim all of the freespace (which is what the majority
> of the user do) then they can run FITRIM with 0 for offset and -1 for
> length.

Yeah but I wanted to give an example of usecase when the range is not
0/-1 and how this is could to be used.
Jeff Mahoney Jan. 31, 2019, 7:30 p.m. UTC | #4
On 1/31/19 10:35 AM, Nikolay Borisov wrote:
> 
> 
> On 31.01.19 г. 17:21 ч., David Sterba wrote:
>> On Wed, Jan 30, 2019 at 04:50:48PM +0200, Nikolay Borisov wrote:
>>> Up until know trimming the freespace was done irrespective of what the
>>> arguments of the FITRIM ioctl were. For example fstrim's -o/-l arguments
>>> will be entirely ignored. Fix it by correctly handling those paramter.
>>> This requires breaking if the found freespace extent is after the end
>>> of the passed range as well as completing trim after trimming
>>> fstrim_range::len bytes.
>>
>> How does this work with with multipe-device filesystems? The fstrim
>> range would apply to all of them. Which does make some sense, though
>> might be unexpected as this does not happen for other filesystems.
> 
> Well FITRIM doesn't have support for multiple device so it's to the
> discretion of the fs how exactly this is implemented. And this is indeed
> the way things work currently.
> 
>>
>> The FITRIM range is in the physical coordinates, so eg. the taking the
>> maximum size of all devices and iterating over that by 1GiB steps would
>> go though the whole filesystem. Something to put to the changelog and
>> documentation.
> 
> I don't follow, trimming would just trim the physical range as passed by
> fstrim -o/-l options. That 1gb value is not hardcoded anywhere. If
> someone wants to trim all of the freespace (which is what the majority
> of the user do) then they can run FITRIM with 0 for offset and -1 for
> length.

But what does physical range mean in this context?  The address space
presented to the user and which FITRIM operates on is the logical
address space that the extent tree uses.  For allocated chunks, there's
a mapping to physical devices.  It can have holes in it.  What does it
mean when that offset/length hits a hole?

I'm okay with (0,-1) referring to all the space on all devices.  That
seems obvious enough.  Perhaps for bounded trims, it only applies to the
parts within the range that are allocated?

-Jeff


>>> Fixes: 499f377f49f0 ("btrfs: iterate over unused chunk space in FITRIM")
>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>> ---
>>>  fs/btrfs/extent-tree.c | 25 +++++++++++++++++++------
>>>  1 file changed, 19 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>> index 1fdba38761f7..fc3f6acc3c9b 100644
>>> --- a/fs/btrfs/extent-tree.c
>>> +++ b/fs/btrfs/extent-tree.c
>>> @@ -11190,9 +11190,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
>>>   * held back allocations.
>>>   */
>>>  static int btrfs_trim_free_extents(struct btrfs_device *device,
>>> -				   u64 minlen, u64 *trimmed)
>>> +				   struct fstrim_range *range, u64 *trimmed)
>>>  {
>>> -	u64 start = 0, len = 0;
>>> +	u64 start = range->start, len = 0;
>>>  	int ret;
>>>  
>>>  	*trimmed = 0;
>>> @@ -11235,8 +11235,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>  		if (!trans)
>>>  			up_read(&fs_info->commit_root_sem);
>>>  
>>> -		ret = find_free_dev_extent_start(trans, device, minlen, start,
>>> -						 &start, &len);
>>> +		ret = find_free_dev_extent_start(trans, device, range->minlen,
>>> +						 start, &start, &len);
>>>  		if (trans) {
>>>  			up_read(&fs_info->commit_root_sem);
>>>  			btrfs_put_transaction(trans);
>>> @@ -11249,6 +11249,16 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>  			break;
>>>  		}
>>>  
>>> +		/* If we are out of the passed range break */
>>> +		if (start > range->start + range->len - 1) {
>>> +			mutex_unlock(&fs_info->chunk_mutex);
>>> +			ret = 0;
>>> +			break;
>>> +		}
>>> +
>>> +		start = max(range->start, start);
>>> +		len = min(range->len, len);
>>> +
>>>  		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
>>>  		mutex_unlock(&fs_info->chunk_mutex);
>>>  
>>> @@ -11258,6 +11268,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>  		start += len;
>>>  		*trimmed += bytes;
>>>  
>>> +		/* We've trimmed enough */
>>> +		if (*trimmed >= range->len)
>>> +			break;
>>> +
>>>  		if (fatal_signal_pending(current)) {
>>>  			ret = -ERESTARTSYS;
>>>  			break;
>>> @@ -11341,8 +11355,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>>>  	mutex_lock(&fs_info->fs_devices->device_list_mutex);
>>>  	devices = &fs_info->fs_devices->devices;
>>>  	list_for_each_entry(device, devices, dev_list) {
>>> -		ret = btrfs_trim_free_extents(device, range->minlen,
>>> -					      &group_trimmed);
>>> +		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
>>>  		if (ret) {
>>>  			dev_failed++;
>>>  			dev_ret = ret;
>>> -- 
>>> 2.17.1
>
Nikolay Borisov Jan. 31, 2019, 9:45 p.m. UTC | #5
On 31.01.19 г. 21:30 ч., Jeff Mahoney wrote:
> On 1/31/19 10:35 AM, Nikolay Borisov wrote:
>>
>>
>> On 31.01.19 г. 17:21 ч., David Sterba wrote:
>>> On Wed, Jan 30, 2019 at 04:50:48PM +0200, Nikolay Borisov wrote:
>>>> Up until know trimming the freespace was done irrespective of what the
>>>> arguments of the FITRIM ioctl were. For example fstrim's -o/-l arguments
>>>> will be entirely ignored. Fix it by correctly handling those paramter.
>>>> This requires breaking if the found freespace extent is after the end
>>>> of the passed range as well as completing trim after trimming
>>>> fstrim_range::len bytes.
>>>
>>> How does this work with with multipe-device filesystems? The fstrim
>>> range would apply to all of them. Which does make some sense, though
>>> might be unexpected as this does not happen for other filesystems.
>>
>> Well FITRIM doesn't have support for multiple device so it's to the
>> discretion of the fs how exactly this is implemented. And this is indeed
>> the way things work currently.
>>
>>>
>>> The FITRIM range is in the physical coordinates, so eg. the taking the
>>> maximum size of all devices and iterating over that by 1GiB steps would
>>> go though the whole filesystem. Something to put to the changelog and
>>> documentation.
>>
>> I don't follow, trimming would just trim the physical range as passed by
>> fstrim -o/-l options. That 1gb value is not hardcoded anywhere. If
>> someone wants to trim all of the freespace (which is what the majority
>> of the user do) then they can run FITRIM with 0 for offset and -1 for
>> length.
> 
> But what does physical range mean in this context?  The address space
> presented to the user and which FITRIM operates on is the logical
> address space that the extent tree uses.  For allocated chunks, there's
> a mapping to physical devices.  It can have holes in it.  What does it
> mean when that offset/length hits a hole?

Physical in this context mean the actual space on the disk themselves.
This really has repercussion when trimming freespace. So for example when

fstrim -o 50m -l 20g is run this means that all space between 50m and
20g in the block groups (logical free space) will be trimmed.
Additionally, the physical (that is the space on the actual devices)
will also be trimmed. I don't think we can avoid this.

> 
> I'm okay with (0,-1) referring to all the space on all devices.  That
> seems obvious enough.  Perhaps for bounded trims, it only applies to the
> parts within the range that are allocated?
> 
> -Jeff
> 
> 
>>>> Fixes: 499f377f49f0 ("btrfs: iterate over unused chunk space in FITRIM")
>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>> ---
>>>>  fs/btrfs/extent-tree.c | 25 +++++++++++++++++++------
>>>>  1 file changed, 19 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>>> index 1fdba38761f7..fc3f6acc3c9b 100644
>>>> --- a/fs/btrfs/extent-tree.c
>>>> +++ b/fs/btrfs/extent-tree.c
>>>> @@ -11190,9 +11190,9 @@ int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
>>>>   * held back allocations.
>>>>   */
>>>>  static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>> -				   u64 minlen, u64 *trimmed)
>>>> +				   struct fstrim_range *range, u64 *trimmed)
>>>>  {
>>>> -	u64 start = 0, len = 0;
>>>> +	u64 start = range->start, len = 0;
>>>>  	int ret;
>>>>  
>>>>  	*trimmed = 0;
>>>> @@ -11235,8 +11235,8 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>>  		if (!trans)
>>>>  			up_read(&fs_info->commit_root_sem);
>>>>  
>>>> -		ret = find_free_dev_extent_start(trans, device, minlen, start,
>>>> -						 &start, &len);
>>>> +		ret = find_free_dev_extent_start(trans, device, range->minlen,
>>>> +						 start, &start, &len);
>>>>  		if (trans) {
>>>>  			up_read(&fs_info->commit_root_sem);
>>>>  			btrfs_put_transaction(trans);
>>>> @@ -11249,6 +11249,16 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>>  			break;
>>>>  		}
>>>>  
>>>> +		/* If we are out of the passed range break */
>>>> +		if (start > range->start + range->len - 1) {
>>>> +			mutex_unlock(&fs_info->chunk_mutex);
>>>> +			ret = 0;
>>>> +			break;
>>>> +		}
>>>> +
>>>> +		start = max(range->start, start);
>>>> +		len = min(range->len, len);
>>>> +
>>>>  		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
>>>>  		mutex_unlock(&fs_info->chunk_mutex);
>>>>  
>>>> @@ -11258,6 +11268,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
>>>>  		start += len;
>>>>  		*trimmed += bytes;
>>>>  
>>>> +		/* We've trimmed enough */
>>>> +		if (*trimmed >= range->len)
>>>> +			break;
>>>> +
>>>>  		if (fatal_signal_pending(current)) {
>>>>  			ret = -ERESTARTSYS;
>>>>  			break;
>>>> @@ -11341,8 +11355,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>>>>  	mutex_lock(&fs_info->fs_devices->device_list_mutex);
>>>>  	devices = &fs_info->fs_devices->devices;
>>>>  	list_for_each_entry(device, devices, dev_list) {
>>>> -		ret = btrfs_trim_free_extents(device, range->minlen,
>>>> -					      &group_trimmed);
>>>> +		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
>>>>  		if (ret) {
>>>>  			dev_failed++;
>>>>  			dev_ret = ret;
>>>> -- 
>>>> 2.17.1
>>
> 
>

Patch
diff mbox series

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 1fdba38761f7..fc3f6acc3c9b 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -11190,9 +11190,9 @@  int btrfs_error_unpin_extent_range(struct btrfs_fs_info *fs_info,
  * held back allocations.
  */
 static int btrfs_trim_free_extents(struct btrfs_device *device,
-				   u64 minlen, u64 *trimmed)
+				   struct fstrim_range *range, u64 *trimmed)
 {
-	u64 start = 0, len = 0;
+	u64 start = range->start, len = 0;
 	int ret;
 
 	*trimmed = 0;
@@ -11235,8 +11235,8 @@  static int btrfs_trim_free_extents(struct btrfs_device *device,
 		if (!trans)
 			up_read(&fs_info->commit_root_sem);
 
-		ret = find_free_dev_extent_start(trans, device, minlen, start,
-						 &start, &len);
+		ret = find_free_dev_extent_start(trans, device, range->minlen,
+						 start, &start, &len);
 		if (trans) {
 			up_read(&fs_info->commit_root_sem);
 			btrfs_put_transaction(trans);
@@ -11249,6 +11249,16 @@  static int btrfs_trim_free_extents(struct btrfs_device *device,
 			break;
 		}
 
+		/* If we are out of the passed range break */
+		if (start > range->start + range->len - 1) {
+			mutex_unlock(&fs_info->chunk_mutex);
+			ret = 0;
+			break;
+		}
+
+		start = max(range->start, start);
+		len = min(range->len, len);
+
 		ret = btrfs_issue_discard(device->bdev, start, len, &bytes);
 		mutex_unlock(&fs_info->chunk_mutex);
 
@@ -11258,6 +11268,10 @@  static int btrfs_trim_free_extents(struct btrfs_device *device,
 		start += len;
 		*trimmed += bytes;
 
+		/* We've trimmed enough */
+		if (*trimmed >= range->len)
+			break;
+
 		if (fatal_signal_pending(current)) {
 			ret = -ERESTARTSYS;
 			break;
@@ -11341,8 +11355,7 @@  int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	devices = &fs_info->fs_devices->devices;
 	list_for_each_entry(device, devices, dev_list) {
-		ret = btrfs_trim_free_extents(device, range->minlen,
-					      &group_trimmed);
+		ret = btrfs_trim_free_extents(device, range, &group_trimmed);
 		if (ret) {
 			dev_failed++;
 			dev_ret = ret;