From unknown Fri Sep 05 09:10:54 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#5915 <5915@debbugs.gnu.org> To: bug#5915 <5915@debbugs.gnu.org> Subject: Status: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 Reply-To: bug#5915 <5915@debbugs.gnu.org> Date: Fri, 05 Sep 2025 16:10:54 +0000 retitle 5915 [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) = v2 reassign 5915 coreutils submitter 5915 Jim Meyering severity 5915 normal thanks From debbugs-submit-bounces@debbugs.gnu.org Fri Apr 09 11:49:45 2010 Received: (at submit) by debbugs.gnu.org; 9 Apr 2010 15:49:45 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O0GSm-00083s-A5 for submit@debbugs.gnu.org; Fri, 09 Apr 2010 11:49:44 -0400 Received: from mail.gnu.org ([199.232.76.166] helo=mx10.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O0GSj-00083m-Rf for submit@debbugs.gnu.org; Fri, 09 Apr 2010 11:49:42 -0400 Received: from lists.gnu.org ([199.232.76.165]:50422) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1O0GSe-0003Is-SV for submit@debbugs.gnu.org; Fri, 09 Apr 2010 11:49:36 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O0GSe-00007n-85 for bug-coreutils@gnu.org; Fri, 09 Apr 2010 11:49:36 -0400 Received: from [140.186.70.92] (port=36297 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O0GSc-00006d-HN for bug-coreutils@gnu.org; Fri, 09 Apr 2010 11:49:35 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.0 (2010-01-18) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=unavailable version=3.3.0 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O0GSb-0003kO-59 for bug-coreutils@gnu.org; Fri, 09 Apr 2010 11:49:34 -0400 Received: from smtp3-g21.free.fr ([212.27.42.3]:49600) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O0GSa-0003k6-Hk for bug-coreutils@gnu.org; Fri, 09 Apr 2010 11:49:33 -0400 Received: from smtp3-g21.free.fr (localhost [127.0.0.1]) by smtp3-g21.free.fr (Postfix) with ESMTP id AF18B81817D; Fri, 9 Apr 2010 17:49:25 +0200 (CEST) Received: from mx.meyering.net (mx.meyering.net [82.230.74.64]) by smtp3-g21.free.fr (Postfix) with ESMTP id C9E908180CC; Fri, 9 Apr 2010 17:49:22 +0200 (CEST) Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id AB83C9BB; Fri, 9 Apr 2010 17:49:22 +0200 (CEST) From: Jim Meyering To: "jeff.liu" Subject: Re: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 In-Reply-To: <4BBF490E.6020005@oracle.com> (jeff liu's message of "Fri, 09 Apr 2010 23:34:38 +0800") References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> Date: Fri, 09 Apr 2010 17:49:22 +0200 Message-ID: <87iq8039tp.fsf@meyering.net> Lines: 16 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Spam-Score: -4.8 (----) X-Debbugs-Envelope-To: submit Cc: Sunil Mushran , bug-coreutils@gnu.org, Joel Becker , Tao Ma X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -4.9 (----) jeff.liu wrote: > [...explanation...] Thanks. ... >> Run "make syntax-check", or even "make distcheck" ... > Sorry to make this stupidly mistake again, I will do double check for furture patches submition. It's not a big deal. Notice that you can probably tell your editor not to do that anymore. I've just adjusted the Cc list to include bug-coreutils. Let's keep the discussion there, for the sake of the bug tracker. From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 17 04:27:19 2010 Received: (at 5915) by debbugs.gnu.org; 17 Apr 2010 08:27:20 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O33N1-0007uv-0A for submit@debbugs.gnu.org; Sat, 17 Apr 2010 04:27:19 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O33My-0007uo-ER for 5915@debbugs.gnu.org; Sat, 17 Apr 2010 04:27:17 -0400 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3H8RA01006438 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 17 Apr 2010 08:27:12 GMT Received: from acsmt354.oracle.com (acsmt354.oracle.com [141.146.40.154]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3H8R9d3020175; Sat, 17 Apr 2010 08:27:09 GMT Received: from abhmt014.oracle.com by acsmt353.oracle.com with ESMTP id 167980571271492801; Sat, 17 Apr 2010 01:26:41 -0700 Received: from [221.223.105.53] (/221.223.105.53) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sat, 17 Apr 2010 01:26:40 -0700 Message-ID: <4BC970DC.5020004@oracle.com> Date: Sat, 17 Apr 2010 16:27:08 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> In-Reply-To: <87iq8039tp.fsf@meyering.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: acsinet15.oracle.com [141.146.126.227] X-CT-RefId: str=0001.0A090208.4BC970E1.0009:SCFMA922111,ss=1,pt=113351,fgs=0 X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?ISO-8859-1?Q?P=E1draig_Brady?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.5 (---) Hello All, This is the revised version. Both '--fiemap' and '--fiemap-sync' options were eliminated in this version. Now cp(1) will perform fiemap optimization whenever possible if cp(1) invoked with --sparse=[WHEN], otherwise, it will fall back to the standard copy. I have run 'make syntax-check' to ensure the code style is ok. The tests were run against 'btrfs', 'ocfs2' and 'ext4', each mounted as physical partitions. /dev/sda9 on /btrfs type btrfs (rw) /dev/sda8 on /ocfs2 type ocfs2 (rw,_netdev,nointr,user_xattr,acl,heartbeat=local) /dev/sda11 on /ext4 type ext4 (rw,acl,user_xattr) jeff@jeff-laptop:~/opensource_dev/coreutils$ uname -a Linux jeff-laptop 2.6.33-rc5-00238-gb04da8b-dirty #11 SMP Sat Dec 19 22:02:01 CST 2009 i686 GNU/Linux For the following performance compares, './src/cp' means the fiemap enabled `cp', and '/bin/cp' is the normal one which build against the git upstream source code. The performance are good for 'ocfs2' and 'ext4'. but for 'btrfs', it does not shown not as expected. Tests: ====== jeff@jeff-laptop:~/opensource_dev/coreutils$ dd if=/dev/zero of=/ocfs2/sparse_2 bs=1k count=1 seek=10022 1+0 records in 1+0 records out 1024 bytes (1.0 kB) copied, 0.000382239 s, 2.7 MB/s jeff@jeff-laptop:~/opensource_dev/coreutils$ time ./src/cp --sparse=always /btrfs/sparse_2 /btrfs/sparse_2_fiemap real 0m0.252s user 0m0.004s sys 0m0.232s jeff@jeff-laptop:~/opensource_dev/coreutils$ time /bin/cp --sparse=always /btrfs/sparse_2 /btrfs/sparse_2_normal real 0m0.069s user 0m0.004s sys 0m0.036s jeff@jeff-laptop:~/opensource_dev/coreutils$ time ./src/cp --sparse=always /ocfs2/sparse_2 /ocfs2/sparse_2_fiemap real 0m0.019s user 0m0.000s sys 0m0.012s jeff@jeff-laptop:~/opensource_dev/coreutils$ time /bin/cp --sparse=always /ocfs2/sparse_2 /ocfs2/sparse_2_normal real 0m0.103s user 0m0.000s sys 0m0.092s jeff@jeff-laptop:~/opensource_dev/coreutils$ time /bin/cp --sparse=always /ext4/sparse_2 /ext4/sparse_2_normal real 0m0.103s user 0m0.000s sys 0m0.064s jeff@jeff-laptop:~/opensource_dev/coreutils$ time ./src/cp --sparse=always /ext4/sparse_2 /ext4/sparse_2_fiemap real 0m0.012s user 0m0.000s sys 0m0.008s I also tried to test a sparse file which fills with holes in the middle. jeff@jeff-laptop:~/opensource_dev/coreutils$ ls -l /btrfs/sparse_4 -rw-r-xr-- 1 jeff jeff 838860800 Apr 17 14:24 /btrfs/sparse_4 jeff@jeff-laptop:~/opensource_dev/coreutils$ time ./src/cp --sparse=always /btrfs/sparse_4 /btrfs/sparse_4_fiemap real 0m43.227s user 0m0.156s sys 0m21.749s jeff@jeff-laptop:~/opensource_dev/coreutils$ time /bin/cp --sparse=always /btrfs/sparse_4 /btrfs/sparse_4_normal real 0m18.282s user 0m0.232s sys 0m14.301s jeff@jeff-laptop:~/opensource_dev/coreutils$ time sudo /bin/cp --sparse=always /ocfs2/sparse_4 /ocfs2/sparse_4_normal real 0m7.583s user 0m0.216s sys 0m7.292s jeff@jeff-laptop:~/opensource_dev/coreutils$ time sudo src/cp --sparse=always /ocfs2/sparse_4 /ocfs2/sparse_4_fiemap real 0m0.133s user 0m0.000s sys 0m0.076s jeff@jeff-laptop:~/opensource_dev/du_enhance_project/coreutils$ ls -l /ocfs2/sparse_4* -rw-r-xr-- 1 root root 838860800 Apr 16 22:52 /ocfs2/sparse_4 -rw-r-xr-- 1 jeff jeff 838860800 Apr 17 14:26 /ocfs2/sparse_4_fiemap -rw-r-xr-- 1 jeff jeff 838860800 Apr 17 14:27 /ocfs2/sparse_4_normal jeff@jeff-laptop:~/opensource_dev/coreutils$ rdiff signature /ocfs2/sparse_4 /ocfs2/sparse_4_fiemap jeff@jeff-laptop:~/opensource_dev/coreutils$ echo $? 0 jeff@jeff-laptop:~/opensource_dev/coreutils$ time /bin/cp --sparse=always /ext4/sparse_4 /ext4/sparse_4_normal real 0m7.258s user 0m0.244s sys 0m4.468s jeff@jeff-laptop:~/opensource_dev/coreutils$ time ./src/cp --sparse=always /ext4/sparse_4 /ext4/sparse_4_fiemap real 0m0.173s user 0m0.000s sys 0m0.052s >From 95f26596e932b87b2da1695086af156f24ceadfa Mon Sep 17 00:00:00 2001 From: Jie Liu Date: Sat, 17 Apr 2010 16:19:49 +0800 Subject: [PATCH 1/1] Introduct fiemap ioctl(2) for efficient sparse file copy v3 Signed-off-by: Jie Liu --- src/copy.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+), 0 deletions(-) create mode 100644 src/fiemap.h diff --git a/src/copy.c b/src/copy.c index 3c32fa3..b754d9e 100644 --- a/src/copy.c +++ b/src/copy.c @@ -65,6 +65,10 @@ # include #endif +#ifndef HAVE_FIEMAP +# include "fiemap.h" +#endif + #ifndef HAVE_FCHOWN # define HAVE_FCHOWN false # define fchown(fd, uid, gid) (-1) @@ -151,6 +155,130 @@ clone_file (int dest_fd, int src_fd) #endif } +#ifdef __linux__ +# ifndef FS_IOC_FIEMAP +# define FS_IOC_FIEMAP _IOWR ('f', 11, struct fiemap) +# endif +/* Perform FIEMAP(available in mainline 2.6.27) copy if possible. + Call ioctl(2) with FS_IOC_FIEMAP to efficiently map file allocation + excepts holes. So the overhead to deal with holes with lseek(2) in + normal copy could be saved. This would result in much faster backups + for any kind of sparse file. */ +static bool +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, + off_t src_total_size, char const *src_name, + char const *dst_name) +{ + int last = 0; + unsigned int i; + bool return_val = true; + char fiemap_buf[4096] = ""; + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / + sizeof (struct fiemap_extent); + uint64_t last_ext_logical = 0; + uint64_t last_ext_len = 0; + uint64_t last_read_size = 0; + + memset (fiemap, 0, sizeof (*fiemap)); + + do + { + fiemap->fm_start = 0ULL; + fiemap->fm_length = FIEMAP_MAX_OFFSET; + fiemap->fm_extent_count = count; + + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) + return false; + + /* If 0 extents are returned, then more ioctls are not needed. */ + if (fiemap->fm_mapped_extents == 0) + return true; + + for (i = 0; i < fiemap->fm_mapped_extents; i++) + { + uint64_t ext_logical = fm_ext[i].fe_logical; + uint64_t ext_len = fm_ext[i].fe_length; + + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL) + { + error (0, errno, _("cannot lseek %s"), quote (src_name)); + return_val = false; + } + + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL) + { + error (0, errno, _("cannot lseek %s"), quote (dst_name)); + return_val = false; + } + + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) + { + last_ext_logical = ext_logical; + last_ext_len = ext_len; + last = 1; + } + + char buf[buf_size]; + while (0 < ext_len) + { + memset (buf, 0, sizeof (buf)); + + /* Avoid reading into the holes if the left extent + length is shorter than the buffer size. */ + if (ext_len < buf_size) + buf_size = ext_len; + + 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_val = false; + } + + if (n_read == 0) + { + /* Figure out how many bytes read from the last extent. */ + 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)); + return_val = false; + } + + ext_len -= n_read; + } + + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); + } + } while (last == 0); + + /* If a file ends up with holes, the sum of the last extent logical offset + and the read returned size should shorter than the actual size of the file. + We should sets the file size to ((struct stat) st_buf.st_size). */ + if (last_ext_logical + last_read_size < src_total_size) + { + if (ftruncate (dest_fd, src_total_size) < 0) + { + error (0, errno, _("truncating %s"), quote (dst_name)); + return_val = false; + } + } + + return return_val; +} +#else +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } +#endif + /* FIXME: describe */ /* FIXME: rewrite this to use a hash table so we avoid the quadratic performance hit that's probably noticeable only on trees deeper @@ -679,6 +807,16 @@ copy_reg (char const *src_name, char const *dst_name, #endif } + if (make_holes) + { + /* Perform efficient FIEMAP copy for sparse files, fall back to the + standard copy if fails. */ + if (fiemap_copy_ok (source_desc, dest_desc, + buf_size, src_open_sb.st_size, + src_name, dst_name)) + goto preserve_metadata; + } + /* If not making a sparse file, try to use a more-efficient buffer size. */ if (! make_holes) @@ -807,6 +945,8 @@ copy_reg (char const *src_name, char const *dst_name, } } +preserve_metadata: + if (x->preserve_timestamps) { struct timespec timespec[2]; @@ -897,6 +1037,7 @@ close_src_desc: free (buf_alloc); free (name_alloc); + return return_val; } diff --git a/src/fiemap.h b/src/fiemap.h new file mode 100644 index 0000000..d33293b --- /dev/null +++ b/src/fiemap.h @@ -0,0 +1,102 @@ +/* FS_IOC_FIEMAP ioctl infrastructure. + Some portions copyright (C) 2007 Cluster File Systems, Inc + Authors: Mark Fasheh + Kalpak Shah + Andreas Dilger . */ + +/* Copy from kernel, modified to respect GNU code style by Jie Liu. */ + +#ifndef _LINUX_FIEMAP_H +# define _LINUX_FIEMAP_H + +# include + +struct fiemap_extent +{ + /* Logical offset in bytes for the start of the extent + from the beginning of the file. */ + uint64_t fe_logical; + + /* Physical offset in bytes for the start of the extent + from the beginning of the disk. */ + uint64_t fe_physical; + + /* Length in bytes for this extent. */ + uint64_t fe_length; + + uint64_t fe_reserved64[2]; + + /* FIEMAP_EXTENT_* flags for this extent. */ + uint32_t fe_flags; + + uint32_t fe_reserved[3]; +}; + +struct fiemap +{ + /* Logical offset(inclusive) at which to start mapping(in). */ + uint64_t fm_start; + + /* Logical length of mapping which userspace wants(in). */ + uint64_t fm_length; + + /* FIEMAP_FLAG_* flags for request(in/out). */ + uint32_t fm_flags; + + /* Number of extents that were mapped(out). */ + uint32_t fm_mapped_extents; + + /* Size of fm_extents array(in). */ + uint32_t fm_extent_count; + + uint32_t fm_reserved; + + /* Array of mapped extents(out). */ + struct fiemap_extent fm_extents[0]; +}; + +/* The maximum offset can be mapped for a file. */ +# define FIEMAP_MAX_OFFSET (~0ULL) + +/* Sync file data before map. */ +# define FIEMAP_FLAG_SYNC 0x00000001 + +/* Map extented attribute tree. */ +# define FIEMAP_FLAG_XATTR 0x00000002 + +# define FIEMAP_FLAGS_COMPAT (FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR) + +/* Last extent in file. */ +# define FIEMAP_EXTENT_LAST 0x00000001 + +/* Data location unknown. */ +# define FIEMAP_EXTENT_UNKNOWN 0x00000002 + +/* Location still pending, Sets EXTENT_UNKNOWN. */ +# define FIEMAP_EXTENT_DELALLOC 0x00000004 + +/* Data can not be read while fs is unmounted. */ +# define FIEMAP_EXTENT_ENCODED 0x00000008 + +/* Data is encrypted by fs. Sets EXTENT_NO_BYPASS. */ +# define FIEMAP_EXTENT_DATA_ENCRYPTED 0x00000080 + +/* Extent offsets may not be block aligned. */ +# define FIEMAP_EXTENT_NOT_ALIGNED 0x00000100 + +/* Data mixed with metadata. Sets EXTENT_NOT_ALIGNED. */ +# define FIEMAP_EXTENT_DATA_INLINE 0x00000200 + +/* Multiple files in block. Set EXTENT_NOT_ALIGNED. */ +# define FIEMAP_EXTENT_DATA_TAIL 0x00000400 + +/* Space allocated, but not data (i.e. zero). */ +# define FIEMAP_EXTENT_UNWRITTEN 0x00000800 + +/* File does not natively support extents. Result merged for efficiency. */ +# define FIEMAP_EXTENT_MERGED 0x00001000 + +/* Space shared with other files. */ +# define FIEMAP_EXTENT_SHARED 0x00002000 + +#endif -- 1.5.4.3 Jim Meyering wrote: > jeff.liu wrote: >> [...explanation...] > > Thanks. > > ... >>> Run "make syntax-check", or even "make distcheck" > ... >> Sorry to make this stupidly mistake again, I will do double check for furture patches submition. > > It's not a big deal. > Notice that you can probably tell your editor > not to do that anymore. > > I've just adjusted the Cc list to include bug-coreutils. > Let's keep the discussion there, for the sake of the bug tracker. > > > > > From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 17 05:38:49 2010 Received: (at 5915) by debbugs.gnu.org; 17 Apr 2010 09:38:50 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O34UD-0008Ok-AR for submit@debbugs.gnu.org; Sat, 17 Apr 2010 05:38:49 -0400 Received: from smtp3-g21.free.fr ([212.27.42.3]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O34UA-0008Od-JJ for 5915@debbugs.gnu.org; Sat, 17 Apr 2010 05:38:48 -0400 Received: from smtp3-g21.free.fr (localhost [127.0.0.1]) by smtp3-g21.free.fr (Postfix) with ESMTP id A2E318180B8; Sat, 17 Apr 2010 11:38:37 +0200 (CEST) Received: from mx.meyering.net (mx.meyering.net [82.230.74.64]) by smtp3-g21.free.fr (Postfix) with ESMTP id B2BF9818162; Sat, 17 Apr 2010 11:38:34 +0200 (CEST) Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id 299E2AC2; Sat, 17 Apr 2010 11:38:34 +0200 (CEST) From: Jim Meyering To: "jeff.liu" Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 In-Reply-To: <4BC970DC.5020004@oracle.com> (jeff liu's message of "Sat, 17 Apr 2010 16:27:08 +0800") References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> Date: Sat, 17 Apr 2010 11:38:34 +0200 Message-ID: <87sk6ul8px.fsf@meyering.net> Lines: 241 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Spam-Score: -1.8 (-) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?utf-8?Q?P=C3=A1draig?= Brady , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.1 (---) jeff.liu wrote: > Hello All, > > This is the revised version. Thanks for persevering and for the examples. Would you please maintain a script that automates some of the commands you've been running, so that we can ensure they continue to work as advertised? I keep hoping you'll contribute a test script or two, too, so I don't have write those, but if you can at least provide the basis for the tests, I'll massage that into the required form. I.e., instead of running the many commands as you outlined in your mail, you would run a single script that encapsulates those, where you would be free to assume (specific-to-your-setup) hard-coded absolute directory names for each of the different file system types. Also, hard-coded paths for $cp_orig and $cp_new would be fine, since you'd use those to demonstrate the performance improvements. > Both '--fiemap' and '--fiemap-sync' options were eliminated in this version. > Now cp(1) will perform fiemap optimization whenever possible if cp(1) invoked with --sparse=[WHEN], > otherwise, it will fall back to the standard copy. ... > jeff@jeff-laptop:~/opensource_dev/coreutils$ time sudo /bin/cp --sparse=always /ocfs2/sparse_4 Hmm... you're using sudo here. Does that mean the fiemap ioctl works only as root? Otherwise, why bother with sudo? > Subject: [PATCH 1/1] Introduct fiemap ioctl(2) for efficient sparse file copy v3 > > Signed-off-by: Jie Liu > --- > src/copy.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 243 insertions(+), 0 deletions(-) > create mode 100644 src/fiemap.h > > diff --git a/src/copy.c b/src/copy.c ... > +static bool > +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, > + off_t src_total_size, char const *src_name, > + char const *dst_name) > +{ > + int last = 0; Use "bool" for booleans: bool last = false; > + unsigned int i; > + bool return_val = true; Please don't use a name like "return_val". In this case, using "ok" is more descriptive. (or reverse the meaning and use "fail") > + char fiemap_buf[4096] = ""; This is a dead store. Remove the ' = ""'; > + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; > + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; > + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / > + sizeof (struct fiemap_extent); > + uint64_t last_ext_logical = 0; > + uint64_t last_ext_len = 0; > + uint64_t last_read_size = 0; > + > + memset (fiemap, 0, sizeof (*fiemap)); What is the point of initializing this buffer? I suspect you can safely remove it. > + do > + { > + fiemap->fm_start = 0ULL; > + fiemap->fm_length = FIEMAP_MAX_OFFSET; > + fiemap->fm_extent_count = count; > + > + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) > + return false; > + > + /* If 0 extents are returned, then more ioctls are not needed. */ > + if (fiemap->fm_mapped_extents == 0) > + return true; > + > + for (i = 0; i < fiemap->fm_mapped_extents; i++) > + { > + uint64_t ext_logical = fm_ext[i].fe_logical; There is no point in declaring this as uint64_t when the sole uses are cast to "off_t". Instead declare it as off_t, and add an assertion to ensure that the RHS value was in range. off_t ext_logical = fm_ext[i].fe_logical; assert (fm_ext[i].fe_logical <= OFF_T_MAX); > + uint64_t ext_len = fm_ext[i].fe_length; > + > + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL) > + { > + error (0, errno, _("cannot lseek %s"), quote (src_name)); > + return_val = false; > + } > + > + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL) > + { > + error (0, errno, _("cannot lseek %s"), quote (dst_name)); > + return_val = false; > + } If either lseek fails, then we must not proceed to read or write. Setting "return_val" is insufficient. This makes me think we'll have to redesign a little, to avoid the possibility of corruption. Consider this change: If the fiemap ioctl succeeds: use only this specialized code to perform the copy and do not fall back on normal copy. However, if any operation fails here (like these lseeks, read or write), we diagnose it as usual. Not falling back upon failure has the added benefit that there's no risk of a duplicate diagnostic if the same failure occurs again during the regular copy. Otherwise, imagine that lseek advances the source FD, but the destination lseek fails. Now the FDs are inconsistent. We would have to realign (or restore) them, or risk writing data at the wrong offset: corruption. > + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) > + { > + last_ext_logical = ext_logical; > + last_ext_len = ext_len; > + last = 1; last = true; > + } > + > + char buf[buf_size]; "buf" is used only within the loop, so please declare it there. > + while (0 < ext_len) > + { > + memset (buf, 0, sizeof (buf)); I see no need to initialize "buf". The only data you use from it is put there by "read". > + /* Avoid reading into the holes if the left extent > + length is shorter than the buffer size. */ > + if (ext_len < buf_size) > + buf_size = ext_len; > + > + 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_val = false; > + } > + > + if (n_read == 0) > + { > + /* Figure out how many bytes read from the last extent. */ > + last_read_size = last_ext_len - ext_len; > + break; Each of the above three lines is indented one space too far. > + } > + > + if (full_write (dest_fd, buf, n_read) != n_read) > + { > + error (0, errno, _("writing %s"), quote (dst_name)); > + return_val = false; > + } > + > + ext_len -= n_read; > + } > + > + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); > + } > + } while (last == 0); } while (!last); > + > + /* If a file ends up with holes, the sum of the last extent logical offset > + and the read returned size should shorter than the actual size of the file. > + We should sets the file size to ((struct stat) st_buf.st_size). */ /* If a file ends with a hole, the sum of the last extent logical offset and the read-returned size will be shorter than the actual size of the file. Use ftruncate to extend the length of the destination file. */ > + if (last_ext_logical + last_read_size < src_total_size) > + { > + if (ftruncate (dest_fd, src_total_size) < 0) > + { > + error (0, errno, _("truncating %s"), quote (dst_name)); s/truncating/extending/ > + return_val = false; > + } > + } > + > + return return_val; > +} > +#else > +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } > +#endif > + > /* FIXME: describe */ > /* FIXME: rewrite this to use a hash table so we avoid the quadratic > performance hit that's probably noticeable only on trees deeper > @@ -679,6 +807,16 @@ copy_reg (char const *src_name, char const *dst_name, > #endif > } > > + if (make_holes) > + { > + /* Perform efficient FIEMAP copy for sparse files, fall back to the > + standard copy if fails. */ > + if (fiemap_copy_ok (source_desc, dest_desc, > + buf_size, src_open_sb.st_size, > + src_name, dst_name)) > + goto preserve_metadata; > + } From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 17 08:38:20 2010 Received: (at 5915) by debbugs.gnu.org; 17 Apr 2010 12:38:21 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O37Hv-0001fh-Nd for submit@debbugs.gnu.org; Sat, 17 Apr 2010 08:38:20 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O37Hu-0001fc-5d for 5915@debbugs.gnu.org; Sat, 17 Apr 2010 08:38:19 -0400 Received: from rcsinet13.oracle.com (rcsinet13.oracle.com [148.87.113.125]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3HCcBLd002037 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 17 Apr 2010 12:38:12 GMT Received: from acsmt355.oracle.com (acsmt355.oracle.com [141.146.40.155]) by rcsinet13.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3HBB6T8019292; Sat, 17 Apr 2010 12:38:10 GMT Received: from abhmt003.oracle.com by acsmt354.oracle.com with ESMTP id 184382021271507864; Sat, 17 Apr 2010 05:37:44 -0700 Received: from [221.223.105.53] (/221.223.105.53) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sat, 17 Apr 2010 05:37:43 -0700 Message-ID: <4BC9ABB4.6060500@oracle.com> Date: Sat, 17 Apr 2010 20:38:12 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> In-Reply-To: <87sk6ul8px.fsf@meyering.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: rcsinet13.oracle.com [148.87.113.125] X-CT-RefId: str=0001.0A090205.4BC9ABB4.012D:SCFMA4539811,ss=1,pt=113351,fgs=0 X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.1 (---) Hi Jim, Thank you so much for your so quick response! Jim Meyering wrote: > jeff.liu wrote: >> Hello All, >> >> This is the revised version. > > Thanks for persevering and for the examples. > > Would you please maintain a script that automates some of the > commands you've been running, so that we can ensure they > continue to work as advertised? I keep hoping you'll contribute > a test script or two, too, so I don't have write those, but if > you can at least provide the basis for the tests, I'll massage > that into the required form. I.e., instead of running the many > commands as you outlined in your mail, you would run a single > script that encapsulates those, where you would be free to assume > (specific-to-your-setup) hard-coded absolute directory names for > each of the different file system types. > Also, hard-coded paths for $cp_orig and $cp_new would be fine, > since you'd use those to demonstrate the performance improvements. Yes, I kept in mind for your suggestions about how to meet the requirements for new features. :) Actually, I already working on a test script, the reason why I didn't submit it along with the current patch is I have concern if the current test coverage is enough, so I send out the patch first. The test script will be submit in my next try. > >> Both '--fiemap' and '--fiemap-sync' options were eliminated in this version. >> Now cp(1) will perform fiemap optimization whenever possible if cp(1) invoked with --sparse=[WHEN], >> otherwise, it will fall back to the standard copy. > ... >> jeff@jeff-laptop:~/opensource_dev/coreutils$ time sudo /bin/cp --sparse=always /ocfs2/sparse_4 > > Hmm... you're using sudo here. > Does that mean the fiemap ioctl works only as root? sorry! I also forgot why I using sudo at that time. :( But I am sure fiemap ioctl(2) could works without root privilage. > Otherwise, why bother with sudo? > >> Subject: [PATCH 1/1] Introduct fiemap ioctl(2) for efficient sparse file copy v3 >> >> Signed-off-by: Jie Liu >> --- >> src/copy.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 243 insertions(+), 0 deletions(-) >> create mode 100644 src/fiemap.h >> >> diff --git a/src/copy.c b/src/copy.c > ... >> +static bool >> +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, >> + off_t src_total_size, char const *src_name, >> + char const *dst_name) >> +{ >> + int last = 0; > > Use "bool" for booleans: > > bool last = false; >> + unsigned int i; >> + bool return_val = true; > > Please don't use a name like "return_val". > In this case, using "ok" is more descriptive. > (or reverse the meaning and use "fail") Thanks for the point out. > >> + char fiemap_buf[4096] = ""; > > This is a dead store. Remove the ' = ""'; > >> + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; >> + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; >> + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / >> + sizeof (struct fiemap_extent); >> + uint64_t last_ext_logical = 0; >> + uint64_t last_ext_len = 0; >> + uint64_t last_read_size = 0; >> + >> + memset (fiemap, 0, sizeof (*fiemap)); > > What is the point of initializing this buffer? > I suspect you can safely remove it. I'll double check for this point. > >> + do >> + { >> + fiemap->fm_start = 0ULL; >> + fiemap->fm_length = FIEMAP_MAX_OFFSET; >> + fiemap->fm_extent_count = count; >> + >> + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) >> + return false; >> + >> + /* If 0 extents are returned, then more ioctls are not needed. */ >> + if (fiemap->fm_mapped_extents == 0) >> + return true; >> + >> + for (i = 0; i < fiemap->fm_mapped_extents; i++) >> + { >> + uint64_t ext_logical = fm_ext[i].fe_logical; > > There is no point in declaring this as uint64_t when the > sole uses are cast to "off_t". > Instead declare it as off_t, and add an assertion > to ensure that the RHS value was in range. > > off_t ext_logical = fm_ext[i].fe_logical; > assert (fm_ext[i].fe_logical <= OFF_T_MAX); > Yes, it should be declared to 'off_t' here. >> + uint64_t ext_len = fm_ext[i].fe_length; > >> + >> + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >> + { >> + error (0, errno, _("cannot lseek %s"), quote (src_name)); >> + return_val = false; >> + } >> + >> + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >> + { >> + error (0, errno, _("cannot lseek %s"), quote (dst_name)); >> + return_val = false; >> + } > > If either lseek fails, then we must not proceed to read or write. > Setting "return_val" is insufficient. > > This makes me think we'll have to redesign a little, to avoid > the possibility of corruption. > > Consider this change: > > If the fiemap ioctl succeeds: use only this specialized code to > perform the copy and do not fall back on normal copy. However, if > any operation fails here (like these lseeks, read or write), we > diagnose it as usual. Not falling back upon failure has the added > benefit that there's no risk of a duplicate diagnostic if the same > failure occurs again during the regular copy. > > Otherwise, imagine that lseek advances the source FD, > but the destination lseek fails. Now the FDs are inconsistent. > We would have to realign (or restore) them, or risk writing > data at the wrong offset: corruption. So, do you means if fiemap ioctl(2) succeeds, no matter what error(read, write, lseek) occurred afterwards, we should always diagnosing it and then abort with the corresponding error msg? My original design really cause the file corruption if the lseek(2) already advances the source FD but the destination lseek(2) fails. But is it make sense to seek back to the start offset for both source and destination FDs, then fall back to the regular copy? > >> + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) >> + { >> + last_ext_logical = ext_logical; >> + last_ext_len = ext_len; >> + last = 1; > > last = true; > >> + } >> + >> + char buf[buf_size]; > > "buf" is used only within the loop, so please declare it there. > >> + while (0 < ext_len) >> + { >> + memset (buf, 0, sizeof (buf)); > > I see no need to initialize "buf". > The only data you use from it is put there by "read". This initizalize really do not need here. > >> + /* Avoid reading into the holes if the left extent >> + length is shorter than the buffer size. */ >> + if (ext_len < buf_size) >> + buf_size = ext_len; >> + >> + 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_val = false; >> + } >> + >> + if (n_read == 0) >> + { >> + /* Figure out how many bytes read from the last extent. */ >> + last_read_size = last_ext_len - ext_len; >> + break; > > Each of the above three lines is indented one space too far. oh, I do such stupid thing again!!!! > >> + } >> + >> + if (full_write (dest_fd, buf, n_read) != n_read) >> + { >> + error (0, errno, _("writing %s"), quote (dst_name)); >> + return_val = false; >> + } >> + >> + ext_len -= n_read; >> + } >> + >> + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); >> + } >> + } while (last == 0); > > } while (!last); > >> + >> + /* If a file ends up with holes, the sum of the last extent logical offset >> + and the read returned size should shorter than the actual size of the file. >> + We should sets the file size to ((struct stat) st_buf.st_size). */ > > /* If a file ends with a hole, the sum of the last extent logical offset > and the read-returned size will be shorter than the actual size of the > file. Use ftruncate to extend the length of the destination file. */ > >> + if (last_ext_logical + last_read_size < src_total_size) >> + { >> + if (ftruncate (dest_fd, src_total_size) < 0) >> + { >> + error (0, errno, _("truncating %s"), quote (dst_name)); > > s/truncating/extending/ thanks for above 3 corrections! > >> + return_val = false; >> + } >> + } >> + >> + return return_val; >> +} >> +#else >> +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } >> +#endif >> + >> /* FIXME: describe */ >> /* FIXME: rewrite this to use a hash table so we avoid the quadratic >> performance hit that's probably noticeable only on trees deeper >> @@ -679,6 +807,16 @@ copy_reg (char const *src_name, char const *dst_name, >> #endif >> } >> >> + if (make_holes) >> + { >> + /* Perform efficient FIEMAP copy for sparse files, fall back to the >> + standard copy if fails. */ >> + if (fiemap_copy_ok (source_desc, dest_desc, >> + buf_size, src_open_sb.st_size, >> + src_name, dst_name)) >> + goto preserve_metadata; >> + } Best Regards, -Jeff From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 17 09:01:03 2010 Received: (at 5915) by debbugs.gnu.org; 17 Apr 2010 13:01:03 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O37du-0001ok-J5 for submit@debbugs.gnu.org; Sat, 17 Apr 2010 09:01:03 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O37dr-0001oZ-Nl for 5915@debbugs.gnu.org; Sat, 17 Apr 2010 09:01:00 -0400 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3HD0p5e022969 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 17 Apr 2010 13:00:53 GMT Received: from acsmt353.oracle.com (acsmt353.oracle.com [141.146.40.153]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3HCkbvh014496; Sat, 17 Apr 2010 13:00:50 GMT Received: from abhmt004.oracle.com by acsmt355.oracle.com with ESMTP id 168144771271509214; Sat, 17 Apr 2010 06:00:14 -0700 Received: from [221.223.105.53] (/221.223.105.53) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sat, 17 Apr 2010 06:00:13 -0700 Message-ID: <4BC9B0F9.6090700@oracle.com> Date: Sat, 17 Apr 2010 21:00:41 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> In-Reply-To: <4BC9ABB4.6060500@oracle.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: acsinet15.oracle.com [141.146.126.227] X-CT-RefId: str=0001.0A090203.4BC9B105.0170:SCFMA922111,ss=1,pt=113351,fgs=0 X-Spam-Score: -3.0 (---) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.0 (---) jeff.liu wrote: > Hi Jim, > > Thank you so much for your so quick response! > > Jim Meyering wrote: >> jeff.liu wrote: >>> Hello All, >>> >>> This is the revised version. >> Thanks for persevering and for the examples. >> >> Would you please maintain a script that automates some of the >> commands you've been running, so that we can ensure they >> continue to work as advertised? I keep hoping you'll contribute >> a test script or two, too, so I don't have write those, but if >> you can at least provide the basis for the tests, I'll massage >> that into the required form. I.e., instead of running the many >> commands as you outlined in your mail, you would run a single >> script that encapsulates those, where you would be free to assume >> (specific-to-your-setup) hard-coded absolute directory names for >> each of the different file system types. >> Also, hard-coded paths for $cp_orig and $cp_new would be fine, >> since you'd use those to demonstrate the performance improvements. > Yes, I kept in mind for your suggestions about how to meet the requirements for new features. :) > > Actually, I already working on a test script, the reason why I didn't submit it along with the > current patch is I have concern if the current test coverage is enough, so I send out the patch > first. The test script will be submit in my next try. >>> Both '--fiemap' and '--fiemap-sync' options were eliminated in this version. >>> Now cp(1) will perform fiemap optimization whenever possible if cp(1) invoked with --sparse=[WHEN], >>> otherwise, it will fall back to the standard copy. >> ... >>> jeff@jeff-laptop:~/opensource_dev/coreutils$ time sudo /bin/cp --sparse=always /ocfs2/sparse_4 >> Hmm... you're using sudo here. >> Does that mean the fiemap ioctl works only as root? > sorry! I also forgot why I using sudo at that time. :( > But I am sure fiemap ioctl(2) could works without root privilage. > >> Otherwise, why bother with sudo? >> >>> Subject: [PATCH 1/1] Introduct fiemap ioctl(2) for efficient sparse file copy v3 >>> >>> Signed-off-by: Jie Liu >>> --- >>> src/copy.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 243 insertions(+), 0 deletions(-) >>> create mode 100644 src/fiemap.h >>> >>> diff --git a/src/copy.c b/src/copy.c >> ... >>> +static bool >>> +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, >>> + off_t src_total_size, char const *src_name, >>> + char const *dst_name) >>> +{ >>> + int last = 0; >> Use "bool" for booleans: >> >> bool last = false; >>> + unsigned int i; >>> + bool return_val = true; >> Please don't use a name like "return_val". >> In this case, using "ok" is more descriptive. >> (or reverse the meaning and use "fail") > Thanks for the point out. >>> + char fiemap_buf[4096] = ""; >> This is a dead store. Remove the ' = ""'; >> >>> + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; >>> + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; >>> + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / >>> + sizeof (struct fiemap_extent); >>> + uint64_t last_ext_logical = 0; >>> + uint64_t last_ext_len = 0; >>> + uint64_t last_read_size = 0; >>> + >>> + memset (fiemap, 0, sizeof (*fiemap)); >> What is the point of initializing this buffer? >> I suspect you can safely remove it. > I'll double check for this point. >>> + do >>> + { >>> + fiemap->fm_start = 0ULL; >>> + fiemap->fm_length = FIEMAP_MAX_OFFSET; >>> + fiemap->fm_extent_count = count; >>> + >>> + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) >>> + return false; >>> + >>> + /* If 0 extents are returned, then more ioctls are not needed. */ >>> + if (fiemap->fm_mapped_extents == 0) >>> + return true; >>> + >>> + for (i = 0; i < fiemap->fm_mapped_extents; i++) >>> + { >>> + uint64_t ext_logical = fm_ext[i].fe_logical; >> There is no point in declaring this as uint64_t when the >> sole uses are cast to "off_t". >> Instead declare it as off_t, and add an assertion >> to ensure that the RHS value was in range. >> >> off_t ext_logical = fm_ext[i].fe_logical; >> assert (fm_ext[i].fe_logical <= OFF_T_MAX); >> > Yes, it should be declared to 'off_t' here. > >>> + uint64_t ext_len = fm_ext[i].fe_length; >>> + >>> + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >>> + { >>> + error (0, errno, _("cannot lseek %s"), quote (src_name)); >>> + return_val = false; >>> + } >>> + >>> + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >>> + { >>> + error (0, errno, _("cannot lseek %s"), quote (dst_name)); >>> + return_val = false; >>> + } >> If either lseek fails, then we must not proceed to read or write. >> Setting "return_val" is insufficient. >> >> This makes me think we'll have to redesign a little, to avoid >> the possibility of corruption. >> >> Consider this change: >> >> If the fiemap ioctl succeeds: use only this specialized code to >> perform the copy and do not fall back on normal copy. However, if >> any operation fails here (like these lseeks, read or write), we >> diagnose it as usual. Not falling back upon failure has the added >> benefit that there's no risk of a duplicate diagnostic if the same >> failure occurs again during the regular copy. >> >> Otherwise, imagine that lseek advances the source FD, >> but the destination lseek fails. Now the FDs are inconsistent. >> We would have to realign (or restore) them, or risk writing >> data at the wrong offset: corruption. > So, do you means if fiemap ioctl(2) succeeds, no matter what error(read, write, lseek) occurred > afterwards, we should always diagnosing it and then abort with the corresponding error msg? > > My original design really cause the file corruption if the lseek(2) already advances the source FD > but the destination lseek(2) fails. > > But is it make sense to seek back to the start offset for both source and destination FDs, then fall > back to the regular copy? Oh, My thought to seek back to the start offset of both FDs is wrong, it most possibibly encounter the same issue (read, write, lseek) in regular copy again. > >>> + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) >>> + { >>> + last_ext_logical = ext_logical; >>> + last_ext_len = ext_len; >>> + last = 1; >> last = true; >> >>> + } >>> + >>> + char buf[buf_size]; >> "buf" is used only within the loop, so please declare it there. >> >>> + while (0 < ext_len) >>> + { >>> + memset (buf, 0, sizeof (buf)); >> I see no need to initialize "buf". >> The only data you use from it is put there by "read". > This initizalize really do not need here. >>> + /* Avoid reading into the holes if the left extent >>> + length is shorter than the buffer size. */ >>> + if (ext_len < buf_size) >>> + buf_size = ext_len; >>> + >>> + 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_val = false; >>> + } >>> + >>> + if (n_read == 0) >>> + { >>> + /* Figure out how many bytes read from the last extent. */ >>> + last_read_size = last_ext_len - ext_len; >>> + break; >> Each of the above three lines is indented one space too far. > oh, I do such stupid thing again!!!! >>> + } >>> + >>> + if (full_write (dest_fd, buf, n_read) != n_read) >>> + { >>> + error (0, errno, _("writing %s"), quote (dst_name)); >>> + return_val = false; >>> + } >>> + >>> + ext_len -= n_read; >>> + } >>> + >>> + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); >>> + } >>> + } while (last == 0); >> } while (!last); >> >>> + >>> + /* If a file ends up with holes, the sum of the last extent logical offset >>> + and the read returned size should shorter than the actual size of the file. >>> + We should sets the file size to ((struct stat) st_buf.st_size). */ >> /* If a file ends with a hole, the sum of the last extent logical offset >> and the read-returned size will be shorter than the actual size of the >> file. Use ftruncate to extend the length of the destination file. */ >> >>> + if (last_ext_logical + last_read_size < src_total_size) >>> + { >>> + if (ftruncate (dest_fd, src_total_size) < 0) >>> + { >>> + error (0, errno, _("truncating %s"), quote (dst_name)); >> s/truncating/extending/ > thanks for above 3 corrections! >>> + return_val = false; >>> + } >>> + } >>> + >>> + return return_val; >>> +} >>> +#else >>> +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } >>> +#endif >>> + >>> /* FIXME: describe */ >>> /* FIXME: rewrite this to use a hash table so we avoid the quadratic >>> performance hit that's probably noticeable only on trees deeper >>> @@ -679,6 +807,16 @@ copy_reg (char const *src_name, char const *dst_name, >>> #endif >>> } >>> >>> + if (make_holes) >>> + { >>> + /* Perform efficient FIEMAP copy for sparse files, fall back to the >>> + standard copy if fails. */ >>> + if (fiemap_copy_ok (source_desc, dest_desc, >>> + buf_size, src_open_sb.st_size, >>> + src_name, dst_name)) >>> + goto preserve_metadata; >>> + } > > Best Regards, > -Jeff > From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 17 10:42:59 2010 Received: (at 5915) by debbugs.gnu.org; 17 Apr 2010 14:43:00 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O39EZ-0002VE-5s for submit@debbugs.gnu.org; Sat, 17 Apr 2010 10:42:59 -0400 Received: from smtp5-g21.free.fr ([212.27.42.5]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O39EW-0002V8-4q for 5915@debbugs.gnu.org; Sat, 17 Apr 2010 10:42:57 -0400 Received: from smtp5-g21.free.fr (localhost [127.0.0.1]) by smtp5-g21.free.fr (Postfix) with ESMTP id 28AD0D4805A; Sat, 17 Apr 2010 16:42:44 +0200 (CEST) Received: from mx.meyering.net (mx.meyering.net [82.230.74.64]) by smtp5-g21.free.fr (Postfix) with ESMTP id 4572BD48096; Sat, 17 Apr 2010 16:42:42 +0200 (CEST) Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id D69FCBE2; Sat, 17 Apr 2010 16:42:41 +0200 (CEST) From: Jim Meyering To: "jeff.liu" Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 In-Reply-To: <4BC9ABB4.6060500@oracle.com> (jeff liu's message of "Sat, 17 Apr 2010 20:38:12 +0800") References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> Date: Sat, 17 Apr 2010 16:42:41 +0200 Message-ID: <87pr1yjg2m.fsf@meyering.net> Lines: 55 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Spam-Score: -3.1 (---) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?utf-8?Q?P=C3=A1draig?= Brady , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.1 (---) jeff.liu wrote: ... >>> + uint64_t ext_len = fm_ext[i].fe_length; >> >>> + >>> + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >>> + { >>> + error (0, errno, _("cannot lseek %s"), quote (src_name)); >>> + return_val = false; >>> + } >>> + >>> + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >>> + { >>> + error (0, errno, _("cannot lseek %s"), quote (dst_name)); >>> + return_val = false; >>> + } >> >> If either lseek fails, then we must not proceed to read or write. >> Setting "return_val" is insufficient. >> >> This makes me think we'll have to redesign a little, to avoid >> the possibility of corruption. >> >> Consider this change: >> >> If the fiemap ioctl succeeds: use only this specialized code to >> perform the copy and do not fall back on normal copy. However, if >> any operation fails here (like these lseeks, read or write), we >> diagnose it as usual. Not falling back upon failure has the added >> benefit that there's no risk of a duplicate diagnostic if the same >> failure occurs again during the regular copy. >> >> Otherwise, imagine that lseek advances the source FD, >> but the destination lseek fails. Now the FDs are inconsistent. >> We would have to realign (or restore) them, or risk writing >> data at the wrong offset: corruption. > So, do you means if fiemap ioctl(2) succeeds, no matter what error(read, write, lseek) occurred > afterwards, we should always diagnosing it and then abort with the corresponding error msg? > > My original design really cause the file corruption if the lseek(2) already advances the source FD > but the destination lseek(2) fails. > > But is it make sense to seek back to the start offset for both source and destination FDs, then fall > back to the regular copy? Your changes should make cp handle these cases: ioctl succeeds and fiemap-copying succeeds nothing more to do for this pair of files ioctl succeeds and any aspect of fiemap-copying fails fail: do not try to copy any other way ioctl fails attempt to copy as usual In each case, continue with any remaining files. From debbugs-submit-bounces@debbugs.gnu.org Sat Apr 17 11:10:45 2010 Received: (at 5915) by debbugs.gnu.org; 17 Apr 2010 15:10:46 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O39fR-0002gG-GX for submit@debbugs.gnu.org; Sat, 17 Apr 2010 11:10:45 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O39fP-0002g9-8S for 5915@debbugs.gnu.org; Sat, 17 Apr 2010 11:10:43 -0400 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3HFAatr005640 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 17 Apr 2010 15:10:37 GMT Received: from acsmt355.oracle.com (acsmt355.oracle.com [141.146.40.155]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3HDvbNY013238; Sat, 17 Apr 2010 15:10:35 GMT Received: from abhmt012.oracle.com by acsmt355.oracle.com with ESMTP id 168219411271516992; Sat, 17 Apr 2010 08:09:52 -0700 Received: from [221.223.105.53] (/221.223.105.53) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sat, 17 Apr 2010 08:09:52 -0700 Message-ID: <4BC9CF5E.30606@oracle.com> Date: Sat, 17 Apr 2010 23:10:22 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> <87pr1yjg2m.fsf@meyering.net> In-Reply-To: <87pr1yjg2m.fsf@meyering.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: acsinet15.oracle.com [141.146.126.227] X-CT-RefId: str=0001.0A090202.4BC9CF6D.0188:SCFMA922111,ss=1,fgs=0 X-Spam-Score: -2.9 (--) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.9 (--) Jim Meyering wrote: > jeff.liu wrote: > ... >>>> + uint64_t ext_len = fm_ext[i].fe_length; >>>> + >>>> + if (lseek (src_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >>>> + { >>>> + error (0, errno, _("cannot lseek %s"), quote (src_name)); >>>> + return_val = false; >>>> + } >>>> + >>>> + if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL) >>>> + { >>>> + error (0, errno, _("cannot lseek %s"), quote (dst_name)); >>>> + return_val = false; >>>> + } >>> If either lseek fails, then we must not proceed to read or write. >>> Setting "return_val" is insufficient. >>> >>> This makes me think we'll have to redesign a little, to avoid >>> the possibility of corruption. >>> >>> Consider this change: >>> >>> If the fiemap ioctl succeeds: use only this specialized code to >>> perform the copy and do not fall back on normal copy. However, if >>> any operation fails here (like these lseeks, read or write), we >>> diagnose it as usual. Not falling back upon failure has the added >>> benefit that there's no risk of a duplicate diagnostic if the same >>> failure occurs again during the regular copy. >>> >>> Otherwise, imagine that lseek advances the source FD, >>> but the destination lseek fails. Now the FDs are inconsistent. >>> We would have to realign (or restore) them, or risk writing >>> data at the wrong offset: corruption. >> So, do you means if fiemap ioctl(2) succeeds, no matter what error(read, write, lseek) occurred >> afterwards, we should always diagnosing it and then abort with the corresponding error msg? >> >> My original design really cause the file corruption if the lseek(2) already advances the source FD >> but the destination lseek(2) fails. >> >> But is it make sense to seek back to the start offset for both source and destination FDs, then fall >> back to the regular copy? > > Your changes should make cp handle these cases: > > ioctl succeeds and fiemap-copying succeeds > nothing more to do for this pair of files > ioctl succeeds and any aspect of fiemap-copying fails > fail: do not try to copy any other way > ioctl fails > attempt to copy as usual > > In each case, continue with any remaining files. Thanks for the detailed info! -Jeff From debbugs-submit-bounces@debbugs.gnu.org Mon Apr 19 05:47:07 2010 Received: (at 5915) by debbugs.gnu.org; 19 Apr 2010 09:47:08 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O3nZL-0005Be-9q for submit@debbugs.gnu.org; Mon, 19 Apr 2010 05:47:07 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O3nZJ-0005BI-0A for 5915@debbugs.gnu.org; Mon, 19 Apr 2010 05:47:06 -0400 Received: from rcsinet15.oracle.com (rcsinet15.oracle.com [148.87.113.117]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3J9kxXX015169 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 19 Apr 2010 09:47:00 GMT Received: from acsmt355.oracle.com (acsmt355.oracle.com [141.146.40.155]) by rcsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3J9kvAv010854; Mon, 19 Apr 2010 09:46:57 GMT Received: from abhmt007.oracle.com by acsmt353.oracle.com with ESMTP id 186850911271670327; Mon, 19 Apr 2010 02:45:27 -0700 Received: from [221.223.110.66] (/221.223.110.66) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 19 Apr 2010 02:45:24 -0700 Message-ID: <4BCC2653.2080702@oracle.com> Date: Mon, 19 Apr 2010 17:45:55 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> <87pr1yjg2m.fsf@meyering.net> In-Reply-To: <87pr1yjg2m.fsf@meyering.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: rcsinet15.oracle.com [148.87.113.117] X-CT-RefId: str=0001.0A090208.4BCC2695.0146:SCFMA4539811,ss=1,pt=113351,fgs=0 X-Spam-Score: -1.6 (-) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?ISO-8859-1?Q?P=E1draig_Brady?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.7 (--) Hi Jim, Thanks for your comments for my last patch submition! Now the fiemap copy logical changed to: 1. If fiemap copy succeeds, go ahead with the meta data preserve procedure if they are user specified. 2. If fiemap_copy fails and the 'normal_copy_required' is true, which means the ioctl(2) fails. In this case, fall back to normal copy. 'bool normal_copy_required' in fiemap_copy_ok() is a new argument added in this version used to determine whether normal copy require or not. 3. If fiemap_copy fails for other reason, go to 'close_src_and_dst_desc' directly. I also add a test script to 'tests/cp/sparse-fiemap-copy'(I can not find out a better name at the moment), it create a 41M sparse file on btrfs/ext4/ocfs2 separately, and then try copy them in those 2 different ways. It supply two result verify steps, the first is to compare the sum of 'sys and user' time by run each copy via '/usr/bin/time', the fiemap copy should run faster than normal copy. 2nd is compare the file size in bytes, the fiemap copied file size should be equal to the orignally. Would you please review the following patches at your convenient time? >From 561b7b3413de16595fc71f0cba5bfa918b42f9e5 Mon Sep 17 00:00:00 2001 From: Jie Liu Date: Sun, 18 Apr 2010 14:31:20 +0800 Subject: [PATCH 1/2] Introduct fiemap ioctl(2) for efficient sparse file copy. Signed-off-by: Jie Liu --- src/copy.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 255 insertions(+), 0 deletions(-) create mode 100644 src/fiemap.h diff --git a/src/copy.c b/src/copy.c index 3c32fa3..e7adecc 100644 --- a/src/copy.c +++ b/src/copy.c @@ -65,6 +65,10 @@ # include #endif +#ifndef HAVE_FIEMAP +# include "fiemap.h" +#endif + #ifndef HAVE_FCHOWN # define HAVE_FCHOWN false # define fchown(fd, uid, gid) (-1) @@ -151,6 +155,133 @@ clone_file (int dest_fd, int src_fd) #endif } +#ifdef __linux__ +# ifndef FS_IOC_FIEMAP +# define FS_IOC_FIEMAP _IOWR ('f', 11, struct fiemap) +# endif +/* Perform FIEMAP(available in mainline 2.6.27) copy if possible. + Call ioctl(2) with FS_IOC_FIEMAP to efficiently map file allocation + excepts holes. So the overhead to deal with holes with lseek(2) in + normal copy could be saved. This would result in much faster backups + for any kind of sparse file. */ +static bool +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, + off_t src_total_size, char const *src_name, + char const *dst_name, bool *normal_copy_required) +{ + bool fail = false; + bool last = false; + char fiemap_buf[4096]; + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / + sizeof (struct fiemap_extent); + unsigned int i; + off_t last_ext_logical = 0; + uint64_t last_ext_len = 0; + uint64_t last_read_size = 0; + + do + { + fiemap->fm_start = 0ULL; + fiemap->fm_length = FIEMAP_MAX_OFFSET; + fiemap->fm_extent_count = count; + + /* When ioctl(2) fails for any reason, fall back to the normal copy. */ + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) + { + *normal_copy_required = true; + return false; + } + + /* If 0 extents are returned, then more ioctls are not needed. */ + if (fiemap->fm_mapped_extents == 0) + return true; + + for (i = 0; i < fiemap->fm_mapped_extents; i++) + { + assert (fm_ext[i].fe_logical <= OFF_T_MAX); + + off_t ext_logical = fm_ext[i].fe_logical; + uint64_t ext_len = fm_ext[i].fe_length; + + if (lseek (src_fd, ext_logical, SEEK_SET) < 0LL) + { + error (0, errno, _("cannot lseek %s"), quote (src_name)); + return fail; + } + + if (lseek (dest_fd, ext_logical, SEEK_SET) < 0LL) + { + error (0, errno, _("cannot lseek %s"), quote (dst_name)); + return fail; + } + + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) + { + last_ext_logical = ext_logical; + last_ext_len = ext_len; + last = true; + } + + while (0 < ext_len) + { + char buf[buf_size]; + + /* Avoid reading into the holes if the left extent + length is shorter than the buffer size. */ + if (ext_len < buf_size) + buf_size = ext_len; + + 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 fail; + } + + if (n_read == 0) + { + /* Figure out how many bytes read from the last extent. */ + 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)); + return fail; + } + + ext_len -= n_read; + } + + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); + } + } while (! last); + + /* If a file ends up with holes, the sum of the last extent logical offset + and the read-returned size will be shorter than the actual size of the + file. Use ftruncate to extend the length of the destination file. */ + if (last_ext_logical + last_read_size < src_total_size) + { + if (ftruncate (dest_fd, src_total_size) < 0) + { + error (0, errno, _("extending %s"), quote (dst_name)); + return fail; + } + } + + return ! fail; +} +#else +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } +#endif + /* FIXME: describe */ /* FIXME: rewrite this to use a hash table so we avoid the quadratic performance hit that's probably noticeable only on trees deeper @@ -679,6 +810,25 @@ copy_reg (char const *src_name, char const *dst_name, #endif } + if (make_holes) + { + bool require_normal_copy = false; + /* Perform efficient FIEMAP copy for sparse files, fall back to the + standard copy only if the ioctl(2) fails. */ + if (fiemap_copy_ok (source_desc, dest_desc, buf_size, + src_open_sb.st_size, src_name, + dst_name, &require_normal_copy)) + goto preserve_metadata; + else + { + if (! require_normal_copy) + { + return_val = false; + goto close_src_and_dst_desc; + } + } + } + /* If not making a sparse file, try to use a more-efficient buffer size. */ if (! make_holes) @@ -807,6 +957,8 @@ copy_reg (char const *src_name, char const *dst_name, } } +preserve_metadata: + if (x->preserve_timestamps) { struct timespec timespec[2]; @@ -897,6 +1049,7 @@ close_src_desc: free (buf_alloc); free (name_alloc); + return return_val; } diff --git a/src/fiemap.h b/src/fiemap.h new file mode 100644 index 0000000..d33293b --- /dev/null +++ b/src/fiemap.h @@ -0,0 +1,102 @@ +/* FS_IOC_FIEMAP ioctl infrastructure. + Some portions copyright (C) 2007 Cluster File Systems, Inc + Authors: Mark Fasheh + Kalpak Shah + Andreas Dilger . */ + +/* Copy from kernel, modified to respect GNU code style by Jie Liu. */ + +#ifndef _LINUX_FIEMAP_H +# define _LINUX_FIEMAP_H + +# include + +struct fiemap_extent +{ + /* Logical offset in bytes for the start of the extent + from the beginning of the file. */ + uint64_t fe_logical; + + /* Physical offset in bytes for the start of the extent + from the beginning of the disk. */ + uint64_t fe_physical; + + /* Length in bytes for this extent. */ + uint64_t fe_length; + + uint64_t fe_reserved64[2]; + + /* FIEMAP_EXTENT_* flags for this extent. */ + uint32_t fe_flags; + + uint32_t fe_reserved[3]; +}; + +struct fiemap +{ + /* Logical offset(inclusive) at which to start mapping(in). */ + uint64_t fm_start; + + /* Logical length of mapping which userspace wants(in). */ + uint64_t fm_length; + + /* FIEMAP_FLAG_* flags for request(in/out). */ + uint32_t fm_flags; + + /* Number of extents that were mapped(out). */ + uint32_t fm_mapped_extents; + + /* Size of fm_extents array(in). */ + uint32_t fm_extent_count; + + uint32_t fm_reserved; + + /* Array of mapped extents(out). */ + struct fiemap_extent fm_extents[0]; +}; + +/* The maximum offset can be mapped for a file. */ +# define FIEMAP_MAX_OFFSET (~0ULL) + +/* Sync file data before map. */ +# define FIEMAP_FLAG_SYNC 0x00000001 + +/* Map extented attribute tree. */ +# define FIEMAP_FLAG_XATTR 0x00000002 + +# define FIEMAP_FLAGS_COMPAT (FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR) + +/* Last extent in file. */ +# define FIEMAP_EXTENT_LAST 0x00000001 + +/* Data location unknown. */ +# define FIEMAP_EXTENT_UNKNOWN 0x00000002 + +/* Location still pending, Sets EXTENT_UNKNOWN. */ +# define FIEMAP_EXTENT_DELALLOC 0x00000004 + +/* Data can not be read while fs is unmounted. */ +# define FIEMAP_EXTENT_ENCODED 0x00000008 + +/* Data is encrypted by fs. Sets EXTENT_NO_BYPASS. */ +# define FIEMAP_EXTENT_DATA_ENCRYPTED 0x00000080 + +/* Extent offsets may not be block aligned. */ +# define FIEMAP_EXTENT_NOT_ALIGNED 0x00000100 + +/* Data mixed with metadata. Sets EXTENT_NOT_ALIGNED. */ +# define FIEMAP_EXTENT_DATA_INLINE 0x00000200 + +/* Multiple files in block. Set EXTENT_NOT_ALIGNED. */ +# define FIEMAP_EXTENT_DATA_TAIL 0x00000400 + +/* Space allocated, but not data (i.e. zero). */ +# define FIEMAP_EXTENT_UNWRITTEN 0x00000800 + +/* File does not natively support extents. Result merged for efficiency. */ +# define FIEMAP_EXTENT_MERGED 0x00001000 + +/* Space shared with other files. */ +# define FIEMAP_EXTENT_SHARED 0x00002000 + +#endif -- 1.5.4.3 >From 99927649578934e3436d8370e3a7d34da5ee9195 Mon Sep 17 00:00:00 2001 From: Jie Liu Date: Mon, 19 Apr 2010 15:40:05 +0800 Subject: [PATCH 2/2] Add fiemap copy test script to tests/cp/sparse-fiemap-copy. Signed-off-by: Jie Liu --- tests/Makefile.am | 1 + tests/cp/sparse-fiemap-copy | 71 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 0 deletions(-) create mode 100755 tests/cp/sparse-fiemap-copy diff --git a/tests/Makefile.am b/tests/Makefile.am index db1610d..21c1f32 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -315,6 +315,7 @@ TESTS = \ cp/same-file \ cp/slink-2-slink \ cp/sparse \ + cp/sparse-fiemap-copy \ cp/special-f \ cp/src-base-dot \ cp/symlink-slash \ diff --git a/tests/cp/sparse-fiemap-copy b/tests/cp/sparse-fiemap-copy new file mode 100755 index 0000000..8ae31da --- /dev/null +++ b/tests/cp/sparse-fiemap-copy @@ -0,0 +1,71 @@ +#!/bin/sh +# Test cp --sparse=always with fiemap copy + +# Copyright (C) 2006-2010 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 . + +if test "$VERBOSE" = yes; then + set -x + cp --version +fi + +. $srcdir/test-lib.sh + +test -x "/bin/cp" && cp_orig="/bin/cp" || test -x "/usr/bin/cp" && cp_orig="/usr/bin/cp" \ +|| test -x "/usr/local/bin/cp" && cp_orig="/usr/local/bin/cp" \ +|| test -x "$abs_top_builddir/src/cp" && cp_new="$abs_top_builddir/src/cp" || Exit 1 + +# Please forgive me for the hard code +ocfs2_mount_point="/ocfs2" +ocfs2_sparse=$ocfs2_mount_point/sparse_file +ocfs2_sparse_normal=$ocfs2_mount_point/sparse_normal_copy +ocfs2_sparse_fiemap=$ocfs2_mount_point/sparse_fiemap_copy + +btrfs_mount_point="/btrfs" +btrfs_sparse=$btrfs_mount_point/sparse_file +btrfs_sparse_normal=$btrfs_mount_point/sparse_normal_copy +btrfs_sparse_fiemap=$btrfs_mount_point/sparse_fiemap_copy + +ext4_mount_point="/ext4" +ext4_sparse=$ext4_mount_point/sparse_file +ext4_sparse_normal=$ext4_mount_point/sparse_normal_copy +ext4_sparse_fiemap=$ext4_mount_point/sparse_fiemap_copy + +# Create a 41Mbytes sparse file for each file sysytem. +size=`expr 10 \* 1024` +dd if=/dev/zero bs=4k count=1 seek=$size of=$ocfs2_sparse > /dev/null 2>&1 || fail=1 +dd if=/dev/zero bs=4k count=1 seek=$size of=$btrfs_sparse > /dev/null 2>&1 || fail=1 +dd if=/dev/zero bs=4k count=1 seek=$size of=$ext4_sparse > /dev/null 2>&1 || fail=1 + +my_time="/usr/bin/time" +t1=$($my_time -f "%U + %S" $cp_orig --sparse=always $ocfs2_sparse $ocfs2_sparse_normal 2>&1 | bc) +t2=$($my_time -f "%U + %S" $cp_new --sparse=always $ocfs2_sparse $ocfs2_sparse_fiemap 2>&1 | bc) +# Ensure that the fiemap copy has the same size in bytes as the original. +test `stat --printf %s $ocfs2_sparse` -eq `stat --printf %s $ocfs2_sparse_fiemap` || fail=1 +echo "$t2 < $t1" | bc || fail=1 + +t1=$($my_time -f "%U + %S" $cp_orig --sparse=always $btrfs_sparse $btrfs_sparse_normal 2>&1 | bc) +t2=$($my_time -f "%U + %S" $cp_new --sparse=always $btrfs_sparse $btrfs_sparse_fiemap 2>&1 | bc) +# Ensure that the fiemap copy has the same size in bytes as the original. +test `stat --printf %s $btrfs_sparse` -eq `stat --printf %s $btrfs_sparse_fiemap` || fail=1 +echo "$t2 < $t1" | bc || fail=1 + +t1=$($my_time -f "%U + %S" $cp_orig --sparse=always $ext4_sparse $ext4_sparse_normal 2>&1 | bc) +t2=$($my_time -f "%U + %S" $cp_new --sparse=always $ext4_sparse $ext4_sparse_fiemap 2>&1 | bc) +# Ensure that the fiemap copy has the same size in bytes as the original. +test `stat --printf %s $ext4_sparse` -eq `stat --printf %s $ext4_sparse_fiemap` || fail=1 +echo "$t2 < $t1" | bc || fail=1 + +Exit $fail -- 1.5.4.3 Best Regards, -Jeff From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 21 23:04:59 2010 Received: (at 5915) by debbugs.gnu.org; 22 Apr 2010 03:05:00 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O4mip-0001ur-8x for submit@debbugs.gnu.org; Wed, 21 Apr 2010 23:04:59 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O4min-0001ul-7b for 5915@debbugs.gnu.org; Wed, 21 Apr 2010 23:04:58 -0400 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3M34pf0029531 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 22 Apr 2010 03:04:52 GMT Received: from acsmt353.oracle.com (acsmt353.oracle.com [141.146.40.153]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3LJnZfV025089; Thu, 22 Apr 2010 03:04:49 GMT Received: from abhmt007.oracle.com by acsmt354.oracle.com with ESMTP id 179806811271905415; Wed, 21 Apr 2010 20:03:35 -0700 Received: from [10.182.121.53] (/10.182.121.53) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 21 Apr 2010 20:03:34 -0700 Message-ID: <4BCFBCA9.5010507@oracle.com> Date: Thu, 22 Apr 2010 11:04:09 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> <87pr1yjg2m.fsf@meyering.net> <4BCC2653.2080702@oracle.com> In-Reply-To: <4BCC2653.2080702@oracle.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: acsinet15.oracle.com [141.146.126.227] X-CT-RefId: str=0001.0A090203.4BCFBCD5.0071:SCFMA922111,ss=1,pt=113351,fgs=0 X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?ISO-8859-1?Q?P=E1draig_Brady?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -4.3 (----) jeff.liu wrote: > Hi Jim, > > Thanks for your comments for my last patch submition! > > Now the fiemap copy logical changed to: > 1. If fiemap copy succeeds, go ahead with the meta data preserve procedure if they are user specified. > 2. If fiemap_copy fails and the 'normal_copy_required' is true, which means the ioctl(2) fails. > In this case, fall back to normal copy. 'bool normal_copy_required' in fiemap_copy_ok() is a new > argument added in this version used to determine whether normal copy require or not. > 3. If fiemap_copy fails for other reason, go to 'close_src_and_dst_desc' directly. > > I also add a test script to 'tests/cp/sparse-fiemap-copy'(I can not find out a better name at the > moment), it create a 41M sparse file on btrfs/ext4/ocfs2 separately, and then try copy them in those > 2 different ways. > It supply two result verify steps, the first is to compare the sum of 'sys and user' time by run > each copy via '/usr/bin/time', the fiemap copy should run faster than normal copy. > 2nd is compare the file size in bytes, the fiemap copied file size should be equal to the orignally. > > Would you please review the following patches at your convenient time? > > From 561b7b3413de16595fc71f0cba5bfa918b42f9e5 Mon Sep 17 00:00:00 2001 > From: Jie Liu > Date: Sun, 18 Apr 2010 14:31:20 +0800 > Subject: [PATCH 1/2] Introduct fiemap ioctl(2) for efficient sparse file copy. > > Signed-off-by: Jie Liu > --- > src/copy.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 255 insertions(+), 0 deletions(-) > create mode 100644 src/fiemap.h > > diff --git a/src/copy.c b/src/copy.c > index 3c32fa3..e7adecc 100644 > --- a/src/copy.c > +++ b/src/copy.c > @@ -65,6 +65,10 @@ > # include > #endif > > +#ifndef HAVE_FIEMAP > +# include "fiemap.h" > +#endif > + > #ifndef HAVE_FCHOWN > # define HAVE_FCHOWN false > # define fchown(fd, uid, gid) (-1) > @@ -151,6 +155,133 @@ clone_file (int dest_fd, int src_fd) > #endif > } > > +#ifdef __linux__ > +# ifndef FS_IOC_FIEMAP > +# define FS_IOC_FIEMAP _IOWR ('f', 11, struct fiemap) > +# endif > +/* Perform FIEMAP(available in mainline 2.6.27) copy if possible. > + Call ioctl(2) with FS_IOC_FIEMAP to efficiently map file allocation > + excepts holes. So the overhead to deal with holes with lseek(2) in > + normal copy could be saved. This would result in much faster backups > + for any kind of sparse file. */ > +static bool > +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, > + off_t src_total_size, char const *src_name, > + char const *dst_name, bool *normal_copy_required) > +{ > + bool fail = false; > + bool last = false; > + char fiemap_buf[4096]; > + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; > + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; > + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / > + sizeof (struct fiemap_extent); > + unsigned int i; > + off_t last_ext_logical = 0; > + uint64_t last_ext_len = 0; > + uint64_t last_read_size = 0; > + > + do > + { > + fiemap->fm_start = 0ULL; > + fiemap->fm_length = FIEMAP_MAX_OFFSET; > + fiemap->fm_extent_count = count; > + > + /* When ioctl(2) fails for any reason, fall back to the normal copy. */ > + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) > + { > + *normal_copy_required = true; > + return false; > + } > + > + /* If 0 extents are returned, then more ioctls are not needed. */ > + if (fiemap->fm_mapped_extents == 0) > + return true; This is a bug I just tried out. 'return true' is not enough if '0' extents returned. Below is the test case could reveal this issue, it just seek(2) but not write any byte, its extent count is *zero* because there is no data blocks allocated from the extent tree. In this case, I should truncate it to the src_desc.st_size, this issue will be fixed in my next post. dd bs=1 seek=10240 of=/ocfs2/sparse /dev/null Besides, I have tried this case on ocfs2,ext4 and btrfs, the first 2 file systems could handle this kind of sparse file properly, but btrfs actually allocaed an extent for it. IMHO, maybe this is a btrfs issue, I will post it to the mail-list for further discussion. > + > + for (i = 0; i < fiemap->fm_mapped_extents; i++) > + { > + assert (fm_ext[i].fe_logical <= OFF_T_MAX); > + > + off_t ext_logical = fm_ext[i].fe_logical; > + uint64_t ext_len = fm_ext[i].fe_length; > + > + if (lseek (src_fd, ext_logical, SEEK_SET) < 0LL) > + { > + error (0, errno, _("cannot lseek %s"), quote (src_name)); > + return fail; > + } > + > + if (lseek (dest_fd, ext_logical, SEEK_SET) < 0LL) > + { > + error (0, errno, _("cannot lseek %s"), quote (dst_name)); > + return fail; > + } > + > + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) > + { > + last_ext_logical = ext_logical; > + last_ext_len = ext_len; > + last = true; > + } > + > + while (0 < ext_len) > + { > + char buf[buf_size]; > + > + /* Avoid reading into the holes if the left extent > + length is shorter than the buffer size. */ > + if (ext_len < buf_size) > + buf_size = ext_len; > + > + 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 fail; > + } > + > + if (n_read == 0) > + { > + /* Figure out how many bytes read from the last extent. */ > + 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)); > + return fail; > + } > + > + ext_len -= n_read; > + } > + > + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); > + } > + } while (! last); > + goto Here if fiemap->fm_mapped_extents == 0. > + /* If a file ends up with holes, the sum of the last extent logical offset > + and the read-returned size will be shorter than the actual size of the > + file. Use ftruncate to extend the length of the destination file. */ > + if (last_ext_logical + last_read_size < src_total_size) > + { > + if (ftruncate (dest_fd, src_total_size) < 0) > + { > + error (0, errno, _("extending %s"), quote (dst_name)); > + return fail; > + } > + } > + > + return ! fail; > +} > +#else > +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } > +#endif > + > /* FIXME: describe */ > /* FIXME: rewrite this to use a hash table so we avoid the quadratic > performance hit that's probably noticeable only on trees deeper > @@ -679,6 +810,25 @@ copy_reg (char const *src_name, char const *dst_name, > #endif > } > > + if (make_holes) > + { > + bool require_normal_copy = false; > + /* Perform efficient FIEMAP copy for sparse files, fall back to the > + standard copy only if the ioctl(2) fails. */ > + if (fiemap_copy_ok (source_desc, dest_desc, buf_size, > + src_open_sb.st_size, src_name, > + dst_name, &require_normal_copy)) > + goto preserve_metadata; > + else > + { > + if (! require_normal_copy) > + { > + return_val = false; > + goto close_src_and_dst_desc; > + } > + } > + } > + > /* If not making a sparse file, try to use a more-efficient > buffer size. */ > if (! make_holes) > @@ -807,6 +957,8 @@ copy_reg (char const *src_name, char const *dst_name, > } > } > > +preserve_metadata: > + > if (x->preserve_timestamps) > { > struct timespec timespec[2]; > @@ -897,6 +1049,7 @@ close_src_desc: > > free (buf_alloc); > free (name_alloc); > + > return return_val; > } > > diff --git a/src/fiemap.h b/src/fiemap.h > new file mode 100644 > index 0000000..d33293b > --- /dev/null > +++ b/src/fiemap.h > @@ -0,0 +1,102 @@ > +/* FS_IOC_FIEMAP ioctl infrastructure. > + Some portions copyright (C) 2007 Cluster File Systems, Inc > + Authors: Mark Fasheh > + Kalpak Shah > + Andreas Dilger . */ > + > +/* Copy from kernel, modified to respect GNU code style by Jie Liu. */ > + > +#ifndef _LINUX_FIEMAP_H > +# define _LINUX_FIEMAP_H > + > +# include > + > +struct fiemap_extent > +{ > + /* Logical offset in bytes for the start of the extent > + from the beginning of the file. */ > + uint64_t fe_logical; > + > + /* Physical offset in bytes for the start of the extent > + from the beginning of the disk. */ > + uint64_t fe_physical; > + > + /* Length in bytes for this extent. */ > + uint64_t fe_length; > + > + uint64_t fe_reserved64[2]; > + > + /* FIEMAP_EXTENT_* flags for this extent. */ > + uint32_t fe_flags; > + > + uint32_t fe_reserved[3]; > +}; > + > +struct fiemap > +{ > + /* Logical offset(inclusive) at which to start mapping(in). */ > + uint64_t fm_start; > + > + /* Logical length of mapping which userspace wants(in). */ > + uint64_t fm_length; > + > + /* FIEMAP_FLAG_* flags for request(in/out). */ > + uint32_t fm_flags; > + > + /* Number of extents that were mapped(out). */ > + uint32_t fm_mapped_extents; > + > + /* Size of fm_extents array(in). */ > + uint32_t fm_extent_count; > + > + uint32_t fm_reserved; > + > + /* Array of mapped extents(out). */ > + struct fiemap_extent fm_extents[0]; > +}; > + > +/* The maximum offset can be mapped for a file. */ > +# define FIEMAP_MAX_OFFSET (~0ULL) > + > +/* Sync file data before map. */ > +# define FIEMAP_FLAG_SYNC 0x00000001 > + > +/* Map extented attribute tree. */ > +# define FIEMAP_FLAG_XATTR 0x00000002 > + > +# define FIEMAP_FLAGS_COMPAT (FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR) > + > +/* Last extent in file. */ > +# define FIEMAP_EXTENT_LAST 0x00000001 > + > +/* Data location unknown. */ > +# define FIEMAP_EXTENT_UNKNOWN 0x00000002 > + > +/* Location still pending, Sets EXTENT_UNKNOWN. */ > +# define FIEMAP_EXTENT_DELALLOC 0x00000004 > + > +/* Data can not be read while fs is unmounted. */ > +# define FIEMAP_EXTENT_ENCODED 0x00000008 > + > +/* Data is encrypted by fs. Sets EXTENT_NO_BYPASS. */ > +# define FIEMAP_EXTENT_DATA_ENCRYPTED 0x00000080 > + > +/* Extent offsets may not be block aligned. */ > +# define FIEMAP_EXTENT_NOT_ALIGNED 0x00000100 > + > +/* Data mixed with metadata. Sets EXTENT_NOT_ALIGNED. */ > +# define FIEMAP_EXTENT_DATA_INLINE 0x00000200 > + > +/* Multiple files in block. Set EXTENT_NOT_ALIGNED. */ > +# define FIEMAP_EXTENT_DATA_TAIL 0x00000400 > + > +/* Space allocated, but not data (i.e. zero). */ > +# define FIEMAP_EXTENT_UNWRITTEN 0x00000800 > + > +/* File does not natively support extents. Result merged for efficiency. */ > +# define FIEMAP_EXTENT_MERGED 0x00001000 > + > +/* Space shared with other files. */ > +# define FIEMAP_EXTENT_SHARED 0x00002000 > + > +#endif From debbugs-submit-bounces@debbugs.gnu.org Thu Apr 22 10:45:44 2010 Received: (at 5915) by debbugs.gnu.org; 22 Apr 2010 14:45:44 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O4xex-0007dZ-K9 for submit@debbugs.gnu.org; Thu, 22 Apr 2010 10:45:44 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O4xeu-0007dU-PI for 5915@debbugs.gnu.org; Thu, 22 Apr 2010 10:45:41 -0400 Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3MEjXxN032609 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 22 Apr 2010 14:45:34 GMT Received: from acsmt353.oracle.com (acsmt353.oracle.com [141.146.40.153]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id o3MEjV0Q004285; Thu, 22 Apr 2010 14:45:31 GMT Received: from abhmt017.oracle.com by acsmt353.oracle.com with ESMTP id 199391001271947426; Thu, 22 Apr 2010 07:43:46 -0700 Received: from [123.119.97.114] (/123.119.97.114) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 22 Apr 2010 07:43:45 -0700 Message-ID: <4BD060BE.8000703@oracle.com> Date: Thu, 22 Apr 2010 22:44:14 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> <87pr1yjg2m.fsf@meyering.net> <4BCC2653.2080702@oracle.com> <4BCFBCA9.5010507@oracle.com> In-Reply-To: <4BCFBCA9.5010507@oracle.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Auth-Type: Internal IP X-Source-IP: acsinet15.oracle.com [141.146.126.227] X-CT-RefId: str=0001.0A090209.4BD0610F.0091:SCFMA922111,ss=1,pt=113351,fgs=0 X-Spam-Score: -4.6 (----) X-Debbugs-Envelope-To: 5915 Cc: Sunil Mushran , 5915@debbugs.gnu.org, Joel Becker , Tao Ma , =?ISO-8859-1?Q?P=E1draig_Brady?= , chris.mason@oracle.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -4.3 (----) jeff.liu wrote: > jeff.liu wrote: >> Hi Jim, >> >> Thanks for your comments for my last patch submition! >> >> Now the fiemap copy logical changed to: >> 1. If fiemap copy succeeds, go ahead with the meta data preserve procedure if they are user specified. >> 2. If fiemap_copy fails and the 'normal_copy_required' is true, which means the ioctl(2) fails. >> In this case, fall back to normal copy. 'bool normal_copy_required' in fiemap_copy_ok() is a new >> argument added in this version used to determine whether normal copy require or not. >> 3. If fiemap_copy fails for other reason, go to 'close_src_and_dst_desc' directly. >> >> I also add a test script to 'tests/cp/sparse-fiemap-copy'(I can not find out a better name at the >> moment), it create a 41M sparse file on btrfs/ext4/ocfs2 separately, and then try copy them in those >> 2 different ways. >> It supply two result verify steps, the first is to compare the sum of 'sys and user' time by run >> each copy via '/usr/bin/time', the fiemap copy should run faster than normal copy. >> 2nd is compare the file size in bytes, the fiemap copied file size should be equal to the orignally. >> >> Would you please review the following patches at your convenient time? >> >> From 561b7b3413de16595fc71f0cba5bfa918b42f9e5 Mon Sep 17 00:00:00 2001 >> From: Jie Liu >> Date: Sun, 18 Apr 2010 14:31:20 +0800 >> Subject: [PATCH 1/2] Introduct fiemap ioctl(2) for efficient sparse file copy. >> >> Signed-off-by: Jie Liu >> --- >> src/copy.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> src/fiemap.h | 102 ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 255 insertions(+), 0 deletions(-) >> create mode 100644 src/fiemap.h >> >> diff --git a/src/copy.c b/src/copy.c >> index 3c32fa3..e7adecc 100644 >> --- a/src/copy.c >> +++ b/src/copy.c >> @@ -65,6 +65,10 @@ >> # include >> #endif >> >> +#ifndef HAVE_FIEMAP >> +# include "fiemap.h" >> +#endif >> + >> #ifndef HAVE_FCHOWN >> # define HAVE_FCHOWN false >> # define fchown(fd, uid, gid) (-1) >> @@ -151,6 +155,133 @@ clone_file (int dest_fd, int src_fd) >> #endif >> } >> >> +#ifdef __linux__ >> +# ifndef FS_IOC_FIEMAP >> +# define FS_IOC_FIEMAP _IOWR ('f', 11, struct fiemap) >> +# endif >> +/* Perform FIEMAP(available in mainline 2.6.27) copy if possible. >> + Call ioctl(2) with FS_IOC_FIEMAP to efficiently map file allocation >> + excepts holes. So the overhead to deal with holes with lseek(2) in >> + normal copy could be saved. This would result in much faster backups >> + for any kind of sparse file. */ >> +static bool >> +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, >> + off_t src_total_size, char const *src_name, >> + char const *dst_name, bool *normal_copy_required) >> +{ >> + bool fail = false; >> + bool last = false; >> + char fiemap_buf[4096]; >> + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; >> + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; >> + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / >> + sizeof (struct fiemap_extent); >> + unsigned int i; >> + off_t last_ext_logical = 0; >> + uint64_t last_ext_len = 0; >> + uint64_t last_read_size = 0; >> + >> + do >> + { >> + fiemap->fm_start = 0ULL; >> + fiemap->fm_length = FIEMAP_MAX_OFFSET; >> + fiemap->fm_extent_count = count; >> + >> + /* When ioctl(2) fails for any reason, fall back to the normal copy. */ >> + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) >> + { >> + *normal_copy_required = true; >> + return false; >> + } >> + >> + /* If 0 extents are returned, then more ioctls are not needed. */ >> + if (fiemap->fm_mapped_extents == 0) >> + return true; > This is a bug I just tried out. 'return true' is not enough if '0' extents returned. > Below is the test case could reveal this issue, it just seek(2) but not write any byte, its extent > count is *zero* because there is no data blocks allocated from the extent tree. In this case, > I should truncate it to the src_desc.st_size, this issue will be fixed in my next post. > dd bs=1 seek=10240 of=/ocfs2/sparse /dev/null > > Besides, I have tried this case on ocfs2,ext4 and btrfs, the first 2 file systems could handle this > kind of sparse file properly, but btrfs actually allocaed an extent for it. IMHO, maybe this is a > btrfs issue, I will post it to the mail-list for further discussion. FYI, Tao has submitted a patch against the issue which I mentioned above. For detail, please check the following link: http://permalink.gmane.org/gmane.comp.file-systems.btrfs/5416 I have tried this fix, it works fine. Besides that, looks this fix also solved the performance issue for fiemap copy vs normal copy on btrfs: jeff@jeff-laptop:~/opensource_dev/coreutils$ ls -l /btrfs/sparse_file -rw-r--r-- 1 jeff jeff 41947136 Apr 19 16:54 /btrfs/sparse_file jeff@jeff-laptop:~/opensource_dev/coreutils$ time /bin/cp --sparse=always /btrfs/sparse_file /btrfs/sparse_normal_copy real 0m0.208s user 0m0.016s sys 0m0.164s jeff@jeff-laptop:~/opensource_dev/coreutils$ time ./src/cp --sparse=always /btrfs/sparse_file /btrfs/sparse_fiemap_copy real 0m0.050s user 0m0.000s sys 0m0.040s Below is the updated patch, it fixed 2 issues: 1. If the extent count is 0, we should truncated it to the "src_desc (struct stat)buf.st_size" if possible instead of return. 2. I just think of another possible issue, if ioctl(2) fails, but it is not the first time in the loop. In this point, we also need to stop the copy rather than fall back to the normal one. Otherwise, the destination file will be corrupted as well, Am I right? >From 9d049ceef3cfefbf3a85254044458dd9a39eb944 Mon Sep 17 00:00:00 2001 From: Jie Liu Date: Thu, 22 Apr 2010 21:48:09 +0800 Subject: [PATCH 1/1] Add fiemap support for efficient sparse file copy. Signed-off-by: Jie Liu --- src/copy.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 158 insertions(+), 0 deletions(-) diff --git a/src/copy.c b/src/copy.c index 3c32fa3..5f2da52 100644 --- a/src/copy.c +++ b/src/copy.c @@ -65,6 +65,10 @@ # include #endif +#ifndef HAVE_FIEMAP +# include "fiemap.h" +#endif + #ifndef HAVE_FCHOWN # define HAVE_FCHOWN false # define fchown(fd, uid, gid) (-1) @@ -151,6 +155,138 @@ clone_file (int dest_fd, int src_fd) #endif } +#ifdef __linux__ +# ifndef FS_IOC_FIEMAP +# define FS_IOC_FIEMAP _IOWR ('f', 11, struct fiemap) +# endif +/* Perform FIEMAP(available in mainline 2.6.27) copy if possible. + Call ioctl(2) with FS_IOC_FIEMAP to efficiently map file allocation + excepts holes. So the overhead to deal with holes with lseek(2) in + normal copy could be saved. This would result in much faster backups + for any kind of sparse file. */ +static bool +fiemap_copy_ok (int src_fd, int dest_fd, size_t buf_size, + off_t src_total_size, char const *src_name, + char const *dst_name, bool *normal_copy_required) +{ + bool fail = false; + bool last = false; + char fiemap_buf[4096]; + struct fiemap *fiemap = (struct fiemap *)fiemap_buf; + struct fiemap_extent *fm_ext = &fiemap->fm_extents[0]; + uint32_t count = (sizeof (fiemap_buf) - sizeof (*fiemap)) / + sizeof (struct fiemap_extent); + off_t last_ext_logical = 0; + uint64_t last_ext_len = 0; + uint64_t last_read_size = 0; + unsigned int n_ioctl = 0; + unsigned int i; + + do + { + fiemap->fm_start = 0ULL; + fiemap->fm_length = FIEMAP_MAX_OFFSET; + fiemap->fm_extent_count = count; + + /* When ioctl(2) fails, if this is the frist time we met, fall back + to the normal copy. */ + if (ioctl (src_fd, FS_IOC_FIEMAP, (unsigned long) fiemap) < 0) + { + if (n_ioctl < 1) + *normal_copy_required = true; + return false; + } + + ++n_ioctl; + + /* If 0 extents are returned, then more ioctls are not needed. */ + if (fiemap->fm_mapped_extents == 0) + break; + + for (i = 0; i < fiemap->fm_mapped_extents; i++) + { + assert (fm_ext[i].fe_logical <= OFF_T_MAX); + + off_t ext_logical = fm_ext[i].fe_logical; + uint64_t ext_len = fm_ext[i].fe_length; + + if (lseek (src_fd, ext_logical, SEEK_SET) < 0LL) + { + error (0, errno, _("cannot lseek %s"), quote (src_name)); + return fail; + } + + if (lseek (dest_fd, ext_logical, SEEK_SET) < 0LL) + { + error (0, errno, _("cannot lseek %s"), quote (dst_name)); + return fail; + } + + if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST) + { + last_ext_logical = ext_logical; + last_ext_len = ext_len; + last = true; + } + + while (0 < ext_len) + { + char buf[buf_size]; + + /* Avoid reading into the holes if the left extent + length is shorter than the buffer size. */ + if (ext_len < buf_size) + buf_size = ext_len; + + 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 fail; + } + + if (n_read == 0) + { + /* Figure out how many bytes read from the last extent. */ + 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)); + return fail; + } + + ext_len -= n_read; + } + + fiemap->fm_start = (fm_ext[i-1].fe_logical + fm_ext[i-1].fe_length); + } + } while (! last); + + /* If a file ends up with holes, the sum of the last extent logical offset + and the read-returned size will be shorter than the actual size of the + file. Use ftruncate to extend the length of the destination file. */ + if (last_ext_logical + last_read_size < src_total_size) + { + if (ftruncate (dest_fd, src_total_size) < 0) + { + error (0, errno, _("extending %s"), quote (dst_name)); + return fail; + } + } + + return ! fail; +} +#else +static bool fiemap_copy_ok (ignored) { errno == ENOTSUP; return false; } +#endif + /* FIXME: describe */ /* FIXME: rewrite this to use a hash table so we avoid the quadratic performance hit that's probably noticeable only on trees deeper @@ -679,6 +815,25 @@ copy_reg (char const *src_name, char const *dst_name, #endif } + if (make_holes) + { + bool require_normal_copy = false; + /* Perform efficient FIEMAP copy for sparse files, fall back to the + standard copy only if the ioctl(2) fails. */ + if (fiemap_copy_ok (source_desc, dest_desc, buf_size, + src_open_sb.st_size, src_name, + dst_name, &require_normal_copy)) + goto preserve_metadata; + else + { + if (! require_normal_copy) + { + return_val = false; + goto close_src_and_dst_desc; + } + } + } + /* If not making a sparse file, try to use a more-efficient buffer size. */ if (! make_holes) @@ -807,6 +962,8 @@ copy_reg (char const *src_name, char const *dst_name, } } +preserve_metadata: + if (x->preserve_timestamps) { struct timespec timespec[2]; @@ -897,6 +1054,7 @@ close_src_desc: free (buf_alloc); free (name_alloc); + return return_val; } -- 1.5.4.3 Best Regards, -Jeff From debbugs-submit-bounces@debbugs.gnu.org Tue May 31 17:10:52 2011 Received: (at 5915-done) by debbugs.gnu.org; 31 May 2011 21:10:53 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QRWDE-0001lP-BP for submit@debbugs.gnu.org; Tue, 31 May 2011 17:10:52 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QRWDC-0001lD-Jy for 5915-done@debbugs.gnu.org; Tue, 31 May 2011 17:10:51 -0400 Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id B71D26018E; Tue, 31 May 2011 23:10:44 +0200 (CEST) From: Jim Meyering To: "jeff.liu" Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 In-Reply-To: <4BC9B0F9.6090700@oracle.com> (jeff liu's message of "Sat, 17 Apr 2010 21:00:41 +0800") References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> <4BC9B0F9.6090700@oracle.com> Date: Tue, 31 May 2011 23:10:44 +0200 Message-ID: <87vcwqfuej.fsf@rho.meyering.net> Lines: 2 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -6.0 (------) X-Debbugs-Envelope-To: 5915-done Cc: 5915-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.0 (------) Hi Jeff, Just a heads up that I've closed this issue. From debbugs-submit-bounces@debbugs.gnu.org Wed Jun 29 07:06:12 2011 Received: (at 5915) by debbugs.gnu.org; 29 Jun 2011 11:06:12 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qbsax-0003Z0-33 for submit@debbugs.gnu.org; Wed, 29 Jun 2011 07:06:11 -0400 Received: from rcsinet10.oracle.com ([148.87.113.121]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qbsav-0003Yo-4k for 5915@debbugs.gnu.org; Wed, 29 Jun 2011 07:06:09 -0400 Received: from acsinet22.oracle.com (acsinet22.oracle.com [141.146.126.238]) by rcsinet10.oracle.com (Switch-3.4.4/Switch-3.4.2) with ESMTP id p5TB60xb013693 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 29 Jun 2011 11:06:02 GMT Received: from acsmt357.oracle.com (acsmt357.oracle.com [141.146.40.157]) by acsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id p5TB5xn7020108 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 29 Jun 2011 11:06:00 GMT Received: from abhmt107.oracle.com (abhmt107.oracle.com [141.146.116.59]) by acsmt357.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id p5TB5snR006668; Wed, 29 Jun 2011 06:05:54 -0500 Received: from [10.191.12.227] (/10.191.12.227) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 29 Jun 2011 04:05:54 -0700 Message-ID: <4E0B0734.8070903@oracle.com> Date: Wed, 29 Jun 2011 19:06:28 +0800 From: "jeff.liu" User-Agent: Thunderbird 2.0.0.14 (X11/20080505) MIME-Version: 1.0 To: 5915@debbugs.gnu.org, jim@meyering.net Subject: Re: bug#5915: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2 References: <4BB8ADB2.9070604@oracle.com> <4BBF2E8B.8050403@oracle.com> <87hbnk4tsu.fsf@meyering.net> <4BBF490E.6020005@oracle.com> <87iq8039tp.fsf@meyering.net> <4BC970DC.5020004@oracle.com> <87sk6ul8px.fsf@meyering.net> <4BC9ABB4.6060500@oracle.com> <4BC9B0F9.6090700@oracle.com> <87vcwqfuej.fsf@rho.meyering.net> In-Reply-To: <87vcwqfuej.fsf@rho.meyering.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Source-IP: acsinet22.oracle.com [141.146.126.238] X-Auth-Type: Internal IP X-CT-RefId: str=0001.0A090208.4E0B071B.001D:SCFMA922111,ss=1,re=-4.000,fgs=0 X-Spam-Score: -6.4 (------) X-Debbugs-Envelope-To: 5915 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.4 (------) Hi Jim, Pretty sorry for my late response and thanks for your head up! Cheers, -Jeff Jim Meyering wrote: > Hi Jeff, > Just a heads up that I've closed this issue. > > > From unknown Fri Sep 05 09:10:54 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Wed, 27 Jul 2011 11:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator