From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQDpD-0006xs-Hz for qemu-devel@nongnu.org; Thu, 21 Jul 2016 09:15:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQDpC-0004gF-48 for qemu-devel@nongnu.org; Thu, 21 Jul 2016 09:15:39 -0400 Date: Thu, 21 Jul 2016 23:15:24 +1000 From: Dave Chinner Message-ID: <20160721131524.GS2031@devil.localdomain> References: <20160720033402.GA7641@ad.usersys.redhat.com> <578EF446.70202@redhat.com> <20160720043709.GA10539@ad.usersys.redhat.com> <913397c9-6edc-2561-3d2e-e32032f9db22@redhat.com> <20160720073836.GF10539@ad.usersys.redhat.com> <1796238868.8815050.1469006377577.JavaMail.zimbra@redhat.com> <20160720123025.GO2031@devil.localdomain> <20160720133517.GE23632@ndevos-x240.usersys.redhat.com> <20160721114342.GQ2031@devil.localdomain> <5790C099.9020900@draigBrady.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <5790C099.9020900@draigBrady.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] semantics of FIEMAP without FIEMAP_FLAG_SYNC (was Re: [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?P=E1draig?= Brady Cc: Niels de Vos , Paolo Bonzini , Fam Zheng , Eric Blake , Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Max Reitz , Lukas Czerner On Thu, Jul 21, 2016 at 01:31:21PM +0100, P=E1draig Brady wrote: > On 21/07/16 12:43, Dave Chinner wrote: > > On Wed, Jul 20, 2016 at 03:35:17PM +0200, Niels de Vos wrote: > >> Oh... And I was surprised to learn that "cp" does use FIEMAP and not > >> SEEK_HOLE/SEEK_DATA. You give a strong suggestion to fix that. > >> http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/extent-sca= n.c#n87 > >=20 > > My understanding was that FIEMAP was disabled almost immediately > > after the data corruption problems were understood, and it hasn't > > been turned back on since. I don't see FIEMAP calls in strace when I > > copy sparse files, even when I use --sparse=3Dauto which is supposed > > to trigger the sparse source file checks using FIEMAP. > >=20 > > So, yeah, while there's FIEMAP code present, that doesn't mean it's > > actually used. And, yes, there are comments in coreutils commits in > > 2011 saying that the FIEMAP workarounds are temporary until > > SEK_HOLE/DATA are supported, which were added to the kernel in 2011 > > soon after the 'cp-corrupts-data-with-fiemap' debacle came to light. > > Five years later, and the fiemap code is stil there? >=20 > Yes it's still there, and hasn't caused issues. > It's used only for sparse files: >=20 > $ truncate -s1G t.fiemap > $ strace -e ioctl cp t.fiemap t2.fiemap > ioctl(3, FS_IOC_FIEMAP, 0x7ffff007c910) =3D 0 So, my tests were done with coreutils 8.25, and a sparse 100M file with a layout like this: $ xfs_bmap -vvp testfile testfile: EXT: FILE-OFFSET BLOCK-RANGE AG AG-OFFSET TOTAL = FLAGS 0: [0..20479]: 71768912..71789391 3 (11216720..11237199) 20480 = 00000 1: [20480..40959]: hole 20480 2: [40960..61439]: 71809872..71830351 3 (11257680..11278159) 20480 = 00000 3: [61440..81919]: 71830352..71850831 3 (11278160..11298639) 20480 = 10000 4: [81920..96223]: 71850832..71865135 3 (11298640..11312943) 14304 = 00000 5: [96224..190471]: 60457944..60552191 2 (20089816..20184063) 94248 = 00000 6: [190472..204799]: 73101864..73116191 3 (12549672..12563999) 14328 = 00000 FLAG Values: 010000 Unwritten preallocated extent 001000 Doesn't begin on stripe unit 000100 Doesn't end on stripe unit 000010 Doesn't begin on stripe width 000001 Doesn't end on stripe width i.e. a 10MB hole @ 10MB, a 10MB unwritten extent @ 30MB. And, yes, I do see a fiemap call with your example above. IOWs, it seems the "is sparse" heuristic that cp uses is not very reliable. That doesn't inspire confidence, and explains why none of my cp tests on sparse files ove rthe past 5 years have ever shown FIEMAP calls.... /me is now somewhat afraid to use cp for copies that require data integrity. > It's a pity fiemap is racy (on some file systems?) wrt reporting > of data not yet written out, and is thus fairly useless IMHO > without the FIEMAP_FLAG_SYNC workaround. It's racy on *all filesystems* - all the FIEMAP_FLAG_SYNC flag does is make the race window smaller. The extent map is only coherent with the data in memory while the inode locks are held by the kernel. The moment the inode locks are dropped by the kernel in the syscall, the extent map held in the fiemap structure is considered stale and no longer coherent with the extent map held on the inode. Cheers, Dave. --=20 Dave Chinner dchinner@redhat.com