Bulk discard doesn't work after add/delete of devices

Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

... maybe even the block group cache is nonfunctional then?

I am using a btrfs file system, mirrored data and metadata on two SSDs,
and recently tried for the first time to run fstrim on it. fstrim
unexpectedly said "0 bytes were trimmed". strace -T shows that it spends
only a few microseconds in the ioctl system call (basically the overhead
of strace, it seems), so I engaged in some "printk" debugging and found
that after "btrfs_trim_fs" executes its first statement,

  cache = btrfs_lookup_block_group(fs_info, range->start);

cache is 0. As the file system was created with a very recent kernel and
always mounted with the default "space_cache" option I guessed that this
might have something to do with the fact that I exchanged both the
filesystem's devices earlier (as you can see from the devid's in the
following output -- this is the only btrfs file system on the machine):

# btrfs fi show /dev/sda3
Label: none  uuid: 88af7576-3027-4a3b-a5ae-34bfd167982f
	Total devices 2 FS bytes used 28.13GB
	devid    4 size 74.53GB used 38.06GB path /dev/sdb1
	devid    3 size 75.24GB used 38.06GB path /dev/sda3

("exchanged" means: I created the filesystem as a mirror of sdb1 and
sdb2, put data on it, then added sda3, balanced, deleted sdb2, balanced
again, then unmounted, repartitioned sdb1 and sdb2 together as a larger
sdb1, mounted degraded, added sdb1, balanced and deleted "missing".)

So I tried to reproduce this and got the following:

# dd if=/dev/zero of=img0 bs=1 count=0 seek=5G
# dd if=/dev/zero of=img1 bs=1 count=0 seek=5G
# losetup -f img0
# losetup -f img1
# mkfs.btrfs -d raid1 -m raid1 /dev/loop0 /dev/loop1
# mount /dev/loop0 /mnt
# fstrim -v /mnt
/mnt: 4332265472 bytes were trimmed

(The above mentioned kernel instrumentation shows that "cache" was
not 0 here.)

# dd if=/dev/zero of=img2 bs=1 count=0 seek=5G
# losetup -f img2
# btrfs device add /dev/loop2 /mnt
# btrfs device delete /dev/loop0 /mnt
# fstrim -v /mnt
/mnt: 0 bytes were trimmed

("cache" was 0 here.)

Software versions: See my earlier mail about degraded mirrors;
reproducing the issue was on v3.3-rc2-172-g23783f8. I built fstrim
from git, that is, util-linux pulled yesterday from git.kernel.org.

What's going on here?
If the space cache is indeed broken on this filesystem, can I repair
it without risk to my data?
By mounting with "nospace_cache" once or somehow using "clear_cache"?

Greetings,

Lutz

Re: Bulk discard doesn't work after add/delete of devices

[Liu Bo]

On 02/06/2012 04:37 AM, Lutz Euler wrote:
> ... maybe even the block group cache is nonfunctional then?
> 
> I am using a btrfs file system, mirrored data and metadata on two SSDs,
> and recently tried for the first time to run fstrim on it. fstrim
> unexpectedly said "0 bytes were trimmed". strace -T shows that it spends
> only a few microseconds in the ioctl system call (basically the overhead
> of strace, it seems), so I engaged in some "printk" debugging and found
> that after "btrfs_trim_fs" executes its first statement,
> 
>   cache = btrfs_lookup_block_group(fs_info, range->start);
> 
> cache is 0. As the file system was created with a very recent kernel and
> always mounted with the default "space_cache" option I guessed that this
> might have something to do with the fact that I exchanged both the
> filesystem's devices earlier (as you can see from the devid's in the
> following output -- this is the only btrfs file system on the machine):
> 
> # btrfs fi show /dev/sda3
> Label: none  uuid: 88af7576-3027-4a3b-a5ae-34bfd167982f
> 	Total devices 2 FS bytes used 28.13GB
> 	devid    4 size 74.53GB used 38.06GB path /dev/sdb1
> 	devid    3 size 75.24GB used 38.06GB path /dev/sda3
> 
> ("exchanged" means: I created the filesystem as a mirror of sdb1 and
> sdb2, put data on it, then added sda3, balanced, deleted sdb2, balanced
> again, then unmounted, repartitioned sdb1 and sdb2 together as a larger
> sdb1, mounted degraded, added sdb1, balanced and deleted "missing".)
> 
> So I tried to reproduce this and got the following:
> 
> # dd if=/dev/zero of=img0 bs=1 count=0 seek=5G
> # dd if=/dev/zero of=img1 bs=1 count=0 seek=5G
> # losetup -f img0
> # losetup -f img1
> # mkfs.btrfs -d raid1 -m raid1 /dev/loop0 /dev/loop1
> # mount /dev/loop0 /mnt
> # fstrim -v /mnt
> /mnt: 4332265472 bytes were trimmed
> 
> (The above mentioned kernel instrumentation shows that "cache" was
> not 0 here.)
> 
> # dd if=/dev/zero of=img2 bs=1 count=0 seek=5G
> # losetup -f img2
> # btrfs device add /dev/loop2 /mnt
> # btrfs device delete /dev/loop0 /mnt
> # fstrim -v /mnt
> /mnt: 0 bytes were trimmed
> 
> ("cache" was 0 here.)
> 
> Software versions: See my earlier mail about degraded mirrors;
> reproducing the issue was on v3.3-rc2-172-g23783f8. I built fstrim
> from git, that is, util-linux pulled yesterday from git.kernel.org.
> 
> What's going on here?
> If the space cache is indeed broken on this filesystem, can I repair
> it without risk to my data?
> By mounting with "nospace_cache" once or somehow using "clear_cache"?
> 

Hi Lutz,

Would you please test the following patch on your box?

thanks,
liubo

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 77ea23c..b6e2c92 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -7653,9 +7653,16 @@ 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;
 
