GNU bug report logs -
#6131
[PATCH]: fiemap support for efficient sparse file copy
Previous Next
Reported by: "jeff.liu" <jeff.liu <at> oracle.com>
Date: Fri, 7 May 2010 14:16:02 UTC
Severity: normal
Tags: patch
Done: Jim Meyering <jim <at> meyering.net>
Bug is archived. No further changes may be made.
Full log
Message #338 received at submit <at> debbugs.gnu.org (full text, mbox):
On 29/01/11 09:47, Jim Meyering wrote:
> Jim Meyering wrote:
>> Jeff liu wrote:
>>> Now make check passed against the following combination:
>>> 1. Refresh installed host in Ubuntu10.0.4,
>>> filefrag comes from E2fsprogs 1.41.11 && Kernel: 2.6.32-16
>>> 2. filefrag in e2fsprogs-1.4.12 && kernel-2.6.36.
>> [passes]
>>
>> Glad to here it passes for you, now.
>> FYI, I have spent pretty much time on cp over the last
>> couple days, factoring out the hole-inducing code and
>> making extent_copy use it. Part of the motivation was
>> to fix cp --sparse=always, which was broken on the branch.
>> It would not induce holes when going through extent_copy.
>> I've added a couple more tests and will post the series as
>> soon I've cleaned things up a little more.
>
> Here are 9 more patches, just pushed to the fiemap-copy-2 branch:
>
> http://git.savannah.gnu.org/cgit/coreutils.git/log/?h=fiemap-copy-2
>
> The first and last add tests, and the others consolidate,
> clean up, and fix a few bugs.
>
> 1/9 tests: ensure that FIEMAP-enabled cp copies a sparse file efficiently
> Ensure that copying a sparse 1TiB file completes in less than 3 seconds
> That can only succeed with FIEMAP (or --reflink=, which is off by default)
>
> 2/9 fiemap copy: rename some locals
> The _logical suffix was not useful. Change it to _start
>
> 3/9 fiemap copy: simplify post-loop logic; improve comments
>
> 4/9 fiemap copy: avoid a performance hit due to very small buffer
> I didn't measure this, but once you see it, it's an obvious bug.
> Using an arbitrarily small buffer size is bound to cause trouble.
>
> 5/9 fiemap copy: avoid leak-on-error
> Failing from within the loop, we have to free the extent buffer.
>
> 6/9 copy: factor sparse-copying code into its own function, because
> we're going to have to use it from within extent_copy, too.
> I realized that cp --sparse=always could no longer create holes
> in the destination. Factoring this out is the first step.
>
> 7/9 copy: remove obsolete comment
> unrelated to the rest, but hard to pull out since it's in moved code
>
> 8/9 copy: make extent_copy use sparse_copy, rather than its own code
> Now that sparse_copy is separate, and used by copy_reg, adapt it
> so that it can also be used by extent_copy.
>
> 9/9 tests: cp/fiemap: exercise previously-failing parts
> This is a hole-inducing test that would have failed with previous
> fiemap-based copying code.
>
> I may change or remove the sparse_copy_finalize function, which just calls
> ftruncate, especially now that it's used from only one place (initially
> I was using it from each sparse_copy caller, but that didn't work out),
> and don't particularly like the added lseek call that is performed for
> each file copied, but keeping track of total written/offset byte counts
> and inflicting the need to do that on both callers seems like too much
> added code/complexity to justify avoiding that single lseek call.
>
>>From 8e4f0efd3ad17f1dd7a561369da22dfaf43ab3e8 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering <at> redhat.com>
> Date: Fri, 28 Jan 2011 22:31:23 +0100
> Subject: [PATCH 1/9] tests: ensure that FIEMAP-enabled cp copies a sparse file efficiently
>
> * tests/cp/fiemap-perf: New file.
> * tests/Makefile.am (TESTS): Add it.
> ---
> tests/Makefile.am | 1 +
> tests/cp/fiemap-perf | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 0 deletions(-)
> create mode 100755 tests/cp/fiemap-perf
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 847f181..7855ac5 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -320,6 +320,7 @@ TESTS = \
> cp/dir-vs-file \
> cp/existing-perm-race \
> cp/fail-perm \
> + cp/fiemap-perf \
> cp/file-perm-race \
> cp/into-self \
> cp/link \
> diff --git a/tests/cp/fiemap-perf b/tests/cp/fiemap-perf
> new file mode 100755
> index 0000000..429e59b
> --- /dev/null
> +++ b/tests/cp/fiemap-perf
> @@ -0,0 +1,32 @@
> +#!/bin/sh
> +# ensure that a sparse file is copied efficiently, by default
> +
> +# Copyright (C) 2011 Free Software Foundation, Inc.
> +
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +. "${srcdir=.}/init.sh"; path_prepend_ ../src
> +print_ver_ cp
> +
> +# Require a fiemap-enabled FS.
> +df -T -t btrfs -t xfs -t ext4 -t ocfs2 . \
> + || skip_ "this file system lacks FIEMAP support"
> +
> +# Create a large-but-sparse file.
> +timeout 1 dd bs=1 seek=1T of=f < /dev/null || framework_failure_
> +
> +# Nothing can read (much less write) that many bytes in so little time.
> +timeout 3 cp f f2 || framework_failure_
I'm a bit worried with a 1s timeout.
The following will only give false negatives over 100GB/s
timeout 10 truncate -s1T f || framework_failure_
timeout 10 cp f f2 || framework_failure_
I wouldn't worry about filling file systems either,
as we're already limiting to ext4 etc.
>>From 7f154dcfc5641c9616921d4c5ac5005bcb2507eb Mon Sep 17 00:00:00 20: Jim Meyering <meyering <at> redhat.com>
> Date: Thu, 27 Jan 2011 15:17:42 +0100
> Subject: [PATCH 9/9] tests: cp/fiemap: exercise previously-failing parts
> +# Ensure that --sparse=always can restore holes.
> +rm -f k
> +# Create a file starting with an "x", followed by 257K-1 0 bytes.
> +printf x > k || framework_failure_
> +dd bs=1k seek=1 of=k count=255 < /dev/zero || framework_failure_
S/257/256/ ?
This bug report was last modified 14 years and 119 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.