Subject: Re: bug#6131: [PATCH]: fiemap support for efficient sparse file copy

From: "jeff.liu" <[email-masked]>
Date: Mon, 20 Sep 2010 15:18:01 +0800

Hi Jim and All,

Do you have any comments for the current implementation?

Sorry for my so delayed asking.

Thanks,
-Jeff

jeff.liu wrote:
> Hello All,
> 
> Below is my patches to isolate the extents scan and fetch functions in a new module to improve its
> extendibility.
> 
> It introduce a new file 'src/extent-scan.c' to place those functions, and then call those functions
> from extents_copy() at copy_reg() to process the regular file copy.
> 
> In addition to this, another major change is to copy all data if '--sparse=never' option is
> specified.  It write all data to destination file but using extents_copy() if available for
> efficient read source file, and it try to figure out the holes between the previous and current
> extents, and call fill_with_holes_ok() to write zeros as holes to the destination file if it is.
> Call file_with_holes_ok() to write zeros up to the source file size if hit the last extent of the
> source file and there is a hole behind it.
> 
> I have not implement the solaris lseek(2) at the moment for lack of solaris environment, it need to
> delay a period of time.
> 
> According to my tryout, it works for those 4 filesystems in common use, you all know.
> As usual, any comments are welcome!
> 
> 
> From 70773fdf1d85ba070e054b0467a7a0e1e2b00ea8 Mon Sep 17 00:00:00 2001
> From: Jie Liu <[email-masked]>
> Date: Tue, 20 Jul 2010 20:35:25 +0800
> Subject: [PATCH 1/3] cp: delete fiemap_copy() related stuff from copy.c
> 
> * delete fiemap_copy(), now it is implemented as a module.
> 
> Signed-off-by: Jie Liu <[email-masked]>
> ---
>  src/copy.c |  171 ------------------------------------------------------------
>  1 files changed, 0 insertions(+), 171 deletions(-)
> 
> diff --git a/src/copy.c b/src/copy.c
> index f48c74d..171499c 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -63,10 +63,6 @@
> 
>  #include <sys/ioctl.h>
> 
> -#ifndef HAVE_FIEMAP
> -# include "fiemap.h"
> -#endif
> -
>  #ifndef HAVE_FCHOWN
>  # define HAVE_FCHOWN false
>  # define fchown(fd, uid, gid) (-1)
> @@ -153,153 +149,6 @@ 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 a FIEMAP copy, if possible.
> -   Call ioctl(2) with FS_IOC_FIEMAP (available in linux 2.6.27) to
> -   obtain a map of file extents excluding holes.  This avoids the
> -   overhead of detecting holes in a hole-introducing/preserving copy,
> -   and thus makes copying sparse files much more efficient.  Upon a
> -   successful copy, return true.  If the initial ioctl fails, set
> -   *NORMAL_COPY_REQUIRED to true and return false.  Upon any other
> -   failure, set *NORMAL_COPY_REQUIRED to false and return false.  */
> -static bool
> -fiemap_copy (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 last = false;
> -  union { struct fiemap f; char c[4096]; } fiemap_buf;
> -  struct fiemap *fiemap = &fiemap_buf.f;
> -  struct fiemap_extent *fm_ext = &fiemap->fm_extents[0];
> -  enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_ext };
> -  verify (count != 0);
> -
> -  off_t last_ext_logical = 0;
> -  uint64_t last_ext_len = 0;
> -  uint64_t last_read_size = 0;
> -  unsigned int i = 0;
> -  *normal_copy_required = false;
> -
> -  /* This is required at least to initialize fiemap->fm_start,
> -     but also serves (in mid 2010) to appease valgrind, which
> -     appears not to know the semantics of the FIEMAP ioctl. */
> -  memset (&fiemap_buf, 0, sizeof fiemap_buf);
> -
> -  do
> -    {
> -      fiemap->fm_length = FIEMAP_MAX_OFFSET;
> -      fiemap->fm_flags = FIEMAP_FLAG_SYNC;
> -      fiemap->fm_extent_count = count;
> -
> -      /* When ioctl(2) fails, fall back to the normal copy only if it
> -         is the first time we met.  */
> -      if (ioctl (src_fd, FS_IOC_FIEMAP, fiemap) < 0)
> -        {
> -          /* If the first ioctl fails, tell the caller that it is
> -             ok to proceed with a normal copy.  */
> -          if (i == 0)
> -            *normal_copy_required = true;
> -          else
> -            {
> -              /* If the second or subsequent ioctl fails, diagnose it,
> -                 since it ends up causing the entire copy/cp to fail.  */
> -              error (0, errno, _("%s: FIEMAP ioctl failed"), quote (src_name));
> -            }
> -          return false;
> -        }
> -
> -      /* 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) < 0)
> -            {
> -              error (0, errno, _("cannot lseek %s"), quote (src_name));
> -              return false;
> -            }
> -
> -          if (lseek (dest_fd, ext_logical, SEEK_SET) < 0)
> -            {
> -              error (0, errno, _("cannot lseek %s"), quote (dst_name));
> -              return false;
> -            }
> -
> -          if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST)
> -            {
> -              last_ext_logical = ext_logical;
> -              last_ext_len = ext_len;
> -              last = true;
> -            }
> -
> -          while (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 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 false;
> -                }
> -
> -              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, _("failed to extend %s"), quote (dst_name));
> -          return false;
> -        }
> -    }
> -
> -  return true;
> -}
> -#else
> -static bool fiemap_copy (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
> @@ -830,25 +679,6 @@ copy_reg (char const *src_name, char const *dst_name,
>  #endif
>          }
> 
> -      if (make_holes)
> -        {
> -          bool require_normal_copy;
> -          /* Perform efficient FIEMAP copy for sparse files, fall back to the
> -             standard copy only if the ioctl(2) fails.  */
> -          if (fiemap_copy (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)
> @@ -977,7 +807,6 @@ copy_reg (char const *src_name, char const *dst_name,
>          }
>      }
> 
> -preserve_metadata:
>    if (x->preserve_timestamps)
>      {
>        struct timespec timespec[2];