-	cache = btrfs_lookup_block_group(fs_info, range->start);
+	/*
+	 * 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);
 
 	while (cache) {
 		if (cache->key.objectid >= (range->start + range->len)) {
-- 
1.6.5.2


> Greetings,
> 
> Lutz
> --
> 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
> 

Re: Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

Hi Liu,

thanks for looking into this!

You wrote:

> Would you please test the following patch on your box?
> 
> thanks,
> liubo
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 77ea23c..b6e2c92 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -7653,9 +7653,16 @@ 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;
>  
> -	cache = btrfs_lookup_block_group(fs_info, range->start);
> +	/*
> +	 * 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);
>  
>  	while (cache) {
>  		if (cache->key.objectid >= (range->start + range->len)) {
> -- 
> 1.6.5.2

I'm sorry but that didn't help.
I applied the patch on top of v3.3-rc2-172-g23783f8 and still fstrim
says: "0 bytes were trimmed".

I left the printk's I mentioned in place. They show that now "cache"
is not 0 so the "while (cache)" loop is entered but nevertheless
"btrfs_trim_block_group" is never called. So I put in more printk's
and found that the objectid is so large that the loop is immediately
terminated (output of printk's in "//" comments below):

int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
{
	struct btrfs_fs_info *fs_info = root->fs_info;
	struct btrfs_block_group_cache *cache = NULL;
	u64 group_trimmed;
	u64 start;
	u64 end;
	u64 trimmed = 0;
	u64 total_bytes = btrfs_super_total_bytes(fs_info->super_copy);
	int ret = 0;

// range.start = 0, .len = 160815300608, .minlen = 512

	/*
	 * 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 != 0

	while (cache) {

// cache->key.objectid = 213702934528

		if (cache->key.objectid >= (range->start + range->len)) {
			btrfs_put_block_group(cache);
			break;
		}

// This line is not reached.

Here's the output of "btrfs fi show /dev/sda3" again:

failed to read /dev/sr0
Label: none  uuid: 88af7576-3027-4a3b-a5ae-34bfd167982f
	Total devices 2 FS bytes used 29.05GB
	devid    4 size 74.53GB used 40.06GB path /dev/sdb1
	devid    3 size 75.24GB used 40.06GB path /dev/sda3

Kind regards,

Lutz

Re: Bulk discard doesn't work after add/delete of devices

[Liu Bo]

On 02/09/2012 11:50 PM, Lutz Euler wrote:
> Hi Liu,
> 
> thanks for looking into this!
> 
> You wrote:
> 
>> Would you please test the following patch on your box?
>>
>> thanks,
>> liubo
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 77ea23c..b6e2c92 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -7653,9 +7653,16 @@ 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;
>>  
>> -	cache = btrfs_lookup_block_group(fs_info, range->start);
>> +	/*
>> +	 * 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);
>>  
>>  	while (cache) {
>>  		if (cache->key.objectid >= (range->start + range->len)) {
>> -- 
>> 1.6.5.2
> 
> I'm sorry but that didn't help.
> I applied the patch on top of v3.3-rc2-172-g23783f8 and still fstrim
> says: "0 bytes were trimmed".
> 
> I left the printk's I mentioned in place. They show that now "cache"
> is not 0 so the "while (cache)" loop is entered but nevertheless
> "btrfs_trim_block_group" is never called. So I put in more printk's
> and found that the objectid is so large that the loop is immediately
> terminated (output of printk's in "//" comments below):
> 
> int btrfs_trim_fs(struct btrfs_root *root, struct fstrim_range *range)
> {
> 	struct btrfs_fs_info *fs_info = root->fs_info;
> 	struct btrfs_block_group_cache *cache = NULL;
> 	u64 group_trimmed;
> 	u64 start;
> 	u64 end;
> 	u64 trimmed = 0;
> 	u64 total_bytes = btrfs_super_total_bytes(fs_info->super_copy);
> 	int ret = 0;
> 
> // range.start = 0, .len = 160815300608, .minlen = 512
> 
> 	/*
> 	 * 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 != 0
> 
> 	while (cache) {
> 
> // cache->key.objectid = 213702934528
> 
> 		if (cache->key.objectid >= (range->start + range->len)) {
> 			btrfs_put_block_group(cache);
> 			break;
> 		}
> 
> // This line is not reached.
> 
> Here's the output of "btrfs fi show /dev/sda3" again:
> 
> failed to read /dev/sr0
> Label: none  uuid: 88af7576-3027-4a3b-a5ae-34bfd167982f
> 	Total devices 2 FS bytes used 29.05GB
> 	devid    4 size 74.53GB used 40.06GB path /dev/sdb1
> 	devid    3 size 75.24GB used 40.06GB path /dev/sda3
> 

Hi Lutz,

Great thanks for your details, now this should be a worked one.

liubo,
thanks,

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 77ea23c..693241a 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -7653,12 +7653,22 @@ 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);
+	bool all = false;
 	int ret = 0;
 
-	cache = btrfs_lookup_block_group(fs_info, range->start);
+	/* try to trim all FS space */
+	if (range->len == total_bytes)
+		all = true;
+
+	if (all)
+		/* our block group may start from non-zero */
+		cache = btrfs_lookup_first_block_group(fs_info, range->start);
+	else
+		cache = btrfs_lookup_block_group(fs_info, range->start);
 
 	while (cache) {
-		if (cache->key.objectid >= (range->start + range->len)) {
+		if (!all && cache->key.objectid >= (range->start + range->len)) {
 			btrfs_put_block_group(cache);
 			break;
 		}
-- 
1.6.5.2



> Kind regards,
> 
> Lutz
> 

Re: Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

Hi,

tl;dr: btrfs_trim_fs, IMHO, needs more thorough surgery.

Thanks for providing the new patch. I think it will work in the case
that "fstrim" is called without specifying a range to trim (that is,
defaulting to the whole filesystem), but I didn't test that yet, sorry.

Instead, I have been thinking about what happens if a range smaller than
the whole filesystem is specified. After all, the observation that in my
filesystem the smallest "objectid" is already larger than the filesystem
size still holds even in that case, and wanting to trim only part of the
filesystem seems to be a valid wish, too.

So I dug into the code myself and came to the conclusion that the
way "btrfs_trim_fs" interprets the range passed from the ioctl is
fundamentally broken.

Instead of papering over that I'd very much prefer a more thorough fix
here, which in addition might make it unnecessary to treat the "trim the
complete filesystem" case specially. Please read on for the details:

The current implementation of "btrfs_trim_fs" simply compares the
objectid's it finds in the chunk tree against the range passed from
the ioctl, seemingly assuming that both kinds of numbers span the same
range. But this is clearly not true: Even without adding and deleting
of devices, in a simple mirror the objectids will span only about half
the size of the filesystem. With suitable add/delete of devices I can
construct a filesystem with a smallest objectid of 0 and a largest
objectid much larger than the size of the filesystem (so, obviously,
with large holes in the set of used objectid's), or, as in my filesystem
mentioned above, with a smallest objectid larger than the size of the
filesystem.

So, in the function the statement
  cache = btrfs_lookup_block_group(fs_info, range->start);
will lead to nothing being trimmed if the smallest objectid is too
large; if the
  if (cache->key.objectid >= (range->start + range->len))
is left in, too little will be trimmed; if it is left out, the following
calculations
  start = max(range->start, cache->key.objectid);
  end = min(range->start + range->len,
            cache->key.objectid + cache->key.offset);
  if (end - start >= range->minlen)
are bound to misbehave, for example start being larger than end seems
possible, overflowing the subtraction and, I believe, leading to the
whole filesystem being trimmed where the user only requested a part.

Here is the start of the chunk tree of my filesystem for reference,
showing the device sizes and the first objectid:

chunk tree
node 241620221952 level 1 items 2 free 119 generation 16831 owner 3
fs uuid 88af7576-3027-4a3b-a5ae-34bfd167982f
chunk uuid 9a7d25e3-ebcf-4f31-bdd4-f44531a2fb1c
        key (DEV_ITEMS DEV_ITEM 3) block 241620226048 (58989313) gen
          16831
        key (FIRST_CHUNK_TREE CHUNK_ITEM 231956545536) block
          241620230144 (58989314) gen 16831
leaf 241620226048 items 19 free space 1420 generation 16831 owner 3
fs uuid 88af7576-3027-4a3b-a5ae-34bfd167982f
chunk uuid 9a7d25e3-ebcf-4f31-bdd4-f44531a2fb1c
        item 0 key (DEV_ITEMS DEV_ITEM 3) itemoff 3897 itemsize 98
                dev item devid 3 total_bytes 80789987328 bytes used
                  44090523648
        item 1 key (DEV_ITEMS DEV_ITEM 4) itemoff 3799 itemsize 98
                dev item devid 4 total_bytes 80025313280 bytes used
                  44090523648
        item 2 key (FIRST_CHUNK_TREE CHUNK_ITEM 213702934528) itemoff
          3687 itemsize 112
                chunk length 1073741824 owner 2 type 20 num_stripes 2
                        stripe 0 devid 4 offset 1074790400
                        stripe 1 devid 3 offset 40870346752

So, to make bulk discard of ranges useful, it seems the incoming range
should be interpreted relative to the size of the filesystem and not to
the allocated chunks. As AFAIK the size of the filesystem is just the
sum of the sizes of its devices it might be possible to map the range
onto a virtual concatenation of the devices, these perhaps ordered by
devid, and then to find the free space by searching for the resulting
devid(s) and device-relative offsets in the device tree?

I understand that currently unallocated space is never trimmed?
To nevertheless do that might be useful after a balance or at file
system creation time? If there is a way to find the unallocated space
for a device, this could be improved at the same time, too.

Kind regards,

Lutz

Re: Bulk discard doesn't work after add/delete of devices

[Liu Bo]

On 02/13/2012 01:01 AM, Lutz Euler wrote:
> Hi,
> 
> tl;dr: btrfs_trim_fs, IMHO, needs more thorough surgery.
> 
> Thanks for providing the new patch. I think it will work in the case
> that "fstrim" is called without specifying a range to trim (that is,
> defaulting to the whole filesystem), but I didn't test that yet, sorry.
> 
> Instead, I have been thinking about what happens if a range smaller than
> the whole filesystem is specified. After all, the observation that in my
> filesystem the smallest "objectid" is already larger than the filesystem
> size still holds even in that case, and wanting to trim only part of the
> filesystem seems to be a valid wish, too.
> 
> So I dug into the code myself and came to the conclusion that the
> way "btrfs_trim_fs" interprets the range passed from the ioctl is
> fundamentally broken.
> 
> Instead of papering over that I'd very much prefer a more thorough fix
> here, which in addition might make it unnecessary to treat the "trim the
> complete filesystem" case specially. Please read on for the details:
> 
> The current implementation of "btrfs_trim_fs" simply compares the
> objectid's it finds in the chunk tree against the range passed from
> the ioctl, seemingly assuming that both kinds of numbers span the same
> range. But this is clearly not true: Even without adding and deleting
> of devices, in a simple mirror the objectids will span only about half
> the size of the filesystem. With suitable add/delete of devices I can
> construct a filesystem with a smallest objectid of 0 and a largest
> objectid much larger than the size of the filesystem (so, obviously,
> with large holes in the set of used objectid's), or, as in my filesystem
> mentioned above, with a smallest objectid larger than the size of the
> filesystem.
> 
[...]
> So, to make bulk discard of ranges useful, it seems the incoming range
> should be interpreted relative to the size of the filesystem and not to
> the allocated chunks. As AFAIK the size of the filesystem is just the
> sum of the sizes of its devices it might be possible to map the range
> onto a virtual concatenation of the devices, these perhaps ordered by
> devid, and then to find the free space by searching for the resulting
> devid(s) and device-relative offsets in the device tree?
> 

Actually I have no idea how to deal with this properly :(

Because btrfs supports multi-devices so that we have to set the filesystem
logical range to [0, (u64)-1] to get things to work well, while other
filesystems's logical range is [0, device's total_bytes].

What's more, in btrfs, devices can be mirrored to RAID, and the free space is
also ordered by logical addr [0, (u64)-1], so IMO it is impossible to interpreted the range.

I'd better pick up "treat the "trim the complete filesystem" case specially",
and drop the following commit:

commit f4c697e6406da5dd445eda8d923c53e1138793dd
Author: Lukas Czerner <lczerner@redhat.com>
Date:   Mon Sep 5 16:34:54 2011 +0200

    btrfs: return EINVAL if start > total_bytes in fitrim ioctl
    
    We should retirn EINVAL if the start is beyond the end of the file
    system in the btrfs_ioctl_fitrim(). Fix that by adding the appropriate
    check for it.
    
    Also in the btrfs_trim_fs() it is possible that len+start might overflow
    if big values are passed. Fix it by decrementing the len so that start+len
    is equal to the file system size in the worst case.
    
    Signed-off-by: Lukas Czerner <lczerner@redhat.com>

thanks,
liubo

> I understand that currently unallocated space is never trimmed?
> To nevertheless do that might be useful after a balance or at file
> system creation time? If there is a way to find the unallocated space
> for a device, this could be improved at the same time, too.
> 
> Kind regards,
> 
> Lutz
> --
> 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
> 

Re: Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

Hi,

Liu Bo wrote:

> Actually I have no idea how to deal with this properly :(
> 
> Because btrfs supports multi-devices so that we have to set the
> filesystem logical range to [0, (u64)-1] to get things to work well,
> while other filesystems's logical range is [0, device's total_bytes].
> 
> What's more, in btrfs, devices can be mirrored to RAID, and the free
> space is also ordered by logical addr [0, (u64)-1], so IMO it is
> impossible to interpreted the range.

I do not concur with this reasoning, but please see below that I concur
with your conclusion.

I still think that the range could be interpreted like I hinted at in my
mail:

>> So, to make bulk discard of ranges useful, it seems the incoming range
>> should be interpreted relative to the size of the filesystem and not
>> to the allocated chunks. As AFAIK the size of the filesystem is just
>> the sum of the sizes of its devices it might be possible to map the
>> range onto a virtual concatenation of the devices, these perhaps
>> ordered by devid, and then to find the free space by searching for the
>> resulting devid(s) and device-relative offsets in the device tree?

This would only be somewhat difficult to use if the filesystem consisted
of a mixture of non trim-capable and trim-capable devices and if the
idea behind ranges is that the user can expect an equal amount of
trim-capable storage behind different equally sized ranges - but I don't
know if this is indeed an idea behind ranges. If, on the other hand,
there is a way outside of the ioctl to find out which devices the
filesystem consists of and which of these support discard, the above
mentioned way to interpret ranges would extend to this setting.

But maybe this would already be too ugly, essentially working around
the shortcomings of an interface that is too restricted.

Moreover, I don't know why ranges smaller than the filesystem are
supported by fstrim. I couldn't find any use cases or rationale for it
in fstrim's or the ioctl's documentation or elsewhere. This makes it
difficult to find out what might be useful in the case of a multi-device
filesystem ;-)

So, with ranges being this unclear, I concur with your suggestion:

> I'd better pick up "treat the "trim the complete filesystem" case
> specially", and drop the following commit:
>
> commit f4c697e6406da5dd445eda8d923c53e1138793dd
> Author: Lukas Czerner <lczerner@redhat.com>
> Date:   Mon Sep 5 16:34:54 2011 +0200
>
>     btrfs: return EINVAL if start > total_bytes in fitrim ioctl

More specifically, what I do wish for is:
- Fix the problem I started the thread for: Make fstrim of the complete
  filesystem work.
And then, if possible:
- Simplify btrfs_trim_fs as much as possible to remove all traces
  of support for ranges smaller than the filesystem, document that this
  anyway wouldn't do what the user might expect, and, if possible,
  return an error in these cases.
- Also trim unallocated space (for use after balancing and at mkfs
  time).

Thanks for your time and your work,

Lutz

Re: Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

Hi,

I have seen that in the meantime a patch for this issue has found
its way into 3.3-rc5. Unfortunately with this kernel my filesystem still
says "0 bytes were trimmed", so the bug is not fixed for me.

Now that might just have been caused by the fact that the patch that
went into 3.3-rc5 (as commit 2cac13e41bf5b99ffc426bd28dfd2248df1dfa67)
is the one from Liu Bo's mail <4F3386D8.6010108@cn.fujitsu.com>, which
I already responded to by saying it didn't help my issue.

But the next patch Liu sent (in the mail
<4F34795B.2020004@cn.fujitsu.com>) also does not help.
At first I responded to that mail:

> Thanks for providing the new patch. I think it will work in the case
> that "fstrim" is called without specifying a range to trim (that is,
> defaulting to the whole filesystem), but I didn't test that yet, sorry.

I was too optimistic. I have now tested this second patch and still get
"0 bytes were trimmed".

Thanks again and regards,

Lutz

Re: Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

Hi,

the issue I raised in this thread is still not fixed. Please, could
someone look into it? A fix should be simple, especially as the issue is
easily reproducable. (In case the information in this thread so far is
not sufficient for that I will happily provide a recipe; just ask.)
Also, I believe so far not even a regression test has been added for
this issue.

Here's the state of affairs again, as I mailed previously:

> I have seen that in the meantime a patch for this issue has found
> its way into 3.3-rc5. Unfortunately with this kernel my filesystem still
> says "0 bytes were trimmed", so the bug is not fixed for me.
> 
> Now that might just have been caused by the fact that the patch that
> went into 3.3-rc5 (as commit 2cac13e41bf5b99ffc426bd28dfd2248df1dfa67)
> is the one from Liu Bo's mail <4F3386D8.6010108@cn.fujitsu.com>, which
> I already responded to by saying it didn't help my issue.
> 
> But the next patch Liu sent (in the mail
> <4F34795B.2020004@cn.fujitsu.com>) also does not help.
> At first I responded to that mail:
> 
> > Thanks for providing the new patch. I think it will work in the case
> > that "fstrim" is called without specifying a range to trim (that is,
> > defaulting to the whole filesystem), but I didn't test that yet, sorry.
> 
> I was too optimistic. I have now tested this second patch and still get
> "0 bytes were trimmed".

Thanks for providing btrfs! Greetings,

Lutz

Re: Bulk discard doesn't work after add/delete of devices

[Lutz Euler]

Hi,

the issue I raised in this thread is still not fixed. So, to get things
going, I prepared a patch myself which I will send in a separate email.
The subject will be "really fix trim 0 bytes after a device delete".
I would be grateful if you could consider the patch -- with it applied
I finally could trim my disks successfully.

Greetings,

Lutz

[PATCH] Btrfs: really fix trim 0 bytes after a device delete

[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 previous commit only fixes the case where the smallest objectid is
nonzero and the largest objectid less than the sum of the device sizes,
but it would still trim 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 only partly trim a filesystem, due to the broken mapping
this often didn't work anyway.)

Reported-by: Lutz Euler <lutz.euler@freenet.de>
Signed-off-by: Lutz Euler <lutz.euler@freenet.de>
---
 fs/btrfs/extent-tree.c |   55 +++++++++++++++++++++--------------------------
 1 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3d3e2c1..9c2bb59 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -8092,44 +8092,39 @@ 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)) {
-				ret = cache_block_group(cache, NULL, root, 0);
-				if (!ret)
-					wait_block_group_cache_done(cache);
-			}
-			ret = btrfs_trim_block_group(cache,
-						     &group_trimmed,
-						     start,
-						     end,
-						     range->minlen);
+		if (!block_group_cache_done(cache)) {
+			ret = cache_block_group(cache, NULL, root, 0);
+			if (!ret)
+				wait_block_group_cache_done(cache);
+		}
+		ret = btrfs_trim_block_group(cache,
+					     &group_trimmed,
+					     start,
+					     end,
+					     range->minlen);
 
-			trimmed += group_trimmed;
-			if (ret) {
-				btrfs_put_block_group(cache);
-				break;
-			}
+		trimmed += group_trimmed;
+		if (ret) {
+			btrfs_put_block_group(cache);
+			break;
 		}
 
 		cache = next_block_group(fs_info->tree_root, cache);
-- 
1.7.4.1

fstrim not working on one of three BTRFS filesystems

[Martin Steigerwald]

Hi!

After my recent tests with my /home filesystem and the up and downsizing of
it I get:


merkaba:~> LANG=C fstrim -v /home
/home: 0 B (0 bytes) trimmed
merkaba:~> LANG=C fstrim -v /    
/: 24.5 GiB (26257555456 bytes) trimmed
merkaba:~> LANG=C fstrim -v /daten
/daten: 2.8 GiB (3016101888 bytes) trimmed


The fstrim on /home returns immediately. It does not even seem to trim
anything. What could be the cause for that?




merkaba:~> btrfs fi sh  
Label: 'debian'  uuid: […]
        Total devices 2 FS bytes used 17.95GiB
        devid    1 size 30.00GiB used 30.00GiB path /dev/mapper/sata-debian
        devid    2 size 30.00GiB used 30.00GiB path /dev/mapper/msata-debian

Label: 'daten'  uuid: […]
        Total devices 1 FS bytes used 191.20GiB
        devid    1 size 220.00GiB used 194.02GiB path /dev/mapper/msata-daten

Label: 'home'  uuid: […]
        Total devices 2 FS bytes used 151.13GiB
        devid    1 size 170.00GiB used 161.85GiB path /dev/mapper/msata-home
        devid    2 size 170.00GiB used 161.85GiB path /dev/mapper/sata-home

Label: none  uuid: […]
        Total devices 2 FS bytes used 512.00KiB
        devid    1 size 10.00GiB used 6.53GiB path /dev/mapper/sata-btrfsraid1
        devid    2 size 10.00GiB used 6.53GiB path /dev/mapper/msata-btrfsraid1

Btrfs v3.17
merkaba:~> btrfs fi df /home
Data, RAID1: total=156.89GiB, used=147.87GiB
System, RAID1: total=32.00MiB, used=48.00KiB
Metadata, RAID1: total=4.94GiB, used=3.26GiB
GlobalReserve, single: total=512.00MiB, used=0.00B
merkaba:~> btrfs fi df /    
Data, RAID1: total=27.99GiB, used=17.37GiB
System, RAID1: total=8.00MiB, used=16.00KiB
Metadata, RAID1: total=2.00GiB, used=597.31MiB
GlobalReserve, single: total=208.00MiB, used=0.00B
merkaba:~> btrfs fi df /daten
Data, single: total=193.01GiB, used=190.90GiB
System, single: total=4.00MiB, used=48.00KiB
Metadata, single: total=1.01GiB, used=306.44MiB
GlobalReserve, single: total=112.00MiB, used=0.00B


As far as I understand it should try to trim the free space in

Data, RAID1: total=156.89GiB, used=147.87GiB

like it does for the others.




This is the strace of the 0-byte fstrim:

merkaba:~> LANG=C strace -f fstrim -v /home
execve("/sbin/fstrim", ["fstrim", "-v", "/home"], [/* 21 vars */]) = 0
brk(0)                                  = 0x1ee5000
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78bc5000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=253940, ...}) = 0
mmap(NULL, 253940, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f4f78b87000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libmount.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0P\254\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=284096, ...}) = 0
mmap(NULL, 2383648, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f78761000
mprotect(0x7f4f787a4000, 2097152, PROT_NONE) = 0
mmap(0x7f4f789a4000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x43000) = 0x7f4f789a4000
mmap(0x7f4f789a6000, 3872, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f789a6000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0P\34\2\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1729984, ...}) = 0
mmap(NULL, 3836448, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f783b8000
mprotect(0x7f4f78557000, 2097152, PROT_NONE) = 0
mmap(0x7f4f78757000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x19f000) = 0x7f4f78757000
mmap(0x7f4f7875d000, 14880, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f7875d000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libblkid.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\340\210\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=258688, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b86000
mmap(NULL, 2358248, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f78178000
mprotect(0x7f4f781b3000, 2097152, PROT_NONE) = 0
mmap(0x7f4f783b3000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x3b000) = 0x7f4f783b3000
mmap(0x7f4f783b7000, 3048, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f783b7000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libselinux.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20c\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=142728, ...}) = 0
mmap(NULL, 2246896, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f77f53000
mprotect(0x7f4f77f74000, 2097152, PROT_NONE) = 0
mmap(0x7f4f78174000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x21000) = 0x7f4f78174000
mmap(0x7f4f78176000, 6384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f78176000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libuuid.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0 \26\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=18904, ...}) = 0
mmap(NULL, 2113952, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f77d4e000
mprotect(0x7f4f77d52000, 2093056, PROT_NONE) = 0
mmap(0x7f4f77f51000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x3000) = 0x7f4f77f51000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libpcre.so.3", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20\27\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=448440, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b85000
mmap(NULL, 2543976, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f77ae0000
mprotect(0x7f4f77b4c000, 2097152, PROT_NONE) = 0
mmap(0x7f4f77d4c000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x6c000) = 0x7f4f77d4c000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libdl.so.2", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\320\16\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0644, st_size=14664, ...}) = 0
mmap(NULL, 2109712, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f778dc000
mprotect(0x7f4f778df000, 2093056, PROT_NONE) = 0
mmap(0x7f4f77ade000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f4f77ade000
close(3)                                = 0
access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
open("/lib/x86_64-linux-gnu/libpthread.so.0", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20o\0\0\0\0\0\0"..., 832) = 832
fstat(3, {st_mode=S_IFREG|0755, st_size=137440, ...}) = 0
mmap(NULL, 2213008, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f776bf000
mprotect(0x7f4f776d7000, 2093056, PROT_NONE) = 0
mmap(0x7f4f778d6000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17000) = 0x7f4f778d6000
mmap(0x7f4f778d8000, 13456, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f778d8000
close(3)                                = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b84000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b83000
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b81000
arch_prctl(ARCH_SET_FS, 0x7f4f78b81840) = 0
mprotect(0x7f4f78757000, 16384, PROT_READ) = 0
mprotect(0x7f4f778d6000, 4096, PROT_READ) = 0
mprotect(0x7f4f77ade000, 4096, PROT_READ) = 0
mprotect(0x7f4f77d4c000, 4096, PROT_READ) = 0
mprotect(0x7f4f77f51000, 4096, PROT_READ) = 0
mprotect(0x7f4f78174000, 4096, PROT_READ) = 0
mprotect(0x7f4f783b3000, 12288, PROT_READ) = 0
mprotect(0x7f4f789a4000, 4096, PROT_READ) = 0
mprotect(0x607000, 4096, PROT_READ)     = 0
mprotect(0x7f4f78bc7000, 4096, PROT_READ) = 0
munmap(0x7f4f78b87000, 253940)          = 0
set_tid_address(0x7f4f78b81b10)         = 3122
set_robust_list(0x7f4f78b81b20, 24)     = 0
rt_sigaction(SIGRTMIN, {0x7f4f776c59f0, [], SA_RESTORER|SA_SIGINFO, 0x7f4f776ce8d0}, NULL, 8) = 0
rt_sigaction(SIGRT_1, {0x7f4f776c5a80, [], SA_RESTORER|SA_RESTART|SA_SIGINFO, 0x7f4f776ce8d0}, NULL, 8) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM64_INFINITY}) = 0
statfs("/sys/fs/selinux", 0x7fffaec07ac0) = -1 ENOENT (No such file or directory)
statfs("/selinux", 0x7fffaec07ac0)      = -1 ENOENT (No such file or directory)
brk(0)                                  = 0x1ee5000
brk(0x1f06000)                          = 0x1f06000
open("/proc/filesystems", O_RDONLY)     = 3
fstat(3, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78bc4000
read(3, "nodev\tsysfs\nnodev\trootfs\nnodev\tr"..., 1024) = 329
read(3, "", 1024)                       = 0
close(3)                                = 0
munmap(0x7f4f78bc4000, 4096)            = 0
open("/home", O_RDONLY)                 = 3
fstat(3, {st_mode=S_IFDIR|0755, st_size=124, ...}) = 0
ioctl(3, FITRIM, 0x7fffaec07970)        = 0
fstat(1, {st_mode=S_IFCHR|0600, st_rdev=makedev(136, 0), ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78bc4000
write(1, "/home: 0 B (0 bytes) trimmed\n", 29/home: 0 B (0 bytes) trimmed
) = 29
close(3)                                = 0
close(1)                                = 0
munmap(0x7f4f78bc4000, 4096)            = 0
close(2)                                = 0
exit_group(0)                           = ?
+++ exited with 0 +++


There is nothing in dmesg about this.

Thanks,
-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

Re: fstrim not working on one of three BTRFS filesystems

[Robert White]

On 12/28/2014 08:58 AM, Martin Steigerwald wrote:
> Hi!
>
> After my recent tests with my /home filesystem and the up and downsizing of
> it I get:
>
>
> merkaba:~> LANG=C fstrim -v /home
> /home: 0 B (0 bytes) trimmed
> merkaba:~> LANG=C fstrim -v /
> /: 24.5 GiB (26257555456 bytes) trimmed
> merkaba:~> LANG=C fstrim -v /daten
> /daten: 2.8 GiB (3016101888 bytes) trimmed
>
>
> The fstrim on /home returns immediately. It does not even seem to trim
> anything. What could be the cause for that?
>
>
>
>
> merkaba:~> btrfs fi sh
> Label: 'debian'  uuid: […]
>          Total devices 2 FS bytes used 17.95GiB
>          devid    1 size 30.00GiB used 30.00GiB path /dev/mapper/sata-debian
>          devid    2 size 30.00GiB used 30.00GiB path /dev/mapper/msata-debian
>
> Label: 'daten'  uuid: […]
>          Total devices 1 FS bytes used 191.20GiB
>          devid    1 size 220.00GiB used 194.02GiB path /dev/mapper/msata-daten
>
> Label: 'home'  uuid: […]
>          Total devices 2 FS bytes used 151.13GiB
>          devid    1 size 170.00GiB used 161.85GiB path /dev/mapper/msata-home
>          devid    2 size 170.00GiB used 161.85GiB path /dev/mapper/sata-home
>
> Label: none  uuid: […]
>          Total devices 2 FS bytes used 512.00KiB
>          devid    1 size 10.00GiB used 6.53GiB path /dev/mapper/sata-btrfsraid1
>          devid    2 size 10.00GiB used 6.53GiB path /dev/mapper/msata-btrfsraid1
>
> Btrfs v3.17
> merkaba:~> btrfs fi df /home
> Data, RAID1: total=156.89GiB, used=147.87GiB
> System, RAID1: total=32.00MiB, used=48.00KiB
> Metadata, RAID1: total=4.94GiB, used=3.26GiB
> GlobalReserve, single: total=512.00MiB, used=0.00B
> merkaba:~> btrfs fi df /
> Data, RAID1: total=27.99GiB, used=17.37GiB
> System, RAID1: total=8.00MiB, used=16.00KiB
> Metadata, RAID1: total=2.00GiB, used=597.31MiB
> GlobalReserve, single: total=208.00MiB, used=0.00B
> merkaba:~> btrfs fi df /daten
> Data, single: total=193.01GiB, used=190.90GiB
> System, single: total=4.00MiB, used=48.00KiB
> Metadata, single: total=1.01GiB, used=306.44MiB
> GlobalReserve, single: total=112.00MiB, used=0.00B
>
>
> As far as I understand it should try to trim the free space in
>
> Data, RAID1: total=156.89GiB, used=147.87GiB
>
> like it does for the others.
>
>
>
>
> This is the strace of the 0-byte fstrim:
>
> merkaba:~> LANG=C strace -f fstrim -v /home
> execve("/sbin/fstrim", ["fstrim", "-v", "/home"], [/* 21 vars */]) = 0
> brk(0)                                  = 0x1ee5000
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78bc5000
> access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
> open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
> fstat(3, {st_mode=S_IFREG|0644, st_size=253940, ...}) = 0
> mmap(NULL, 253940, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f4f78b87000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libmount.so.1", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0P\254\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0644, st_size=284096, ...}) = 0
> mmap(NULL, 2383648, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f78761000
> mprotect(0x7f4f787a4000, 2097152, PROT_NONE) = 0
> mmap(0x7f4f789a4000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x43000) = 0x7f4f789a4000
> mmap(0x7f4f789a6000, 3872, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f789a6000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0P\34\2\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0755, st_size=1729984, ...}) = 0
> mmap(NULL, 3836448, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f783b8000
> mprotect(0x7f4f78557000, 2097152, PROT_NONE) = 0
> mmap(0x7f4f78757000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x19f000) = 0x7f4f78757000
> mmap(0x7f4f7875d000, 14880, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f7875d000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libblkid.so.1", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\340\210\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0644, st_size=258688, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b86000
> mmap(NULL, 2358248, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f78178000
> mprotect(0x7f4f781b3000, 2097152, PROT_NONE) = 0
> mmap(0x7f4f783b3000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x3b000) = 0x7f4f783b3000
> mmap(0x7f4f783b7000, 3048, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f783b7000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libselinux.so.1", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20c\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0644, st_size=142728, ...}) = 0
> mmap(NULL, 2246896, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f77f53000
> mprotect(0x7f4f77f74000, 2097152, PROT_NONE) = 0
> mmap(0x7f4f78174000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x21000) = 0x7f4f78174000
> mmap(0x7f4f78176000, 6384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f78176000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libuuid.so.1", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0 \26\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0644, st_size=18904, ...}) = 0
> mmap(NULL, 2113952, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f77d4e000
> mprotect(0x7f4f77d52000, 2093056, PROT_NONE) = 0
> mmap(0x7f4f77f51000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x3000) = 0x7f4f77f51000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libpcre.so.3", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20\27\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0644, st_size=448440, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b85000
> mmap(NULL, 2543976, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f77ae0000
> mprotect(0x7f4f77b4c000, 2097152, PROT_NONE) = 0
> mmap(0x7f4f77d4c000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x6c000) = 0x7f4f77d4c000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libdl.so.2", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\320\16\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0644, st_size=14664, ...}) = 0
> mmap(NULL, 2109712, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f778dc000
> mprotect(0x7f4f778df000, 2093056, PROT_NONE) = 0
> mmap(0x7f4f77ade000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f4f77ade000
> close(3)                                = 0
> access("/etc/ld.so.nohwcap", F_OK)      = -1 ENOENT (No such file or directory)
> open("/lib/x86_64-linux-gnu/libpthread.so.0", O_RDONLY|O_CLOEXEC) = 3
> read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\20o\0\0\0\0\0\0"..., 832) = 832
> fstat(3, {st_mode=S_IFREG|0755, st_size=137440, ...}) = 0
> mmap(NULL, 2213008, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f4f776bf000
> mprotect(0x7f4f776d7000, 2093056, PROT_NONE) = 0
> mmap(0x7f4f778d6000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17000) = 0x7f4f778d6000
> mmap(0x7f4f778d8000, 13456, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f4f778d8000
> close(3)                                = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b84000
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b83000
> mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78b81000
> arch_prctl(ARCH_SET_FS, 0x7f4f78b81840) = 0
> mprotect(0x7f4f78757000, 16384, PROT_READ) = 0
> mprotect(0x7f4f778d6000, 4096, PROT_READ) = 0
> mprotect(0x7f4f77ade000, 4096, PROT_READ) = 0
> mprotect(0x7f4f77d4c000, 4096, PROT_READ) = 0
> mprotect(0x7f4f77f51000, 4096, PROT_READ) = 0
> mprotect(0x7f4f78174000, 4096, PROT_READ) = 0
> mprotect(0x7f4f783b3000, 12288, PROT_READ) = 0
> mprotect(0x7f4f789a4000, 4096, PROT_READ) = 0
> mprotect(0x607000, 4096, PROT_READ)     = 0
> mprotect(0x7f4f78bc7000, 4096, PROT_READ) = 0
> munmap(0x7f4f78b87000, 253940)          = 0
> set_tid_address(0x7f4f78b81b10)         = 3122
> set_robust_list(0x7f4f78b81b20, 24)     = 0
> rt_sigaction(SIGRTMIN, {0x7f4f776c59f0, [], SA_RESTORER|SA_SIGINFO, 0x7f4f776ce8d0}, NULL, 8) = 0
> rt_sigaction(SIGRT_1, {0x7f4f776c5a80, [], SA_RESTORER|SA_RESTART|SA_SIGINFO, 0x7f4f776ce8d0}, NULL, 8) = 0
> rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
> getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM64_INFINITY}) = 0
> statfs("/sys/fs/selinux", 0x7fffaec07ac0) = -1 ENOENT (No such file or directory)
> statfs("/selinux", 0x7fffaec07ac0)      = -1 ENOENT (No such file or directory)
> brk(0)                                  = 0x1ee5000
> brk(0x1f06000)                          = 0x1f06000
> open("/proc/filesystems", O_RDONLY)     = 3
> fstat(3, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78bc4000
> read(3, "nodev\tsysfs\nnodev\trootfs\nnodev\tr"..., 1024) = 329
> read(3, "", 1024)                       = 0
> close(3)                                = 0
> munmap(0x7f4f78bc4000, 4096)            = 0
> open("/home", O_RDONLY)                 = 3
> fstat(3, {st_mode=S_IFDIR|0755, st_size=124, ...}) = 0
> ioctl(3, FITRIM, 0x7fffaec07970)        = 0
> fstat(1, {st_mode=S_IFCHR|0600, st_rdev=makedev(136, 0), ...}) = 0
> mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4f78bc4000
> write(1, "/home: 0 B (0 bytes) trimmed\n", 29/home: 0 B (0 bytes) trimmed
> ) = 29
> close(3)                                = 0
> close(1)                                = 0
> munmap(0x7f4f78bc4000, 4096)            = 0
> close(2)                                = 0
> exit_group(0)                           = ?
> +++ exited with 0 +++
>
>
> There is nothing in dmesg about this.
>
> Thanks,
>

If you let that fallocate test loop (see prior emails) run for a long 
time in that filesystem, you might be in an odd state.

When I run that here my "data used" doesn't change but the amount of 
data I can fstrim does.

Gust Work # fstrim --all --verbose
/mnt/Work: 623.2 MiB (653516800 bytes) trimmed
Gust Work # while     touch ${TESTDIR}/$((++counter)) &&     fallocate 
-n -l $((4096 + $RANDOM)) $TESTDIR/$((counter));   do     if 
((counter%100 == 0));     then       echo $counter; if 
((counter%1000==0)); then btrfs fi df /mnt/Work; fi;    fi;   done
313500
313600
313700
313800
313900
314000
Data, RAID1: total=1.72GiB, used=1.64GiB
System, RAID1: total=32.00MiB, used=16.00KiB
Metadata, RAID1: total=256.00MiB, used=58.08MiB
GlobalReserve, single: total=32.00MiB, used=0.00B
314100
314200
314300
314400
314500
314600
314700
314800
314900
315000
Data, RAID1: total=1.72GiB, used=1.64GiB
System, RAID1: total=32.00MiB, used=16.00KiB
Metadata, RAID1: total=256.00MiB, used=58.08MiB
GlobalReserve, single: total=32.00MiB, used=0.00B
^C
Gust Work # fstrim --all --verbose
/mnt/Work: 622.8 MiB (652992512 bytes) trimmed


None of those results make good sense to me but I might be missing 
something.

I could re-harmonize the results by unmounting and remounting the 
filesystem. In a long run I got the fstrim down to ~300MiB and an 
unmount and mount brought it right back to 625. In this case, I got back 
from 622 to 625... e.g.

Gust Work # fstrim --all --verbose
/mnt/Work: 622.8 MiB (652992512 bytes) trimmed
Gust Work # umount /mnt/Work
umount: /mnt/Work: target is busy
         (In some cases useful info about processes that
          use the device is found by lsof(8) or fuser(1).)
Gust Work # cd
Gust ~ # umount /mnt/Work
Gust ~ # mount /dev/loop0 /mnt/Work
Gust ~ # fstrim --all --verbose
/mnt/Work: 625 MiB (655335424 bytes) trimmed


So "fishy" but possibly related to free space caching limitations or 
something.

Given the failure of the Data: line to change during massive fallocate 
activities I suspect there is something off in there somewhere but I 
don't have enough information to quantify what's happening.

Re: fstrim not working on one of three BTRFS filesystems

[Duncan]

Martin Steigerwald posted on Sun, 28 Dec 2014 17:58:17 +0100 as excerpted:

> The fstrim on /home returns immediately. It does not even seem to trim
> anything. What could be the cause for that?

While I don't know your mapper layout, trim working on the others but not 
on /home sounds to me like either one of the physical devices on which 
/home is located doesn't support trim, or they all do, but somewhere 
along the line that information is being lost, so it doesn't believe trim 
can actually work on that filesystem.  Typical shortcut "the information 
available says that can't work, simply return before trying" behavior.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: fstrim not working on one of three BTRFS filesystems

[Martin Steigerwald]

Am Montag, 29. Dezember 2014, 02:08:21 schrieb Duncan:
> Martin Steigerwald posted on Sun, 28 Dec 2014 17:58:17 +0100 as excerpted:
> 
> > The fstrim on /home returns immediately. It does not even seem to trim
> > anything. What could be the cause for that?
> 
> While I don't know your mapper layout, trim working on the others but not 
> on /home sounds to me like either one of the physical devices on which 
> /home is located doesn't support trim, or they all do, but somewhere 
> along the line that information is being lost, so it doesn't believe trim 
> can actually work on that filesystem.  Typical shortcut "the information 
> available says that can't work, simply return before trying" behavior.

Duncan, it did trim before. Its on two SSDs that definately support trim.

Anyway, I got an hint about it of list with a patch to test.

-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

Re: fstrim not working on one of three BTRFS filesystems

[Martin Steigerwald]

Am Sonntag, 28. Dezember 2014, 17:58:17 schrieb Martin Steigerwald:
> Hi!
> 
> After my recent tests with my /home filesystem and the up and downsizing of
> it I get:
> 
> 
> merkaba:~> LANG=C fstrim -v /home
> /home: 0 B (0 bytes) trimmed
> merkaba:~> LANG=C fstrim -v /    
> /: 24.5 GiB (26257555456 bytes) trimmed
> merkaba:~> LANG=C fstrim -v /daten
> /daten: 2.8 GiB (3016101888 bytes) trimmed
> 
> 
> The fstrim on /home returns immediately. It does not even seem to trim
> anything. What could be the cause for that?

I have a 3.19-rc2 with a patch and a working fstrim now:

merkaba:~> btrfs scrub status /home
scrub status for […]
        scrub started at Mon Dec 29 13:48:17 2014 and finished after 568 seconds
        total bytes scrubbed: 272.73GiB with 0 errors

merkaba:~> fstrim -v /home
/home: 18,5 GiB (19873382400 Bytes) getrimmt

merkaba:~> btrfs scrub status /home
scrub status for […]
        scrub started at Mon Dec 29 14:02:58 2014 and finished after 576 seconds
        total bytes scrubbed: 272.82GiB with 0 errors

merkaba:~> btrfs fi df /home
Data, RAID1: total=139.93GiB, used=133.19GiB
System, RAID1: total=32.00MiB, used=48.00KiB
Metadata, RAID1: total=4.99GiB, used=3.24GiB
GlobalReserve, single: total=512.00MiB, used=0.00B

merkaba:~> LANG=C df -hT /home
Filesystem             Type   Size  Used Avail Use% Mounted on
/dev/mapper/msata-home btrfs  170G  137G   32G  82% /home



I wonder about the trimmed size tough:

139,93 - 133,19 = 6,74

4,99 - 3,24 = 1,74

( 6,74 + 1,74 ) * 2 =  16,96 GiB


It is more than whats free inside the chunks but less than the total unused
space. Well it may have been up to what has been allocated as chunks
as I had more chunks allocated in it.


I leave it to the patch author to come up with it on the mailing list :)

Ciao,
-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

[PATCH V2] Btrfs: really fix trim 0 bytes after a device delete

[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 <lutz.euler@freenet.de>
Signed-off-by: Lutz Euler <lutz.euler@freenet.de>
---
 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)) {
-- 
1.7.4.1

Re: fstrim not working on one of three BTRFS filesystems

[Lutz Euler]

Martin Steigerwald wrote:

> I have a 3.19-rc2 with a patch and a working fstrim now:
[...]
> I leave it to the patch author to come up with it on the mailing list :)

That would be me. I have just sent in the patch; please see
http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg40618.html

For a discussion about the issue that goes into more detail than the
commit message and the code comments please see this thread:
http://comments.gmane.org/gmane.comp.file-systems.btrfs/15597
At that time I had sent in a first version of this patch but it has not
been applied so far. I made the new version as the old one did not apply
any more since about kernel 3.9., and had a flaw (range->minlen was
being ignored).

I am using this patch on my system since two years without problems with
kernel versions ranging from 3.3 to 3.16.

Looking forward to a review,

Lutz Euler

Re: [PATCH V2] Btrfs: really fix trim 0 bytes after a device delete

[Martin Steigerwald]

Happy new year!

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 <lutz.euler@freenet.de>
> Signed-off-by: Lutz Euler <lutz.euler@freenet.de>

Is that the patch you send me for testing?

If so, feel free to add:

Reported-and-tested-by: Martin Steigerwald <martin@lichtvoll.de>

If not I can retest with this one.

Thanks,
Martin

> ---
>  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

Re: [PATCH V2] Btrfs: really fix trim 0 bytes after a device delete

[Lutz Euler]

Hello,

happy new year to you, too!

[Martin Steigerwald:]
> Happy new year!
> 
> 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 <lutz.euler@freenet.de>
> > Signed-off-by: Lutz Euler <lutz.euler@freenet.de>
> 
> Is that the patch you send me for testing?

Yes, it is.

Kind regards,

Lutz

> If so, feel free to add:
> 
> Reported-and-tested-by: Martin Steigerwald <martin@lichtvoll.de>
> 
> If not I can retest with this one.
> 
> Thanks,
> Martin
> 
> > ---
> >  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

Re: [PATCH V2] Btrfs: really fix trim 0 bytes after a device delete

[Martin Steigerwald]

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 <martin@lichtvoll.de>

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 <lutz.euler@freenet.de>
> Signed-off-by: Lutz Euler <lutz.euler@freenet.de>
> ---
>  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

Re: fstrim not working on one of three BTRFS filesystems

[Rich Freeman]

On Sat, Jan 3, 2015 at 11:16 AM, Lutz Euler <lutz.euler@freenet.de> wrote:
> Martin Steigerwald wrote:
>
>> I have a 3.19-rc2 with a patch and a working fstrim now:
> [...]
>> I leave it to the patch author to come up with it on the mailing list :)
>
> That would be me. I have just sent in the patch; please see
> http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg40618.html
>
> For a discussion about the issue that goes into more detail than the
> commit message and the code comments please see this thread:
> http://comments.gmane.org/gmane.comp.file-systems.btrfs/15597
> At that time I had sent in a first version of this patch but it has not
> been applied so far. I made the new version as the old one did not apply
> any more since about kernel 3.9., and had a flaw (range->minlen was
> being ignored).
>
> I am using this patch on my system since two years without problems with
> kernel versions ranging from 3.3 to 3.16.
>
> Looking forward to a review,
>

Did this go anywhere?  I don't see this in 3.18.13 unless I'm missing
it, and I also have been unable to fstrim my ssd btrfs volume for as
long as I've had it on an SSD.

--
Rich