* [PATCH v2 1/2] btrfs: Enhance btrfs_trim_fs function to handle error better
@ 2017-11-21 7:21 Qu Wenruo
2017-11-21 7:21 ` [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs Qu Wenruo
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2017-11-21 7:21 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, lists
Function btrfs_trim_fs() doesn't handle errors in a consistent way, if
error happens when trimming existing block groups, it will skip the
remaining blocks and continue to trim unallocated space for each device.
And the return value will only reflect the final error from device
trimming.
This patch will fix such behavior by:
1) Recording first error from block group or device trimming
So return value will also reflect any error found when trimming.
Make developer more aware of the problem.
2) Outputting debug message for total trimming failure
Since trimming failure for device and block group is not fatal, put
the message level to debug and only output the total failure number
along with the info for first block group/device.
3) Continuing trimming if we can
If we failed to trim one block group or device, we could still try
next block group or device.
Such behavior can avoid confusion for case like failure to trim the
first block group and then only unallocated space is trimmed.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
Only report total number of errors and first errno to make it less
noisy.
Change message level from warning to debug
---
fs/btrfs/extent-tree.c | 75 ++++++++++++++++++++++++++++++++++++++------------
1 file changed, 58 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index e2d7e86b51d1..3a252d7af158 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -10995,6 +10995,16 @@ static int btrfs_trim_free_extents(struct btrfs_device *device,
return ret;
}
+/*
+ * Trim the whole fs, by:
+ * 1) Trimming free space in each block group
+ * 2) Trimming unallocated space in each device
+ *
+ * Will try to continue trimming even if we failed to trim one block group or
+ * device.
+ * The return value will be the error return value of the first error.
+ * Or 0 if nothing wrong happened.
+ */
int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
{
struct btrfs_block_group_cache *cache = NULL;
@@ -11005,6 +11015,12 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
u64 end;
u64 trimmed = 0;
u64 total_bytes = btrfs_super_total_bytes(fs_info->super_copy);
+ u64 first_bg_failed = 0;
+ u64 first_dev_failed = 0;
+ int bg_failed = 0;
+ int dev_failed = 0;
+ int bg_ret = 0;
+ int dev_ret = 0;
int ret = 0;
/*
@@ -11015,7 +11031,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
else
cache = btrfs_lookup_block_group(fs_info, range->start);
- while (cache) {
+ for (; cache; cache = next_block_group(fs_info, cache)) {
if (cache->key.objectid >= (range->start + range->len)) {
btrfs_put_block_group(cache);
break;
@@ -11029,45 +11045,70 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
if (!block_group_cache_done(cache)) {
ret = cache_block_group(cache, 0);
if (ret) {
- btrfs_put_block_group(cache);
- break;
+ if (!bg_ret) {
+ bg_ret = ret;
+ first_bg_failed =
+ cache->key.objectid;
+ }
+ bg_failed++;
+ continue;
}
ret = wait_block_group_cache_done(cache);
if (ret) {
- btrfs_put_block_group(cache);
- break;
+ if (!bg_ret) {
+ bg_ret = ret;
+ first_bg_failed =
+ cache->key.objectid;
+ }
+ bg_failed++;
+ continue;
}
}
- ret = btrfs_trim_block_group(cache,
- &group_trimmed,
- start,
- end,
- range->minlen);
+ ret = btrfs_trim_block_group(cache, &group_trimmed,
+ start, end, range->minlen);
trimmed += group_trimmed;
if (ret) {
- btrfs_put_block_group(cache);
- break;
+ if (!bg_ret) {
+ bg_ret = ret;
+ first_bg_failed = cache->key.objectid;
+ }
+ bg_failed++;
+ continue;
}
}
-
- cache = next_block_group(fs_info, cache);
}
+ if (bg_ret)
+ btrfs_debug(fs_info,
+ "failed to trim %d block groups, first error occurs for block group %llu ret %d",
+ bg_failed, first_bg_failed, bg_ret);
+
mutex_lock(&fs_info->fs_devices->device_list_mutex);
devices = &fs_info->fs_devices->alloc_list;
list_for_each_entry(device, devices, dev_alloc_list) {
ret = btrfs_trim_free_extents(device, range->minlen,
&group_trimmed);
- if (ret)
- break;
+ if (ret) {
+ if (!dev_ret) {
+ dev_ret = ret;
+ first_dev_failed = device->devid;
+ }
+ dev_failed++;
+ }
trimmed += group_trimmed;
}
mutex_unlock(&fs_info->fs_devices->device_list_mutex);
+ if (dev_ret)
+ btrfs_debug(fs_info,
+ "failed to trim %d devices, first error occurs for devid %llu ret %d",
+ dev_failed, first_dev_failed, dev_ret);
range->len = trimmed;
- return ret;
+ if (bg_ret)
+ return bg_ret;
+ return dev_ret;
}
/*
--
2.15.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs
2017-11-21 7:21 [PATCH v2 1/2] btrfs: Enhance btrfs_trim_fs function to handle error better Qu Wenruo
@ 2017-11-21 7:21 ` Qu Wenruo
2017-11-21 7:41 ` Nikolay Borisov
2017-11-21 15:12 ` Filipe Manana
0 siblings, 2 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-21 7:21 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, lists
[BUG]
fstrim on some btrfs only trims the unallocated space, not trimming any
space in existing block groups.
[CAUSE]
fstrim_range passed in by default fstrim will be:
range->start = 0
range->len = fs_size (which equals with super->total_bytes)
range->min_len = 512
However btrfs_trim_fs() following above parameter to search block groups
to trim.
While it's quite possible that all chunks start beyond
super->total_bytes if the fs is balanced several times.
In that case, btrfs will skip trimming block groups and only trim the
unallocated space of each device.
[FIX]
For common full fs trimming range passed in, extent its len to (u64)-1
so we will iterate all block groups.
And for custom fs trimming range, due to the fact that the range will
always be truncated by range [0, super->total_bytes), making custom fs
trimming range useless.
Just return -ENOTTY for custom fs trimming range.
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent-tree.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3a252d7af158..22bbcc8c4f6c 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -11024,12 +11024,31 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
int ret = 0;
/*
- * try to trim all FS space, our block group may start from non-zero.
+ * NOTE: Btrfs uses its own logical address space, where its first
+ * chunk can start anywhere if it wants.
+ * If we follow common start = 0 and len = fs_size from @range, we
+ * can end up without trimming any block groups, since it's highly
+ * possible all chunks start beyond that range.
+ *
+ * So if we want to trim the whole fs, extent the len to (u64)-1 to trim
+ * all block groups.
+ *
+ * Also, since @range will always be truncated to fs size, manually
+ * passing range to trim specified range doesn't make much sense.
+ * (No mean to trim any block group whose bytenr starts beyond
+ * @total_bytes)
+ * So in that case, return -ENOTTY directly to prevent any custom trim
+ * request.
*/
- if (range->len == total_bytes)
- cache = btrfs_lookup_first_block_group(fs_info, range->start);
- else
- cache = btrfs_lookup_block_group(fs_info, range->start);
+ if (range->start == 0 && range->len == total_bytes) {
+ range->len = (u64)-1;
+ } else {
+ btrfs_info(fs_info,
+ "trimming custom range is not supported due to the limitation of fstrim_range");
+ return -ENOTTY;
+ }
+
+ cache = btrfs_lookup_first_block_group(fs_info, range->start);
for (; cache; cache = next_block_group(fs_info, cache)) {
if (cache->key.objectid >= (range->start + range->len)) {
--
2.15.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs
2017-11-21 7:21 ` [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs Qu Wenruo
@ 2017-11-21 7:41 ` Nikolay Borisov
2017-11-21 8:03 ` Qu Wenruo
2017-11-21 15:12 ` Filipe Manana
1 sibling, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2017-11-21 7:41 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: dsterba, lists
On 21.11.2017 09:21, Qu Wenruo wrote:
> [BUG]
> fstrim on some btrfs only trims the unallocated space, not trimming any
> space in existing block groups.
>
> [CAUSE]
> fstrim_range passed in by default fstrim will be:
>
> range->start = 0
> range->len = fs_size (which equals with super->total_bytes)
> range->min_len = 512
>
> However btrfs_trim_fs() following above parameter to search block groups
> to trim.
>
> While it's quite possible that all chunks start beyond
> super->total_bytes if the fs is balanced several times.
>
> In that case, btrfs will skip trimming block groups and only trim the
> unallocated space of each device.
>
> [FIX]
> For common full fs trimming range passed in, extent its len to (u64)-1
> so we will iterate all block groups.
>
> And for custom fs trimming range, due to the fact that the range will
> always be truncated by range [0, super->total_bytes), making custom fs
> trimming range useless.
>
> Just return -ENOTTY for custom fs trimming range.
>
> Reported-by: Chris Murphy <lists@colorremedies.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
I think we want this tagged for stable ?
> ---
> fs/btrfs/extent-tree.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 3a252d7af158..22bbcc8c4f6c 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -11024,12 +11024,31 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
> int ret = 0;
>
> /*
> - * try to trim all FS space, our block group may start from non-zero.
> + * NOTE: Btrfs uses its own logical address space, where its first
> + * chunk can start anywhere if it wants.
> + * If we follow common start = 0 and len = fs_size from @range, we
> + * can end up without trimming any block groups, since it's highly
> + * possible all chunks start beyond that range.
> + *
> + * So if we want to trim the whole fs, extent the len to (u64)-1 to trim
> + * all block groups.
> + *
> + * Also, since @range will always be truncated to fs size, manually
> + * passing range to trim specified range doesn't make much sense.
> + * (No mean to trim any block group whose bytenr starts beyond
> + * @total_bytes)
> + * So in that case, return -ENOTTY directly to prevent any custom trim
> + * request.
> */
> - if (range->len == total_bytes)
> - cache = btrfs_lookup_first_block_group(fs_info, range->start);
> - else
> - cache = btrfs_lookup_block_group(fs_info, range->start);
> + if (range->start == 0 && range->len == total_bytes) {
> + range->len = (u64)-1;
> + } else {
> + btrfs_info(fs_info,
> + "trimming custom range is not supported due to the limitation of fstrim_range");
> + return -ENOTTY;
> + }
> +
> + cache = btrfs_lookup_first_block_group(fs_info, range->start);
>
> for (; cache; cache = next_block_group(fs_info, cache)) {
> if (cache->key.objectid >= (range->start + range->len)) {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs
2017-11-21 7:41 ` Nikolay Borisov
@ 2017-11-21 8:03 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-21 8:03 UTC (permalink / raw)
To: Nikolay Borisov, Qu Wenruo, linux-btrfs; +Cc: dsterba, lists
[-- Attachment #1.1: Type: text/plain, Size: 3596 bytes --]
On 2017年11月21日 15:41, Nikolay Borisov wrote:
>
>
> On 21.11.2017 09:21, Qu Wenruo wrote:
>> [BUG]
>> fstrim on some btrfs only trims the unallocated space, not trimming any
>> space in existing block groups.
>>
>> [CAUSE]
>> fstrim_range passed in by default fstrim will be:
>>
>> range->start = 0
>> range->len = fs_size (which equals with super->total_bytes)
>> range->min_len = 512
>>
>> However btrfs_trim_fs() following above parameter to search block groups
>> to trim.
>>
>> While it's quite possible that all chunks start beyond
>> super->total_bytes if the fs is balanced several times.
>>
>> In that case, btrfs will skip trimming block groups and only trim the
>> unallocated space of each device.
>>
>> [FIX]
>> For common full fs trimming range passed in, extent its len to (u64)-1
>> so we will iterate all block groups.
>>
>> And for custom fs trimming range, due to the fact that the range will
>> always be truncated by range [0, super->total_bytes), making custom fs
>> trimming range useless.
>>
>> Just return -ENOTTY for custom fs trimming range.
>>
>> Reported-by: Chris Murphy <lists@colorremedies.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> I think we want this tagged for stable ?
Sounds pretty good.
Although I'm a little concerned about the ENOTTY return branch, not sure
if it will break script of some guys.
Thanks,
Qu
>
>
>> ---
>> fs/btrfs/extent-tree.c | 29 ++++++++++++++++++++++++-----
>> 1 file changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 3a252d7af158..22bbcc8c4f6c 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -11024,12 +11024,31 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>> int ret = 0;
>>
>> /*
>> - * try to trim all FS space, our block group may start from non-zero.
>> + * NOTE: Btrfs uses its own logical address space, where its first
>> + * chunk can start anywhere if it wants.
>> + * If we follow common start = 0 and len = fs_size from @range, we
>> + * can end up without trimming any block groups, since it's highly
>> + * possible all chunks start beyond that range.
>> + *
>> + * So if we want to trim the whole fs, extent the len to (u64)-1 to trim
>> + * all block groups.
>> + *
>> + * Also, since @range will always be truncated to fs size, manually
>> + * passing range to trim specified range doesn't make much sense.
>> + * (No mean to trim any block group whose bytenr starts beyond
>> + * @total_bytes)
>> + * So in that case, return -ENOTTY directly to prevent any custom trim
>> + * request.
>> */
>> - if (range->len == total_bytes)
>> - cache = btrfs_lookup_first_block_group(fs_info, range->start);
>> - else
>> - cache = btrfs_lookup_block_group(fs_info, range->start);
>> + if (range->start == 0 && range->len == total_bytes) {
>> + range->len = (u64)-1;
>> + } else {
>> + btrfs_info(fs_info,
>> + "trimming custom range is not supported due to the limitation of fstrim_range");
>> + return -ENOTTY;
>> + }
>> +
>> + cache = btrfs_lookup_first_block_group(fs_info, range->start);
>>
>> for (; cache; cache = next_block_group(fs_info, cache)) {
>> if (cache->key.objectid >= (range->start + range->len)) {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs
2017-11-21 7:21 ` [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs Qu Wenruo
2017-11-21 7:41 ` Nikolay Borisov
@ 2017-11-21 15:12 ` Filipe Manana
2017-11-22 0:42 ` Qu Wenruo
1 sibling, 1 reply; 6+ messages in thread
From: Filipe Manana @ 2017-11-21 15:12 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs@vger.kernel.org, dsterba@suse.cz, Chris Murphy
On Tue, Nov 21, 2017 at 7:21 AM, Qu Wenruo <wqu@suse.com> wrote:
> [BUG]
> fstrim on some btrfs only trims the unallocated space, not trimming any
> space in existing block groups.
>
> [CAUSE]
> fstrim_range passed in by default fstrim will be:
>
> range->start = 0
> range->len = fs_size (which equals with super->total_bytes)
> range->min_len = 512
>
> However btrfs_trim_fs() following above parameter to search block groups
> to trim.
>
> While it's quite possible that all chunks start beyond
> super->total_bytes if the fs is balanced several times.
>
> In that case, btrfs will skip trimming block groups and only trim the
> unallocated space of each device.
>
> [FIX]
> For common full fs trimming range passed in, extent its len to (u64)-1
> so we will iterate all block groups.
>
> And for custom fs trimming range, due to the fact that the range will
> always be truncated by range [0, super->total_bytes), making custom fs
> trimming range useless.
>
> Just return -ENOTTY for custom fs trimming range.
>
> Reported-by: Chris Murphy <lists@colorremedies.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/extent-tree.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 3a252d7af158..22bbcc8c4f6c 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -11024,12 +11024,31 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
> int ret = 0;
>
> /*
> - * try to trim all FS space, our block group may start from non-zero.
> + * NOTE: Btrfs uses its own logical address space, where its first
> + * chunk can start anywhere if it wants.
> + * If we follow common start = 0 and len = fs_size from @range, we
> + * can end up without trimming any block groups, since it's highly
> + * possible all chunks start beyond that range.
> + *
> + * So if we want to trim the whole fs, extent the len to (u64)-1 to trim
> + * all block groups.
> + *
> + * Also, since @range will always be truncated to fs size, manually
> + * passing range to trim specified range doesn't make much sense.
> + * (No mean to trim any block group whose bytenr starts beyond
> + * @total_bytes)
> + * So in that case, return -ENOTTY directly to prevent any custom trim
> + * request.
> */
> - if (range->len == total_bytes)
> - cache = btrfs_lookup_first_block_group(fs_info, range->start);
> - else
> - cache = btrfs_lookup_block_group(fs_info, range->start);
> + if (range->start == 0 && range->len == total_bytes) {
> + range->len = (u64)-1;
After the fs_trim program gets the value for the range's length and
before it invokes the trim ioctl, the value might have changed,
resulting in returning the enotty error below.
> + } else {
> + btrfs_info(fs_info,
> + "trimming custom range is not supported due to the limitation of fstrim_range");
I can't understand this message, and I doubt the average user/admin can.
To me it seems this can be a lot more simple by ignoring the range,
that is, always considering [0, (u64)-1[. After all, due to the way
btrfs organizes space, the range does not make any sense and I doubt
users/programs will have all the necessary knowledge and willing to
compute a range that makes sense to btrfs based on the current block
group layout of the fs...
> + return -ENOTTY;
> + }
> +
> + cache = btrfs_lookup_first_block_group(fs_info, range->start);
>
> for (; cache; cache = next_block_group(fs_info, cache)) {
> if (cache->key.objectid >= (range->start + range->len)) {
> --
> 2.15.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs
2017-11-21 15:12 ` Filipe Manana
@ 2017-11-22 0:42 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-22 0:42 UTC (permalink / raw)
To: fdmanana, Qu Wenruo
Cc: linux-btrfs@vger.kernel.org, dsterba@suse.cz, Chris Murphy
[-- Attachment #1.1: Type: text/plain, Size: 4848 bytes --]
On 2017年11月21日 23:12, Filipe Manana wrote:
> On Tue, Nov 21, 2017 at 7:21 AM, Qu Wenruo <wqu@suse.com> wrote:
>> [BUG]
>> fstrim on some btrfs only trims the unallocated space, not trimming any
>> space in existing block groups.
>>
>> [CAUSE]
>> fstrim_range passed in by default fstrim will be:
>>
>> range->start = 0
>> range->len = fs_size (which equals with super->total_bytes)
>> range->min_len = 512
>>
>> However btrfs_trim_fs() following above parameter to search block groups
>> to trim.
>>
>> While it's quite possible that all chunks start beyond
>> super->total_bytes if the fs is balanced several times.
>>
>> In that case, btrfs will skip trimming block groups and only trim the
>> unallocated space of each device.
>>
>> [FIX]
>> For common full fs trimming range passed in, extent its len to (u64)-1
>> so we will iterate all block groups.
>>
>> And for custom fs trimming range, due to the fact that the range will
>> always be truncated by range [0, super->total_bytes), making custom fs
>> trimming range useless.
>>
>> Just return -ENOTTY for custom fs trimming range.
>>
>> Reported-by: Chris Murphy <lists@colorremedies.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/extent-tree.c | 29 ++++++++++++++++++++++++-----
>> 1 file changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 3a252d7af158..22bbcc8c4f6c 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -11024,12 +11024,31 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>> int ret = 0;
>>
>> /*
>> - * try to trim all FS space, our block group may start from non-zero.
>> + * NOTE: Btrfs uses its own logical address space, where its first
>> + * chunk can start anywhere if it wants.
>> + * If we follow common start = 0 and len = fs_size from @range, we
>> + * can end up without trimming any block groups, since it's highly
>> + * possible all chunks start beyond that range.
>> + *
>> + * So if we want to trim the whole fs, extent the len to (u64)-1 to trim
>> + * all block groups.
>> + *
>> + * Also, since @range will always be truncated to fs size, manually
>> + * passing range to trim specified range doesn't make much sense.
>> + * (No mean to trim any block group whose bytenr starts beyond
>> + * @total_bytes)
>> + * So in that case, return -ENOTTY directly to prevent any custom trim
>> + * request.
>> */
>> - if (range->len == total_bytes)
>> - cache = btrfs_lookup_first_block_group(fs_info, range->start);
>> - else
>> - cache = btrfs_lookup_block_group(fs_info, range->start);
>> + if (range->start == 0 && range->len == total_bytes) {
>> + range->len = (u64)-1;
>
> After the fs_trim program gets the value for the range's length and
> before it invokes the trim ioctl, the value might have changed,
> resulting in returning the enotty error below.
>
>> + } else {
>> + btrfs_info(fs_info,
>> + "trimming custom range is not supported due to the limitation of fstrim_range");
>
> I can't understand this message, and I doubt the average user/admin can.
>
> To me it seems this can be a lot more simple by ignoring the range,
> that is, always considering [0, (u64)-1[. After all, due to the way
> btrfs organizes space, the range does not make any sense and I doubt
> users/programs will have all the necessary knowledge and willing to
> compute a range that makes sense to btrfs based on the current block
> group layout of the fs...
Yes, just ignoring the range at all is the simplest solution.
Although I'm a little worried about false bug report about wrong trimmed
bytes later.
Current bug is exposed by Chris Murphy who takes extra care about the
trimmed bytes.
If we change the behavior to always trim the whole fs, maybe some other
careful guy trying to trim some strange range will report bug about this.
Maybe put some note in btrfs(5) and always trim the whole fs will be a
better solution?
Thanks,
Qu
>
>> + return -ENOTTY;
>> + }
>> +
>> + cache = btrfs_lookup_first_block_group(fs_info, range->start);
>>
>> for (; cache; cache = next_block_group(fs_info, cache)) {
>> if (cache->key.objectid >= (range->start + range->len)) {
>> --
>> 2.15.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-22 0:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-21 7:21 [PATCH v2 1/2] btrfs: Enhance btrfs_trim_fs function to handle error better Qu Wenruo
2017-11-21 7:21 ` [PATCH 2/2] btrfs: extent-tree: Ensure btrfs_trim_fs can trim the whole fs Qu Wenruo
2017-11-21 7:41 ` Nikolay Borisov
2017-11-21 8:03 ` Qu Wenruo
2017-11-21 15:12 ` Filipe Manana
2017-11-22 0:42 ` Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).