From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mondschein.lichtvoll.de ([194.150.191.11]:56043 "EHLO mail.lichtvoll.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751270AbbEAKnb convert rfc822-to-8bit (ORCPT ); Fri, 1 May 2015 06:43:31 -0400 From: Martin Steigerwald To: Lutz Euler Cc: linux-btrfs@vger.kernel.org Subject: Re: [PATCH V2] Btrfs: really fix trim 0 bytes after a device delete Date: Fri, 01 May 2015 12:43:29 +0200 Message-ID: <1656835.n20mzq2Spt@merkaba> In-Reply-To: <21672.2859.468940.450115@localhost.localdomain> References: <20644.2272.57892.435731@localhost.localdomain> <20644.2675.653732.24816@localhost.localdomain> <21672.2859.468940.450115@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Sender: linux-btrfs-owner@vger.kernel.org List-ID: Hello! The following patch fixes the trimming on my /home Dual SSD BTRFS RAID 1 for quite some kernel releases already. Without it, it doesn´t trim anything, with it, it trims possibly. What would be required to get this or a similar fix into the kernel? In either case this gets my: Reported-and-tested-by: Martin Steigerwald I used trimming with this patch countless of times and scrub still tells all is fine. Thanks, Martin Am Samstag, 3. Januar 2015, 16:30:51 schrieb Lutz Euler: > Commit 2cac13e41bf5b99ffc426bd28dfd2248df1dfa67, "fix trim 0 bytes after > a device delete", said: > A user reported a bug of btrfs's trim, that is we will trim 0 bytes > after a device delete. > The commit didn't attack the root of the problem so did not fix the bug > except for a special case. > > For block discard, btrfs_trim_fs directly compares the range passed in > against the filesystem's objectids. The former is bounded by the sum of > the sizes of the devices of the filesystem, the latter is a completely > unrelated set of intervals of 64-bit integers. The bug reported occurred > as the smallest objectid was larger than the sum of the device sizes. > The above mentioned commit only fixed the case where the smallest > objectid is nonzero and the largest objectid less than the sum of the > device sizes, but it still trims too little if the largest objectid is > larger than that, and nothing in the reported situation. > > The current mapping between the given range and the objectids is thus > clearly broken, so, to fix the bug and as a first step towards a > complete solution, simply ignore the range parameter's start and length > fields and always trim the whole filesystem. (While this makes it > impossible to trim a filesystem only partly, due to the broken mapping > this often didn't work anyway.) > > V2: > - Rebased onto 3.9. (still applies to and works with 3.19-rc2) > - Take range->minlen into account. > > Reported-by: Lutz Euler > Signed-off-by: Lutz Euler > --- > fs/btrfs/extent-tree.c | 25 +++++++++++-------------- > 1 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index cfb3cf7..81006c1 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -8824,26 +8824,23 @@ int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range) > u64 start; > u64 end; > u64 trimmed = 0; > - u64 total_bytes = btrfs_super_total_bytes(fs_info->super_copy); > int ret = 0; > > /* > - * try to trim all FS space, our block group may start from non-zero. > + * The range passed in is a subinterval of the interval from 0 > + * to the sum of the sizes of the devices of the filesystem. > + * The objectid's used in the filesystem can span any set of > + * subintervals of the interval from 0 to (u64)-1. As there is > + * neither a simple nor an agreed upon mapping between these > + * two ranges we ignore the range parameter's start and len > + * fields and always trim the whole filesystem (that is, only > + * the free space in allocated chunks). > */ > - 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, 0); > > while (cache) { > - if (cache->key.objectid >= (range->start + range->len)) { > - btrfs_put_block_group(cache); > - break; > - } > - > - start = max(range->start, cache->key.objectid); > - end = min(range->start + range->len, > - cache->key.objectid + cache->key.offset); > + start = cache->key.objectid; > + end = cache->key.objectid + cache->key.offset; > > if (end - start >= range->minlen) { > if (!block_group_cache_done(cache)) { > -- Martin 'Helios' Steigerwald - http://www.Lichtvoll.de GPG: 03B0 0D6C 0040 0710 4AFA B82F 991B EAAC A599 84C7