From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPqeB-0007A2-RJ for qemu-devel@nongnu.org; Wed, 20 Jul 2016 08:30:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPqeA-0007Rv-9h for qemu-devel@nongnu.org; Wed, 20 Jul 2016 08:30:43 -0400 Date: Wed, 20 Jul 2016 22:30:25 +1000 From: Dave Chinner Message-ID: <20160720123025.GO2031@devil.localdomain> References: <1468901281-22858-1-git-send-email-eblake@redhat.com> <578E4708.5080308@redhat.com> <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1796238868.8815050.1469006377577.JavaMail.zimbra@redhat.com> 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: Paolo Bonzini Cc: Fam Zheng , Eric Blake , Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Max Reitz , Lukas Czerner , P@draigBrady.com, Niels de Vos On Wed, Jul 20, 2016 at 05:19:37AM -0400, Paolo Bonzini wrote: > Adding ext4 and XFS guys (Lukas and Dave respectively). As a quick recap, the > issue here is the semantics of FIEMAP and SEEK_HOLE/SEEK_DATA, which we use in > "qemu-img map". This command prints metadata about a virtual disk image---which > in the case of a raw image amounts to detecting holes and unwritten extents. > > First, it seems like SEEK_HOLE and SEEK_DATA really should be "SEEK_NONZERO" and > "SEEK_ZERO", on both ext4 and XFS. You can see that unwritten extents are > reported by "qemu-img map" as holes: Correctly so. seek hole/data knows nothing about the underlying filesystem storage implementation. A "hole" is defined as a region of zeros, and the filesystem is free call anything it kows for certain contains zeros as a hole. FYI, SEEK_HOLE/DATA were named after the the pre-existing Solaris seek API that uses these definitions for hole and data.... > $ dd if=/dev/urandom of=test.img bs=1M count=100 > $ fallocate -z -o 10M -l 10M test.img > $ du -h test.img > $ qemu-img map --output=json test.img > [{ "start": 0, "length": 10485760, "depth": 0, "zero": false, "data": true, "offset": 0}, > { "start": 10485760, "length": 10485760, "depth": 0, "zero": true, "data": false, "offset": 10485760}, > { "start": 20971520, "length": 83886080, "depth": 0, "zero": false, "data": true, "offset": 20971520}] > > On the second line, zero=true data=false identifies a hole. The right output > would either have zero=true data=true (unwritten extent) or just > > [{ "start": 0, "length": 104857600, "depth": 0, "zero": false, "data": true, "offset": 0}, > > since the zero flag is advisory (it doesn't try to detect zeroes beyond what the > filesystem says). Not from SEEK_HOLE/SEEK_DATA. All it conveys is "this is the start of a range of zeros" and "this is the start of a range of data". And for filesystems that don't specifically implement these seek operations, zeros are considered data. i.e. SEEK_HOLE will take you to the end of file, SEEK_DATA returns the current position.... i.e. unwritten extents contain no data, so they are semantically identical to holes for the purposes of seeking and hence SEEK_DATA can skip over them. > The reason why we disabled FIEMAP was a combination of a corruption and performance > issue. The data corruption bug was at https://bugs.launchpad.net/qemu/+bug/1368815 > and it was reported on Ubuntu Trusty (kernel 3.13 based on the release notes at > https://wiki.ubuntu.com/TrustyTahr/ReleaseNotes). We corrected that by using > FIEMAP_FLAG_SYNC, based on a similar patch to coreutils. This turned out to be too > slow, so we dropped FIEMAP altogether. Yes, because FIEMAP output is only useful for diagnostic purposes as it can be stale even before the syscall returns to userspace. i.e. it can't be used in userspace for optimising file copies.... Finding regions of valid data in a file is what SEEK_HOLE/SEEK_DATA is for, not FIEMAP. FIEMAP only reports the physical layout of the file, now where the currently valid data in the file lies. > However, today I found kernel commit 91dd8c114499 ("ext4: prevent race while walking > extent tree for fiemap", 2012-11-28) whose commit message says: > > Moreover the extent currently in delayed allocation might be allocated > after we search the extent tree and before we search extent status tree > delayed buffers resulting in those delayed buffers being completely > missed, even though completely written and allocated. > > This seems pretty much like our data corruption bug; it would mean that > using FIEMAP_FLAG_SYNC was only working around a bug and delayed allocations > _should_ be reported as usual by FIEMAP. What do you think you can do with the delayed allocation regions in the file? Indeed, with specualtive delayed allocation, XFS can report delalloc regions that have no actual resemblence to the layout of dirty data in the file. SEEK_DATA will iterate the delalloc regions containing data precisely, however. > Except that the commit went in kernel 3.8 and as said above Trusty had 3.13. > So either there are other bugs, or my understanding of the commit is not correct. > So the questions for Lukas and Dave are: > > 1) is it expected that SEEK_HOLE skips unwritten extents? There are multiple answers to this, all of which are correct depending on current context and state: 1. No - some filesystems will report clean unwritten extents as holes. 2. Yes - some filesystems will report clean unwritten extents as data. 3. Maybe - if there is written data in memory over the unwritten extent on disk (i.e. hasn't been flushed to disk, it will be considered a data region with non-zero data. (FIEMAP will still report is as unwritten) > If not, would > it be acceptable to introduce Linux-specific SEEK_ZERO/SEEK_NONZERO, which > would be similar to what SEEK_HOLE/SEEK_DATA do now? To solve what problem? You haven't explained what problem you are trying to solve yet. > 2) for FIEMAP do we really need FIEMAP_FLAG_SYNC? And if not, for what > filesystems and kernel releases is it really not needed? I can't answer this question, either, because I don't know what you want the fiemap information for. Cheers, Dave. -- Dave Chinner dchinner@redhat.com