Btrfs devel - Feb 2011 - Re: bug#8001: cp (8.10) sparse handling fails on compressed btrfs (cp/fiemap-2)

On 08/02/11 03:53, Mike Frysinger wrote:
> after upgrading from coreutils 8.9 to 8.10, the sparse handling in cp is 
> silently breaking on btrfs filesystems with the compressed option enabled.
> using --sparse=never works fine, but "auto" or "always"
tend to fail.  the
> cp/fiemap-2 test catches the issue nicely.
> 
> to reproduce (i''m using linux-2.6.37):
> file=btrfs.img
> mntp=/mnt/tmp/
> dd if=/dev/zero of=$file bs=1M count=0 seek=1024
> mkfs.btrfs $file 
> mount -t btrfs -o compress $file $mntp
> cd $mntp
> tar xf ~/coreutils-8.10.tar.xz
> cd coreutils-8.10/tests
> ./cp/fiemap-2
> 
> and this last test shows:
> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
> /dev/loop0   btrfs     1048576     20420    377028   6% /mnt/tmp
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 2.7598e-05 s, 0.0 kB/s
> k k2 differ: byte 1, line 1
Eek.

That doesn''t trigger here (2.6.35.10-72.fc14.i686)
because I guess this kernel doesn''t honor the compress attribute:

dd if=/dev/zero of=test.size count=1000
# du -B512 test.size
1000    test.size

But on a general note, we may read more (or possibly less)
than is stored in the extent. So how to detect that?
I suppose one could use lseek() to get the current position
and see if it''s ext_start + ext_length, otherwise adjust accordingly.
That would add a little overhead though.
I also notice the FIEMAP_EXTENT_DATA_ENCRYPTED and FIEMAP_EXTENT_ENCODED
flags, which could mean we only need to handle these extents specially.
Does `filefrag -v` show those for you? I see nothing here.

I don''t suppose there is any facility to read the raw data
to also avoid decompressing and compressing again (sendfile, mmap?).

cheers,
Pádraig.
--
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

based on other threads (which i havent been following too closely), did we 
settle on this being a btrfs bug ?
-mike

On 19/02/11 18:28, Mike Frysinger wrote:
> based on other threads (which i havent been following too closely), did we 
> settle on this being a btrfs bug ?
> -mike
Nope, cp 8.10 is not absolved yet.
It may be btrfs not honoring FIEMAP_FLAG_SYNC,
and/or it may be cp needing to handle FIEMAP_EXTENT_ENCODED
specially.

It would help if you ran `sync` before the copy,
to exclude that as a possible issue.

Also `filefrag -v` output for the file on
the compressed BTRFS file system would be helpful.

thanks,
Pádraig.
--
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

On 19/02/11 23:22, Pádraig Brady wrote:
> On 19/02/11 18:28, Mike Frysinger wrote:
>> based on other threads (which i havent been following too closely), did
we
>> settle on this being a btrfs bug ?
>> -mike
> 
> Nope, cp 8.10 is not absolved yet.
> It may be btrfs not honoring FIEMAP_FLAG_SYNC,
> and/or it may be cp needing to handle FIEMAP_EXTENT_ENCODED
> specially.
> 
> It would help if you ran `sync` before the copy,
> to exclude that as a possible issue.
> 
> Also `filefrag -v` output for the file on
> the compressed BTRFS file system would be helpful.
Hmm I just tried the latest Fedora 15 live image
and was tripped up a bit by https://bugzilla.redhat.com/show_bug.c

Anyway it uses 2.6.38 rc4 and "compress" doesn''t
seem to be implemented for BTRFS there?
Writing a file of zeros showed a normal disk usage,
and filefrag -v didn''t report any flags for the extents.

In any case, cp/fiemap-2 passed on each of
about 100 runs, even without introducing
and syncs.

cheers,
Pádraig.
--
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

2011/2/19 Pádraig Brady:
> On 19/02/11 18:28, Mike Frysinger wrote:
>> based on other threads (which i havent been following too closely), did
we
>> settle on this being a btrfs bug ?
>
> Nope, cp 8.10 is not absolved yet.
> It may be btrfs not honoring FIEMAP_FLAG_SYNC,
> and/or it may be cp needing to handle FIEMAP_EXTENT_ENCODED
> specially.
>
> It would help if you ran `sync` before the copy,
> to exclude that as a possible issue.
makes no difference after applying this patch:

--- coreutils-8.10/tests/cp/fiemap-2
+++ coreutils-8.10/tests/cp/fiemap-2
@@ -43,12 +43,19 @@
 printf x > k || framework_failure_
 dd bs=1k seek=1 of=k count=255 < /dev/zero || framework_failure_

+filefrag -v k
+sync
+filefrag -v k
+
 # cp should detect the all-zero blocks and convert some of them to holes.
 # How many it detects/converts currently depends on io_blksize.
 # Currently, on my F14/ext4 desktop, this K starts off with size 256KiB,
 # (note that the K in the preceding test starts off with size 4KiB).
 # cp from coreutils-8.9 with --sparse=always reduces the size to 32KiB.
 cp --sparse=always k k2 || fail=1
+
+sync
+
 test $(stat -c %b k2) -lt $(stat -c %b k) || fail=1

 Exit $fail
> Also `filefrag -v` output for the file on
> the compressed BTRFS file system would be helpful.
the output from the aforementioned patch:
...
+ dd bs=1k seek=1 of=k count=255
255+0 records in
255+0 records out
261120 bytes (261 kB) copied, 0.00313737 s, 83.2 MB/s
+ filefrag -v k
Filesystem type is: 9123683e
File size of k is 262144 (64 blocks, blocksize 4096)
 ext logical physical expected length flags
k: 1 extent found
+ sync
+ filefrag -v k
Filesystem type is: 9123683e
File size of k is 262144 (64 blocks, blocksize 4096)
 ext logical physical expected length flags
   0       0    47917              32
   1      32    47918    47948     32 eof
k: 2 extents found
...
-mike
--
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

Excerpts from Mike Frysinger''s message of 2011-02-19 19:53:18
-0500:
> 2011/2/19 Pádraig Brady:
> > On 19/02/11 18:28, Mike Frysinger wrote:
> >> based on other threads (which i havent been following too
closely), did we
> >> settle on this being a btrfs bug ?
> >
> > Nope, cp 8.10 is not absolved yet.
> > It may be btrfs not honoring FIEMAP_FLAG_SYNC,
> > and/or it may be cp needing to handle FIEMAP_EXTENT_ENCODED
> > specially.
> >
> > It would help if you ran `sync` before the copy,
> > to exclude that as a possible issue.
> 
> makes no difference after applying this patch:
I''ve got a fix for this on the btrfs side in testing.  It is definitely
a btrfs bug.

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

On 23/02/11 03:31, Chris Mason wrote:
> I''ve got a fix for this on the btrfs side in testing.  It is
definitely
> a btrfs bug.
[resend]

Cool.
I''m closing this ticket, so.

Interesting I didn''t notice this with
kernel-2.6.38-0.rc4.git0.2.fc15.i686
(with a 1G loop back BTRFS on ext4)

I''d appreciate a link to the fix at some stage.
If we know what''s going on exactly we might be
able to know how widespread the issue is and
whether we could work around it in `cp`.

cheers,
Pádraig.
--
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