On Tue, 28 Nov 2017 15:20:39 +0800, Qu Wenruo wrote:
> [BUG]
> fstrim on some btrfs only trims the unallocated space, not trimming any
> space in existing block groups.
>
> [CAUSE]
> Before fstrim_range passed to btrfs_trim_fs(), it get truncated to
> range [0, super->total_bytes).
> So later btrfs_trim_fs() will only be able to trim block groups in range
> [0, super->total_bytes).
>
> While for btrfs, any bytenr aligned to sector size is valid, since btrfs use
> its logical address space, there is nothing limiting the location where
> we put block groups.
>
> For btrfs with routine balance, it's quite easy to relocate all
> block groups and bytenr of block groups will start beyond super->total_bytes.
>
> In that case, btrfs will not trim existing block groups.
>
> [FIX]
> Just remove the truncation in btrfs_ioctl_fitrim(), so btrfs_trim_fs()
> can get the unmodified range, which is normally set to [0, U64_MAX].
>
> Reported-by: Chris Murphy <lists@xxxxxxxxxxxxxxxxx>
> Fixes: f4c697e6406d ("btrfs: return EINVAL if start > total_bytes in fitrim ioctl")
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.0+
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> changelog:
> v2:
> Locate the root cause in btrfs_ioctl_fitrim(), remove the truncation
> so we can still allow user to trim custom range.
> v2.1:
> Include the missing change in btrfs_trim_fs() and update the commit
> message to reflect this.
> ---
> fs/btrfs/extent-tree.c | 9 +--------
> fs/btrfs/ioctl.c | 11 +++++++----
> 2 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f830aa91ac3d..f74958e11008 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -10972,14 +10972,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
> int dev_ret = 0;
> int ret = 0;
>
> - /*
> - * try to trim all FS space, our block group may start from non-zero.
> - */
> - 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);
> -
> + 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)) {
> btrfs_put_block_group(cache);
(..snip..)
This first hunk should also remove total_bytes, otherwise we get an
unused-variable warning:
@@ -10972,19 +10972,11 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
u64 start;
u64 end;
u64 trimmed = 0;
- u64 total_bytes = btrfs_super_total_bytes(fs_info->super_copy);
int bg_ret = 0;
int dev_ret = 0;
int ret = 0;
..etc..
cheers,
Holger
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
