[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#6131: [PATCH]: fiemap support for efficient sparse file copy
|
From: |
Jim Meyering |
|
Subject: |
bug#6131: [PATCH]: fiemap support for efficient sparse file copy |
|
Date: |
Sat, 29 Jan 2011 10:47:53 +0100 |
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 <address@hidden>
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_
+
+Exit $fail
--
1.7.3.5.44.g960a
>From dd380c3d672f78adb4cb907e8658db6b3962a281 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 18:28:25 +0100
Subject: [PATCH 2/9] fiemap copy: rename some locals
(extent_copy): Rename locals: s/*ext_logical/*ext_start/
---
src/copy.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index c9cc2f7..e164ab7 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -200,7 +200,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
bool *require_normal_copy)
{
struct extent_scan scan;
- off_t last_ext_logical = 0;
+ off_t last_ext_start = 0;
uint64_t last_ext_len = 0;
uint64_t last_read_size = 0;
@@ -228,10 +228,10 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
unsigned int i;
for (i = 0; i < scan.ei_count; i++)
{
- off_t ext_logical = scan.ext_info[i].ext_logical;
+ off_t ext_start = scan.ext_info[i].ext_logical;
uint64_t ext_len = scan.ext_info[i].ext_length;
- if (lseek (src_fd, ext_logical, SEEK_SET) < 0)
+ if (lseek (src_fd, ext_start, SEEK_SET) < 0)
{
error (0, errno, _("cannot lseek %s"), quote (src_name));
return false;
@@ -239,7 +239,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
if (make_holes)
{
- if (lseek (dest_fd, ext_logical, SEEK_SET) < 0)
+ if (lseek (dest_fd, ext_start, SEEK_SET) < 0)
{
error (0, errno, _("cannot lseek %s"), quote (dst_name));
return false;
@@ -249,10 +249,10 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
{
/* We're not inducing holes; write zeros to the destination file
if there is a hole between the last and current extent. */
- if (last_ext_logical + last_ext_len < ext_logical)
+ if (last_ext_start + last_ext_len < ext_start)
{
- uint64_t hole_size = (ext_logical
- - last_ext_logical
+ uint64_t hole_size = (ext_start
+ - last_ext_start
- last_ext_len);
if (! write_zeros (dest_fd, hole_size))
{
@@ -262,7 +262,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
}
- last_ext_logical = ext_logical;
+ last_ext_start = ext_start;
last_ext_len = ext_len;
last_read_size = 0;
@@ -313,7 +313,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
file. */
if (make_holes)
{
- if (last_ext_logical + last_read_size < src_total_size)
+ if (last_ext_start + last_read_size < src_total_size)
{
if (ftruncate (dest_fd, src_total_size) < 0)
{
@@ -324,9 +324,9 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
else
{
- if (last_ext_logical + last_ext_len < src_total_size)
+ if (last_ext_start + last_ext_len < src_total_size)
{
- uint64_t holes_len = src_total_size - last_ext_logical -
last_ext_len;
+ uint64_t holes_len = src_total_size - last_ext_start - last_ext_len;
if (0 < holes_len)
{
if (! write_zeros (dest_fd, holes_len))
--
1.7.3.5.44.g960a
>From d1067e37b0e4b945ab901e98d6eedb249fa2a42c Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 19:00:48 +0100
Subject: [PATCH 3/9] fiemap copy: simplify post-loop logic; improve comments
* src/copy.c (extent_copy): Avoid duplication in post-loop
extend-to-desired-length code.
---
src/copy.c | 44 +++++++++++++++-----------------------------
1 files changed, 15 insertions(+), 29 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index e164ab7..ab18a76 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -268,8 +268,8 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
while (ext_len)
{
- /* Avoid reading into the holes if the left extent
- length is shorter than the buffer size. */
+ /* Don't read from a following hole if EXT_LEN
+ is smaller than the buffer size. */
buf_size = MIN (ext_len, buf_size);
ssize_t n_read = read (src_fd, buf, buf_size);
@@ -285,7 +285,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
if (n_read == 0)
{
- /* Record number of bytes read from the previous extent. */
+ /* Record number of bytes read from this extent-at-EOF. */
last_read_size = last_ext_len - ext_len;
break;
}
@@ -306,33 +306,19 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
while (! scan.hit_final_extent);
- /* If a file ends up with holes, the sum of the last extent logical offset
- and the read-returned size or the last extent length will be shorter than
- the actual size of the file. Use ftruncate to extend the length of the
- destination file if make_holes, or write zeros up to the actual size of
the
- file. */
- if (make_holes)
- {
- if (last_ext_start + last_read_size < src_total_size)
- {
- if (ftruncate (dest_fd, src_total_size) < 0)
- {
- error (0, errno, _("failed to extend %s"), quote (dst_name));
- return false;
- }
- }
- }
- else
- {
- if (last_ext_start + last_ext_len < src_total_size)
- {
- uint64_t holes_len = src_total_size - last_ext_start - last_ext_len;
- if (0 < holes_len)
- {
- if (! write_zeros (dest_fd, holes_len))
- return false;
- }
- }
+ /* When the source file ends with a hole, the sum of the last extent start
+ offset and (the read-returned size or the last extent length) is smaller
+ than the actual size of the file. In that case, extend the destination
+ file to the required length. When MAKE_HOLES is set, use ftruncate;
+ otherwise, use write_zeros. */
+ uint64_t eof_hole_len = (src_total_size - last_ext_start
+ - (last_read_size ? last_read_size : last_ext_len));
+ if (eof_hole_len && (make_holes
+ ? ftruncate (dest_fd, src_total_size)
+ : ! write_zeros (dest_fd, eof_hole_len)))
+ {
+ error (0, errno, _("failed to extend %s"), quote (dst_name));
+ return false;
}
return true;
--
1.7.3.5.44.g960a
>From 33f4a4a549afb3de94e546091c91586a1ece67ba Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 17:30:08 +0100
Subject: [PATCH 4/9] fiemap copy: avoid a performance hit due to very small
buffer
* src/copy.c (extent_copy): Don't let what should have been a
temporary reduction of buf_size (to handle a short ext_len) become
permanent and thus impact the performance of all further iterations.
---
src/copy.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index ab18a76..9a3a8f7 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -270,9 +270,8 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
{
/* Don't read from a following hole if EXT_LEN
is smaller than the buffer size. */
- buf_size = MIN (ext_len, buf_size);
-
- ssize_t n_read = read (src_fd, buf, buf_size);
+ size_t b_size = MIN (ext_len, buf_size);
+ ssize_t n_read = read (src_fd, buf, b_size);
if (n_read < 0)
{
#ifdef EINTR
--
1.7.3.5.44.g960a
>From 47c8476ec9629239c82caf50b1c68b7bc58ba2d6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 17:49:04 +0100
Subject: [PATCH 5/9] fiemap copy: avoid leak-on-error
* src/copy.c (extent_copy): Don't leak an extent_scan buffer on
failed lseek, read, or write.
---
src/copy.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 9a3a8f7..208e463 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -234,6 +234,8 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
if (lseek (src_fd, ext_start, SEEK_SET) < 0)
{
error (0, errno, _("cannot lseek %s"), quote (src_name));
+ fail:
+ extent_scan_free (&scan);
return false;
}
@@ -242,7 +244,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
if (lseek (dest_fd, ext_start, SEEK_SET) < 0)
{
error (0, errno, _("cannot lseek %s"), quote (dst_name));
- return false;
+ goto fail;
}
}
else
@@ -257,7 +259,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
if (! write_zeros (dest_fd, hole_size))
{
error (0, errno, _("%s: write failed"), quote
(dst_name));
- return false;
+ goto fail;
}
}
}
@@ -279,7 +281,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
continue;
#endif
error (0, errno, _("reading %s"), quote (src_name));
- return false;
+ goto fail;
}
if (n_read == 0)
@@ -292,7 +294,7 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
if (full_write (dest_fd, buf, n_read) != n_read)
{
error (0, errno, _("writing %s"), quote (dst_name));
- return false;
+ goto fail;
}
ext_len -= n_read;
--
1.7.3.5.44.g960a
>From c0b7bc3864c06ea12c2740056e28623449fb63a7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 20:57:17 +0100
Subject: [PATCH 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.
* src/copy.c (sparse_copy): New function, factored out of...
(copy_reg): ...here.
Remove now-unused locals.
---
src/copy.c | 212 ++++++++++++++++++++++++++++++++----------------------------
1 files changed, 114 insertions(+), 98 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 208e463..cc8f68f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -134,6 +134,116 @@ utimens_symlink (char const *file, struct timespec const
*timespec)
return err;
}
+/* Copy the regular file open on SRC_FD/SRC_NAME to DST_FD/DST_NAME,
+ honoring the MAKE_HOLES setting and using the BUF_SIZE-byte buffer
+ BUF for temporary storage. Return true upon successful completion;
+ print a diagnostic and return false upon error. */
+static bool
+sparse_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
+ bool make_holes,
+ char const *src_name, char const *dst_name)
+{
+ typedef uintptr_t word;
+ off_t n_read_total = 0;
+ bool last_write_made_hole = false;
+
+ while (true)
+ {
+ word *wp = NULL;
+
+ ssize_t n_read = read (src_fd, buf, buf_size);
+ if (n_read < 0)
+ {
+#ifdef EINTR
+ if (errno == EINTR)
+ continue;
+#endif
+ error (0, errno, _("reading %s"), quote (src_name));
+ return false;
+ }
+ if (n_read == 0)
+ break;
+
+ n_read_total += n_read;
+
+ if (make_holes)
+ {
+ char *cp;
+
+ /* Sentinel to stop loop. */
+ buf[n_read] = '\1';
+#ifdef lint
+ /* Usually, buf[n_read] is not the byte just before a "word"
+ (aka uintptr_t) boundary. In that case, the word-oriented
+ test below (*wp++ == 0) would read some uninitialized bytes
+ after the sentinel. To avoid false-positive reports about
+ this condition (e.g., from a tool like valgrind), set the
+ remaining bytes -- to any value. */
+ memset (buf + n_read + 1, 0, sizeof (word) - 1);
+#endif
+
+ /* Find first nonzero *word*, or the word with the sentinel. */
+
+ wp = (word *) buf;
+ while (*wp++ == 0)
+ continue;
+
+ /* Find the first nonzero *byte*, or the sentinel. */
+
+ cp = (char *) (wp - 1);
+ while (*cp++ == 0)
+ continue;
+
+ if (cp <= buf + n_read)
+ /* Clear to indicate that a normal write is needed. */
+ wp = NULL;
+ else
+ {
+ /* We found the sentinel, so the whole input block was zero.
+ Make a hole. */
+ if (lseek (dest_fd, n_read, SEEK_CUR) < 0)
+ {
+ error (0, errno, _("cannot lseek %s"), quote (dst_name));
+ return false;
+ }
+ last_write_made_hole = true;
+ }
+ }
+
+ if (!wp)
+ {
+ size_t n = n_read;
+ if (full_write (dest_fd, buf, n) != n)
+ {
+ error (0, errno, _("writing %s"), quote (dst_name));
+ return false;
+ }
+ last_write_made_hole = false;
+
+ /* It is tempting to return early here upon a short read from a
+ regular file. That would save the final read syscall for each
+ file. Unfortunately that doesn't work for certain files in
+ /proc with linux kernels from at least 2.6.9 .. 2.6.29. */
+ }
+ }
+
+ /* If the file ends with a `hole', we need to do something to record
+ the length of the file. On modern systems, calling ftruncate does
+ the job. On systems without native ftruncate support, we have to
+ write a byte at the ending position. Otherwise the kernel would
+ truncate the file at the end of the last write operation. */
+ if (last_write_made_hole)
+ {
+ if (ftruncate (dest_fd, n_read_total) < 0)
+ {
+ error (0, errno, _("truncating %s"), quote (dst_name));
+ return false;
+ }
+ }
+
+ return true;
+}
+
/* Perform the O(1) btrfs clone operation, if possible.
Upon success, return 0. Otherwise, return -1 and set errno. */
static inline int
@@ -824,7 +934,6 @@ copy_reg (char const *src_name, char const *dst_name,
if (data_copy_required)
{
typedef uintptr_t word;
- off_t n_read_total = 0;
/* Choose a suitable buffer size; it may be adjusted later. */
size_t buf_alignment = lcm (getpagesize (), sizeof (word));
@@ -832,7 +941,6 @@ copy_reg (char const *src_name, char const *dst_name,
size_t buf_size = io_blksize (sb);
/* Deal with sparse files. */
- bool last_write_made_hole = false;
bool make_holes = false;
if (S_ISREG (sb.st_mode))
@@ -897,103 +1005,11 @@ copy_reg (char const *src_name, char const *dst_name,
goto close_src_and_dst_desc;
}
- while (true)
+ if ( ! sparse_copy (source_desc, dest_desc, buf, buf_size,
+ make_holes, src_name, dst_name))
{
- word *wp = NULL;
-
- ssize_t n_read = read (source_desc, buf, buf_size);
- if (n_read < 0)
- {
-#ifdef EINTR
- if (errno == EINTR)
- continue;
-#endif
- error (0, errno, _("reading %s"), quote (src_name));
- return_val = false;
- goto close_src_and_dst_desc;
- }
- if (n_read == 0)
- break;
-
- n_read_total += n_read;
-
- if (make_holes)
- {
- char *cp;
-
- /* Sentinel to stop loop. */
- buf[n_read] = '\1';
-#ifdef lint
- /* Usually, buf[n_read] is not the byte just before a "word"
- (aka uintptr_t) boundary. In that case, the word-oriented
- test below (*wp++ == 0) would read some uninitialized bytes
- after the sentinel. To avoid false-positive reports about
- this condition (e.g., from a tool like valgrind), set the
- remaining bytes -- to any value. */
- memset (buf + n_read + 1, 0, sizeof (word) - 1);
-#endif
-
- /* Find first nonzero *word*, or the word with the sentinel. */
-
- wp = (word *) buf;
- while (*wp++ == 0)
- continue;
-
- /* Find the first nonzero *byte*, or the sentinel. */
-
- cp = (char *) (wp - 1);
- while (*cp++ == 0)
- continue;
-
- if (cp <= buf + n_read)
- /* Clear to indicate that a normal write is needed. */
- wp = NULL;
- else
- {
- /* We found the sentinel, so the whole input block was zero.
- Make a hole. */
- if (lseek (dest_desc, n_read, SEEK_CUR) < 0)
- {
- error (0, errno, _("cannot lseek %s"), quote (dst_name));
- return_val = false;
- goto close_src_and_dst_desc;
- }
- last_write_made_hole = true;
- }
- }
-
- if (!wp)
- {
- size_t n = n_read;
- if (full_write (dest_desc, buf, n) != n)
- {
- error (0, errno, _("writing %s"), quote (dst_name));
- return_val = false;
- goto close_src_and_dst_desc;
- }
- last_write_made_hole = false;
-
- /* It is tempting to return early here upon a short read from a
- regular file. That would save the final read syscall for each
- file. Unfortunately that doesn't work for certain files in
- /proc with linux kernels from at least 2.6.9 .. 2.6.29. */
- }
- }
-
- /* If the file ends with a `hole', we need to do something to record
- the length of the file. On modern systems, calling ftruncate does
- the job. On systems without native ftruncate support, we have to
- write a byte at the ending position. Otherwise the kernel would
- truncate the file at the end of the last write operation. */
-
- if (last_write_made_hole)
- {
- if (ftruncate (dest_desc, n_read_total) < 0)
- {
- error (0, errno, _("truncating %s"), quote (dst_name));
- return_val = false;
- goto close_src_and_dst_desc;
- }
+ return_val = false;
+ goto close_src_and_dst_desc;
}
}
--
1.7.3.5.44.g960a
>From 80038c3cba2dee9c6c41ab6a28a1233a538ee2ee Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 21:01:07 +0100
Subject: [PATCH 7/9] copy: remove obsolete comment
* src/copy.c (sparse_copy): Remove now-obsolete comment about
how we used to work around lack of ftruncate. Combine nested
if conditions into one.
---
src/copy.c | 21 +++++++++------------
1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index cc8f68f..4bfdce6 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -137,7 +137,10 @@ utimens_symlink (char const *file, struct timespec const
*timespec)
/* Copy the regular file open on SRC_FD/SRC_NAME to DST_FD/DST_NAME,
honoring the MAKE_HOLES setting and using the BUF_SIZE-byte buffer
BUF for temporary storage. Return true upon successful completion;
- print a diagnostic and return false upon error. */
+ print a diagnostic and return false upon error.
+ Note that for best results, BUF should be "well"-aligned.
+ BUF must have sizeof(uintptr_t)-1 bytes of additional space
+ beyond BUF[BUF_SIZE-1]. */
static bool
sparse_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
bool make_holes,
@@ -227,18 +230,12 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
}
- /* If the file ends with a `hole', we need to do something to record
- the length of the file. On modern systems, calling ftruncate does
- the job. On systems without native ftruncate support, we have to
- write a byte at the ending position. Otherwise the kernel would
- truncate the file at the end of the last write operation. */
- if (last_write_made_hole)
+ /* If the file ends with a `hole', we need to do something to record the
+ length of the file. On modern systems, calling ftruncate does the job.
*/
+ if (last_write_made_hole && ftruncate (dest_fd, n_read_total) < 0)
{
- if (ftruncate (dest_fd, n_read_total) < 0)
- {
- error (0, errno, _("truncating %s"), quote (dst_name));
- return false;
- }
+ error (0, errno, _("truncating %s"), quote (dst_name));
+ return false;
}
return true;
--
1.7.3.5.44.g960a
>From f161ba3fcd5832d1344224ec41627cace5d73544 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 28 Jan 2011 21:19:50 +0100
Subject: [PATCH 8/9] copy: make extent_copy use sparse_copy, rather than its
own code
* src/copy.c (extent_copy): Before this change, extent_copy would fail
to create holes, thus breaking --sparse=auto and --sparse=always.
I.e., copying a large enough file of all zeros, cp --sparse=always
should introduce a hole, but with extent_copy, it would not.
---
src/copy.c | 109 +++++++++++++++++++++++++++---------------------------------
1 files changed, 49 insertions(+), 60 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 4bfdce6..96bb35b 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -136,25 +136,28 @@ utimens_symlink (char const *file, struct timespec const
*timespec)
/* Copy the regular file open on SRC_FD/SRC_NAME to DST_FD/DST_NAME,
honoring the MAKE_HOLES setting and using the BUF_SIZE-byte buffer
- BUF for temporary storage. Return true upon successful completion;
+ BUF for temporary storage. Copy no more than MAX_N_READ bytes.
+ Return true upon successful completion;
print a diagnostic and return false upon error.
Note that for best results, BUF should be "well"-aligned.
BUF must have sizeof(uintptr_t)-1 bytes of additional space
- beyond BUF[BUF_SIZE-1]. */
+ beyond BUF[BUF_SIZE-1].
+ Set *LAST_WRITE_MADE_HOLE to true if the final operation on
+ DEST_FD introduced a hole. */
static bool
sparse_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
bool make_holes,
- char const *src_name, char const *dst_name)
+ char const *src_name, char const *dst_name,
+ uintmax_t max_n_read, bool *last_write_made_hole)
{
typedef uintptr_t word;
- off_t n_read_total = 0;
- bool last_write_made_hole = false;
+ *last_write_made_hole = false;
- while (true)
+ while (max_n_read)
{
word *wp = NULL;
- ssize_t n_read = read (src_fd, buf, buf_size);
+ ssize_t n_read = read (src_fd, buf, MIN (max_n_read, buf_size));
if (n_read < 0)
{
#ifdef EINTR
@@ -166,8 +169,7 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
if (n_read == 0)
break;
-
- n_read_total += n_read;
+ max_n_read -= n_read;
if (make_holes)
{
@@ -209,7 +211,7 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
error (0, errno, _("cannot lseek %s"), quote (dst_name));
return false;
}
- last_write_made_hole = true;
+ *last_write_made_hole = true;
}
}
@@ -221,7 +223,7 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
error (0, errno, _("writing %s"), quote (dst_name));
return false;
}
- last_write_made_hole = false;
+ *last_write_made_hole = false;
/* It is tempting to return early here upon a short read from a
regular file. That would save the final read syscall for each
@@ -230,9 +232,16 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
}
- /* If the file ends with a `hole', we need to do something to record the
- length of the file. On modern systems, calling ftruncate does the job.
*/
- if (last_write_made_hole && ftruncate (dest_fd, n_read_total) < 0)
+ return true;
+}
+
+/* If the file ends with a `hole' (i.e., if sparse_copy set wrote_hole_at_eof),
+ call this function to record the length of the output file. */
+static bool
+sparse_copy_finalize (int dest_fd, char const *dst_name)
+{
+ off_t len = lseek (dest_fd, 0, SEEK_CUR);
+ if (0 <= len && ftruncate (dest_fd, len) < 0)
{
error (0, errno, _("truncating %s"), quote (dst_name));
return false;
@@ -309,10 +318,10 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
struct extent_scan scan;
off_t last_ext_start = 0;
uint64_t last_ext_len = 0;
- uint64_t last_read_size = 0;
extent_scan_init (src_fd, &scan);
+ bool wrote_hole_at_eof = true;
do
{
bool ok = extent_scan_read (&scan);
@@ -356,8 +365,9 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
else
{
- /* We're not inducing holes; write zeros to the destination file
- if there is a hole between the last and current extent. */
+ /* When not inducing holes and when there is a hole between
+ the end of the previous extent and the beginning of the
+ current one, write zeros to the destination file. */
if (last_ext_start + last_ext_len < ext_start)
{
uint64_t hole_size = (ext_start
@@ -373,39 +383,11 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
last_ext_start = ext_start;
last_ext_len = ext_len;
- last_read_size = 0;
-
- while (ext_len)
- {
- /* Don't read from a following hole if EXT_LEN
- is smaller than the buffer size. */
- size_t b_size = MIN (ext_len, buf_size);
- ssize_t n_read = read (src_fd, buf, b_size);
- if (n_read < 0)
- {
-#ifdef EINTR
- if (errno == EINTR)
- continue;
-#endif
- error (0, errno, _("reading %s"), quote (src_name));
- goto fail;
- }
-
- if (n_read == 0)
- {
- /* Record number of bytes read from this extent-at-EOF. */
- last_read_size = last_ext_len - ext_len;
- break;
- }
-
- if (full_write (dest_fd, buf, n_read) != n_read)
- {
- error (0, errno, _("writing %s"), quote (dst_name));
- goto fail;
- }
- ext_len -= n_read;
- }
+ if ( ! sparse_copy (src_fd, dest_fd, buf, buf_size,
+ make_holes, src_name, dst_name, ext_len,
+ &wrote_hole_at_eof))
+ return false;
}
/* Release the space allocated to scan->ext_info. */
@@ -414,16 +396,19 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
}
while (! scan.hit_final_extent);
- /* When the source file ends with a hole, the sum of the last extent start
- offset and (the read-returned size or the last extent length) is smaller
- than the actual size of the file. In that case, extend the destination
- file to the required length. When MAKE_HOLES is set, use ftruncate;
- otherwise, use write_zeros. */
- uint64_t eof_hole_len = (src_total_size - last_ext_start
- - (last_read_size ? last_read_size : last_ext_len));
- if (eof_hole_len && (make_holes
- ? ftruncate (dest_fd, src_total_size)
- : ! write_zeros (dest_fd, eof_hole_len)))
+ /* When the source file ends with a hole, we have to do a little more work,
+ since the above copied only up to and including the final extent.
+ In order to complete the copy, we may have to insert a hole or write
+ zeros in the destination corresponding to the source file's hole-at-EOF.
+
+ In addition, if the final extent was a block of zeros at EOF and we've
+ just converted them to a hole in the destination, we must call ftruncate
+ here in order to record the proper length in the destination. */
+ off_t dest_len = lseek (dest_fd, 0, SEEK_CUR);
+ if ((dest_len < src_total_size || wrote_hole_at_eof)
+ && (make_holes
+ ? ftruncate (dest_fd, src_total_size)
+ : ! write_zeros (dest_fd, src_total_size - dest_len)))
{
error (0, errno, _("failed to extend %s"), quote (dst_name));
return false;
@@ -1002,8 +987,12 @@ copy_reg (char const *src_name, char const *dst_name,
goto close_src_and_dst_desc;
}
+ bool wrote_hole_at_eof;
if ( ! sparse_copy (source_desc, dest_desc, buf, buf_size,
- make_holes, src_name, dst_name))
+ make_holes, src_name, dst_name, UINTMAX_MAX,
+ &wrote_hole_at_eof)
+ || (wrote_hole_at_eof &&
+ ! sparse_copy_finalize (dest_desc, dst_name)))
{
return_val = false;
goto close_src_and_dst_desc;
--
1.7.3.5.44.g960a
>From 7f154dcfc5641c9616921d4c5ac5005bcb2507eb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 27 Jan 2011 15:17:42 +0100
Subject: [PATCH 9/9] tests: cp/fiemap: exercise previously-failing parts
* tests/cp/fiemap-2: New test.
* tests/Makefile.am (TESTS): Add it.
---
tests/Makefile.am | 1 +
tests/cp/fiemap-2 | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 0 deletions(-)
create mode 100755 tests/cp/fiemap-2
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7855ac5..40d35ac 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -321,6 +321,7 @@ TESTS = \
cp/existing-perm-race \
cp/fail-perm \
cp/fiemap-perf \
+ cp/fiemap-2 \
cp/file-perm-race \
cp/into-self \
cp/link \
diff --git a/tests/cp/fiemap-2 b/tests/cp/fiemap-2
new file mode 100755
index 0000000..d40505b
--- /dev/null
+++ b/tests/cp/fiemap-2
@@ -0,0 +1,54 @@
+#!/bin/sh
+# Exercise a few more corners of the fiemap-copying code.
+
+# 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"
+
+# Exercise the code that handles a file ending in a hole.
+printf x > k || framework_failure_
+dd bs=1k seek=128 of=k < /dev/null || framework_failure_
+
+# The first time through the outer loop, the input file, K, ends with a hole.
+# The second time through, we append a byte so that it does not.
+for append in no yes; do
+ test $append = yes && printf y >> k
+ for i in always never; do
+ cp --sparse=$i k k2 || fail=1
+ cmp k k2 || fail=1
+ done
+done
+
+# 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_
+
+# 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
+test $(stat -c %b k2) -lt $(stat -c %b k) || fail=1
+
+Exit $fail
--
1.7.3.5.44.g960a
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jim Meyering, 2011/01/22
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, jeff.liu, 2011/01/25
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jim Meyering, 2011/01/25
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, jeff.liu, 2011/01/25
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jim Meyering, 2011/01/25
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, jeff.liu, 2011/01/25
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jeff liu, 2011/01/26
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jim Meyering, 2011/01/28
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy,
Jim Meyering <=
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Pádraig Brady, 2011/01/29
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jim Meyering, 2011/01/29
- bug#6131: [PATCH]: fiemap support for efficient sparse file copy, Jim Meyering, 2011/01/30