From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7260AC433DB for ; Tue, 12 Jan 2021 14:04:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 331E122CE3 for ; Tue, 12 Jan 2021 14:04:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727144AbhALOE2 (ORCPT ); Tue, 12 Jan 2021 09:04:28 -0500 Received: from mx2.suse.de ([195.135.220.15]:35362 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726560AbhALOE1 (ORCPT ); Tue, 12 Jan 2021 09:04:27 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id B7B07ABD6; Tue, 12 Jan 2021 14:03:45 +0000 (UTC) Received: by ds.suse.cz (Postfix, from userid 10065) id 68FA8DA87C; Tue, 12 Jan 2021 15:01:53 +0100 (CET) Date: Tue, 12 Jan 2021 15:01:53 +0100 From: David Sterba To: fdmanana@kernel.org Cc: linux-btrfs@vger.kernel.org Subject: Re: [PATCH] btrfs: send, fix invalid clone operations when cloning from the same file and root Message-ID: <20210112140153.GO6430@twin.jikos.cz> Reply-To: dsterba@suse.cz Mail-Followup-To: dsterba@suse.cz, fdmanana@kernel.org, linux-btrfs@vger.kernel.org References: <900493c40f7edbd42fe861ccd9a68851ea952499.1610363502.git.fdmanana@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <900493c40f7edbd42fe861ccd9a68851ea952499.1610363502.git.fdmanana@suse.com> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Mon, Jan 11, 2021 at 11:41:42AM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana > > When an incremental send finds an extent that is shared, it checks which > file extent items in the range refer to that extent, and for those it > emits clone operations, while for others it emits regular write operations > to avoid corruption at the destination (as described and fixed by commit > d906d49fc5f4 ("Btrfs: send, fix file corruption due to incorrect cloning > operations")). > > However when the root we are cloning from is the send root, we are cloning > from the inode currently being processed and the source file range has > several extent items that partially point to the desired extent, with an > offset smaller than the offset in the file extent item for the range we > want to clone into, it can cause the algorithm to issue a clone operation > that starts at the current eof of the file being processed in the receiver > side, in which case the receiver will fail, with -EINVAL, when attempting > to execute the clone operation. > > Example reproducer: > > $ cat test-send-clone.sh > #!/bin/bash > > DEV=/dev/sdi > MNT=/mnt/sdi > > mkfs.btrfs -f $DEV >/dev/null > mount $DEV $MNT > > # Create our test file with a single and large extent (1M) and with > # different content for different file ranges that will be reflinked > # later. > xfs_io -f \ > -c "pwrite -S 0xab 0 128K" \ > -c "pwrite -S 0xcd 128K 128K" \ > -c "pwrite -S 0xef 256K 256K" \ > -c "pwrite -S 0x1a 512K 512K" \ > $MNT/foobar > > btrfs subvolume snapshot -r $MNT $MNT/snap1 > btrfs send -f /tmp/snap1.send $MNT/snap1 > > # Now do a series of changes to our file such that we end up with > # different parts of the extent reflinked into different file offsets > # and we overwrite a large part of the extent too, so no file extent > # items refer to that part that was overwritten. This used to confure > # the algorithm used by the kernel to figure out which file ranges to > # clone, making it attempt to clone from a source range starting at > # the current eof of the file, resulting in the receiver to fail since > # it is an invalid clone operation. > # > xfs_io -c "reflink $MNT/foobar 64K 1M 960K" \ > -c "reflink $MNT/foobar 0K 512K 256K" \ > -c "reflink $MNT/foobar 512K 128K 256K" \ > -c "pwrite -S 0x73 384K 640K" \ > $MNT/foobar > > btrfs subvolume snapshot -r $MNT $MNT/snap2 > btrfs send -f /tmp/snap2.send -p $MNT/snap1 $MNT/snap2 > > echo -e "\nFile digest in the original filesystem:" > md5sum $MNT/snap2/foobar > > # Now unmount the filesystem, create a new one, mount it and try to > # apply both send streams to recreate both snapshots. > umount $DEV > > mkfs.btrfs -f $DEV >/dev/null > mount $DEV $MNT > > btrfs receive -f /tmp/snap1.send $MNT > btrfs receive -f /tmp/snap2.send $MNT > > # Must match what we got in the original filesystem of course. > echo -e "\nFile digest in the new filesystem:" > md5sum $MNT/snap2/foobar > > umount $MNT > > When running the reproducer, the incremental send operation fails due to > an invalid clone operation: > > $ ./test-send-clone.sh > wrote 131072/131072 bytes at offset 0 > 128 KiB, 32 ops; 0.0015 sec (80.906 MiB/sec and 20711.9741 ops/sec) > wrote 131072/131072 bytes at offset 131072 > 128 KiB, 32 ops; 0.0013 sec (90.514 MiB/sec and 23171.6148 ops/sec) > wrote 262144/262144 bytes at offset 262144 > 256 KiB, 64 ops; 0.0025 sec (98.270 MiB/sec and 25157.2327 ops/sec) > wrote 524288/524288 bytes at offset 524288 > 512 KiB, 128 ops; 0.0052 sec (95.730 MiB/sec and 24506.9883 ops/sec) > Create a readonly snapshot of '/mnt/sdi' in '/mnt/sdi/snap1' > At subvol /mnt/sdi/snap1 > linked 983040/983040 bytes at offset 1048576 > 960 KiB, 1 ops; 0.0006 sec (1.419 GiB/sec and 1550.3876 ops/sec) > linked 262144/262144 bytes at offset 524288 > 256 KiB, 1 ops; 0.0020 sec (120.192 MiB/sec and 480.7692 ops/sec) > linked 262144/262144 bytes at offset 131072 > 256 KiB, 1 ops; 0.0018 sec (133.833 MiB/sec and 535.3319 ops/sec) > wrote 655360/655360 bytes at offset 393216 > 640 KiB, 160 ops; 0.0093 sec (66.781 MiB/sec and 17095.8436 ops/sec) > Create a readonly snapshot of '/mnt/sdi' in '/mnt/sdi/snap2' > At subvol /mnt/sdi/snap2 > > File digest in the original filesystem: > 9c13c61cb0b9f5abf45344375cb04dfa /mnt/sdi/snap2/foobar > At subvol snap1 > At snapshot snap2 > ERROR: failed to clone extents to foobar: Invalid argument > > File digest in the new filesystem: > 132f0396da8f48d2e667196bff882cfc /mnt/sdi/snap2/foobar > > The clone operation is invalid because its source range starts at the > current eof of the file in the receiver, causing the receiver to get > an -EINVAL error from the clone operation when attempting it. > > For the example above, what happens is the following: > > 1) When processing the extent at file offset 1M, the algorithm checks that > the extent is shared and can be (fully or partially) found at file > offset 0. > > At this point the file has a size (and eof) of 1M at the receiver; > > 2) It finds that our extent item at file offset 1M has a data offset of > 64K and, since the file extent item at file offset 0 has a data offset > of 0, it issues a clone operation, from the same file and root, that > has a source range offset of 64K, destination offset of 1M and a length > of 64K, since the extent item at file offset 0 refers only to the first > 128K of the shared extent. > > After this clone operation, the file size (and eof) at the receiver is > increased from 1M to 1088K (1M + 64K); > > 3) Now there's still 896K (960K - 64K) of data left to clone or write, so > it checks for the next file extent item, which starts at file offset > 128K. This file extent item has a data offset of 0 and a length of > 256K, so a clone operation with a source range offset of 256K, a > destination offset of 1088K (1M + 64K) and length of 128K is issued. > > After this operation the file size (and eof) at the receiver increases > from 1088K to 1216K (1088K + 128K); > > 4) Now there's still 768K (896K - 128K) of data left to clone or write, so > it checks for the next file extent item, located at file offset 384K. > This file extent item points to a different extent, not the one we want > to clone, with a length of 640K. So we issue a write operation into the > file range 1216K (1088K + 128K, end of the last clone operation), with > a length of 640K and with a data matching the one we can find for that > range in send root. > > After this operation, the file size (and eof) at the receiver increases > from 1216K to 1856K (1216K + 640K); > > 5) Now there's still 128K (768K - 640K) of data left to clone or write, so > we look into the file extent item, which is for file offset 1M and it > points to the extent we want to clone, with a data offset of 64K and a > length of 960K. > > However this matches the file offset we started with, the start of the > range to clone into. So we can't for sure find any file extent item > from here onwards with the rest of the data we want to clone, yet we > proceed and since the file extent item points to the shared extent, > with a data offset of 64K, we issue a clone operation with a source > range starting at file offset 1856K, which matches the file extent > item's offset, 1M, plus the amount of data cloned and written so far, > which is 64K (step 2) + 128K (step 3) + 640K (step 4). This clone > operation is invalid since the source range offset matches the current > eof of the file in the receiver. We should have stopped looking for > extents to clone at this point and instead fallback to write, which > would simply the contain the data in the file range from 1856K to > 1856K + 128K. > > So fix this by stopping the loop that looks for file ranges to clone at > clone_range() when we reach the current eof of the file being processed, > if we are cloning from the same file and using the send root as the clone > root. This ensures any data not yet cloned will be sent to the receiver > through a write operation. > > A test case for fstests will follow soon. > > Reported-by: Massimo B. > Link: https://lore.kernel.org/linux-btrfs/6ae34776e85912960a253a8327068a892998e685.camel@gmx.net/ > Fixes: 11f2069c113e ("Btrfs: send, allow clone operations within the same file") > CC: stable@vger.kernel.org # 5.5+ > Signed-off-by: Filipe Manana Added to misc-next, thanks.