On Sun, Nov 19, 2017 at 7:42 PM, Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote:
>
>
> On 2017年11月20日 10:24, Chris Murphy wrote:
>> On Sun, Nov 19, 2017 at 7:13 PM, Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote:
>>>
>>>
>>> On 2017年11月19日 14:17, Chris Murphy wrote:
>>>> fstrim should trim free space, but it only trims unallocated. This is
>>>> with kernel 4.14.0 and the entire 4.13 series. I'm pretty sure it
>>>> behaved this way with 4.12 also.
>>>
>>> Tested with 4.14-rc7, can't reproduce it.
>>
>> $ sudo btrfs fi us /
>> Overall:
>> Device size: 70.00GiB
>> Device allocated: 31.03GiB
>> Device unallocated: 38.97GiB
>> Device missing: 0.00B
>> Used: 22.12GiB
>> Free (estimated): 47.62GiB (min: 47.62GiB)
>> ...snip...
>>
>> $ sudo fstrim -v /
>> /: 39 GiB (41841328128 bytes) trimmed
>>
>> Then I run btrfs-debug -b / and find the least used block group, at 8% usage;
>>
>> block group offset 174202028032 len 1073741824 used 89206784
>> chunk_objectid 256 flags 1 usage 0.08
>>
>> And balance that block group:
>>
>> $ sudo btrfs balance start -dvrange=174202028032..174202028033 -dlimit=1 /
>> Done, had to relocate 1 out of 32 chunks
>>
>> And trim again:
>>
>> /: 39 GiB (41841328128 bytes) trimmed
>>
>>
>>> Any special mount options or setup?
>>> (BTW, I also tried space_cache=v2 and default v1, no obvious difference)
>>
>>
>> /dev/nvme0n1p8 on / type btrfs
>> (rw,relatime,seclabel,ssd,space_cache,subvolid=333,subvol=/root27)
>
> Nothing special at all.
>
> And unfortunately, no trace point inside btrfs_trim_block_group() at all.
>
> But a quick glance shows me that, the loop to iterate existing block
> groups to trim free space inside them has a return value overwrite bug.
>
> So only unallocated space get trimmed.
>
> Would you please try this diff to get the return value?
>
> ------
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 309a109069f1..dbec05dc8810 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -10983,12 +10983,12 @@ int btrfs_trim_fs(struct btrfs_fs_info
> *fs_info, struct fstrim_range *range)
> ret = cache_block_group(cache, 0);
> if (ret) {
> btrfs_put_block_group(cache);
> - break;
> + goto out;
> }
> ret = wait_block_group_cache_done(cache);
> if (ret) {
> btrfs_put_block_group(cache);
> - break;
> + goto out;
> }
> }
> ret = btrfs_trim_block_group(cache,
> @@ -11000,7 +11000,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info,
> struct fstrim_range *range)
> trimmed += group_trimmed;
> if (ret) {
> btrfs_put_block_group(cache);
> - break;
> + goto out;
> }
> }
>
> @@ -11019,6 +11019,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info,
> struct fstrim_range *range)
> }
> mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>
> +out:
> range->len = trimmed;
> return ret;
> }
> ------
This won't apply on tag v4.14 for some reason.
[chris@f27s linux]$ git apply -v ~/qutrim1.patch
Checking patch fs/btrfs/extent-tree.c...
error: while searching for:
ret = cache_block_group(cache, 0);
if (ret) {
btrfs_put_block_group(cache);
break;
}
ret = wait_block_group_cache_done(cache);
if (ret) {
btrfs_put_block_group(cache);
break;
}
}
ret = btrfs_trim_block_group(cache,
error: patch failed: fs/btrfs/extent-tree.c:10983
error: fs/btrfs/extent-tree.c: patch does not apply
[chris@f27s linux]$
If I do it manually (just adding the goto and build it, reboot, I
still get the same result for fstrim and nothing in dmesg.
--
Chris Murphy
--
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
