From patchwork Tue Nov 21 04:29:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Qu Wenruo X-Patchwork-Id: 10067649 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D7F8E6022E for ; Tue, 21 Nov 2017 04:29:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C35DC2893D for ; Tue, 21 Nov 2017 04:29:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B690928A1E; Tue, 21 Nov 2017 04:29:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_HI,T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0335C2893D for ; Tue, 21 Nov 2017 04:29:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752721AbdKUE3b (ORCPT ); Mon, 20 Nov 2017 23:29:31 -0500 Received: from mout.gmx.net ([212.227.17.21]:61939 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752330AbdKUE3a (ORCPT ); Mon, 20 Nov 2017 23:29:30 -0500 Received: from [0.0.0.0] ([210.140.77.29]) by mail.gmx.com (mrgmx103 [212.227.17.174]) with ESMTPSA (Nemesis) id 0M1WHV-1f9e2g1Sgr-00tPON; Tue, 21 Nov 2017 05:29:28 +0100 Subject: Re: bug? fstrim only trims unallocated space, not unused in bg's To: Chris Murphy Cc: Btrfs BTRFS References: <0f6c7514-1cf2-1d88-1307-c43f7642b525@gmx.com> From: Qu Wenruo Message-ID: Date: Tue, 21 Nov 2017 12:29:24 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: X-Provags-ID: V03:K0:oCAwW6mAsGjYgl5djM3NT9k9RPOP6196yXgYkqXvPf5RvHEEA+3 IH2DG7aOXRgFadRI5iSj/csR/p//Jzh7v56N455QToo92YzsTaFUUNlimga+c12bQdLRToe 3VxWmdZN/t4y17dTprkQouS+9q1Vrt+EkzsJUTtB1aNnIb+q9BLj3iwqgvBLtBlaIC0z6g0 Ui+mqnK8rhbQuS56IRRwg== X-UI-Out-Filterresults: notjunk:1; V01:K0:fyUH8tQjLo8=:yWiFKffblSHVMTuWnlog/j DN+Fd267CROcrx0pNgBNK96JE4aKBbk4sYrwgr1rR5cSXt4TG5bxgkJkwQEJ5J5C5z3uiIcoN N51AEghOQL6b1uJfG41GPwjtT0KZXAreCxSfPdHWlfd+jsctlyO3dqFXL00hzjzX4cMuAojww 5ZPxeyYnMB9U6jVsy2PxYWAxPHD00t++Bb6tDlES3rWRuPqIjhLFXjynNm/NtPEO9iNnTzfO7 k58pY5XmxrKnO7DyX11fxMoeqf9p6zRXHlifMClU6l3beCUjsJhKeGBiskPwORL+JyFmkRB0w jnaYjlL59N6dO5C49B5yEFfdIgYPPxKDV21KmYNo+/AMJaT9YJc7Z2xFxwv6wyFU1QS2ffUWp jMkYfUCuHq7xpHLm4oUW/sBFd+cU8uAS/30qvbh/5+67mXn5BnQgGKTCDVet9SK9IiL8kHl8f epsBJuY15bUG67edv7bdLplFWJ18nDvH9VDipJVD/79MLqwsVemtcsSnSZjfCUTUOyYaJNr/c t79cKbhmHbdlJqjjSyCkju5LBWb7gk9aenTZaffu3WTFD8zGdxYk7lkne3GuZbv3vN9GerDeA agaTJF0YRz1CV4yT6t/kYPUMaqE1LZgun1/vrbLRvN1v/+cNBBYu08d98dal3slwZNqygBKRE oD0FhWeQkd/gYxsiUIUluPsu+Y84BQSFTgIRV2nlkuX+r4rcFJWNn3dBSnmiuJRl/ohS4YGDi QbqjD6KLiVBsr5Q1K68eY7Vest+cYnRXc12MUFjMJB61d5+PsAXPbc6lhJcKDZ6QG+m5D/Ik+ UTq1LXKkGQ4lxkgBOu8UhLr0MZ6rg== Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 2017年11月21日 12:06, Chris Murphy wrote: > On Mon, Nov 20, 2017 at 6:16 PM, Qu Wenruo wrote: >> >> >> On 2017年11月21日 06:23, Chris Murphy wrote: >>> On Sun, Nov 19, 2017 at 7:42 PM, Qu Wenruo wrote: >>>> >>>> >>>> On 2017年11月20日 10:24, Chris Murphy wrote: >>>>> On Sun, Nov 19, 2017 at 7:13 PM, Qu Wenruo 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. >> >> Sorry, that diff will not output extra info. Just to abort the process >> and return true error code. > > OK? It didn't seem to do that either. I see no change. > > >> >> I have update the patch to output more verbose output. >> You could find it in patchwork: >> https://patchwork.kernel.org/patch/10065991/ > > Patch applies on v4.14.0, and still nothing in dmesg, or in user space > when issuing fstrim. > > # fstrim -v / > /: 38 GiB (40767586304 bytes) trimmed > # dmesg | grep -i btrfs > [ 2.745902] btrfs: loading out-of-tree module taints kernel. > [ 2.749905] Btrfs loaded, crc32c=crc32c-intel > [ 2.751072] BTRFS: device label fedora devid 1 transid 252048 /dev/nvme0n1p8 > [ 4.295891] BTRFS info (device nvme0n1p8): disk space caching is enabled > [ 4.295892] BTRFS info (device nvme0n1p8): has skinny extents > [ 4.307326] BTRFS info (device nvme0n1p8): enabling ssd optimizations > [ 4.959467] BTRFS info (device nvme0n1p8): disk space caching is enabled > [root@f27h ~]# > > > Pretty sure the patch is applied because of the first message about > the out of tree module, which I do not get with Fedora kernels. So that's not something wrong happened to make you skip trimming one chunk, but something else just skipped the block group trimming. And I don't think DEBUG config is related to this. I doubt if it's the @fstrim_range passed in has something strange that prevent us from trimming block groups. Would you please try this diff based on the patch from patchwork? ------ * try to trim all FS space, our block group may start from non-zero. */ @@ -10981,6 +10983,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) cache = btrfs_lookup_block_group(fs_info, range->start); for (; cache; cache = next_block_group(fs_info, cache)) { + btrfs_info(fs_info, "bg start=%llu len=%llu", + cache->key.objectid, cache->key.offset); if (cache->key.objectid >= (range->start + range->len)) { btrfs_put_block_group(cache); break; @@ -11045,6 +11049,7 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) mutex_unlock(&fs_info->fs_devices->device_list_mutex); range->len = trimmed; + btrfs_info(fs_info, "trimming done"); if (bg_ret) return bg_ret; return dev_ret; ------ Thanks, Qu diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index f830aa91ac3d..a4bf29a4a860 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -10972,6 +10972,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) int dev_ret = 0; int ret = 0; + btrfs_info(fs_info, "trimming btrfs, start=%llu len=%llu minlen=%llu", + range->start, range->len, range->minlen); /*