From 2db1433eabc847d9ad43e059764222e9b33233aa Mon Sep 17 00:00:00 2001 From: "jeff.liu" Date: Wed, 29 Sep 2010 16:11:41 +0800 Subject: [PATCH] fiemap copy: add extent-scan.[ch], avoid a double-free and reorganize Changes: ======== 1. fix write_zeros() per Jim's comments. 2. remove char const *fname from struct extent_scan. 3. change the signature of open_extent_scan() from "void open_extent_scan(struct extent_scan **scan)" to "void open_extent_scan(struct extent_scan *scan)" to avoid having to malloc the extent_scan variable; instead save it on the stack. 4. move close_extent_scan() from a function defined in extent-scan.c to extent-scan.h as a macro definition, but it does nothing for now, since initial extent scan defined at stack. 5. add a macro "free_extents_info()" defined at extent-scan.h to release the memory allocated to extent info which should be called combine with get_extents_info(), it just one line, so IMHO, define it as macro should be ok. * src/extent-scan.c: New file; functions to read "extents". * src/extent-scan.h: Header file of extent-scan.c. * src/Makefile.am: Reference it and link it to copy_source. * src/copy.c: Use the new functions and avoid double-free. --- src/Makefile.am | 2 +- src/copy.c | 193 ++++++++++++++++++++++++++++++++---------------------- src/extent-scan.c | 113 ++++++++++++++++++++++++++++++++ src/extent-scan.h | 68 +++++++++++++++++++ 4 files changed, 296 insertions(+), 80 deletions(-) create mode 100644 src/extent-scan.c create mode 100644 src/extent-scan.h diff --git a/src/Makefile.am b/src/Makefile.am index 986bd21..9fd7822 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -450,7 +450,7 @@ uninstall-local: fi; \ fi -copy_sources = copy.c cp-hash.c +copy_sources = copy.c cp-hash.c extent-scan.c # Use `ginstall' in the definition of PROGRAMS and in dependencies to avoid # confusion with the `install' target. The install rule transforms `ginstall' diff --git a/src/copy.c b/src/copy.c index ead15b8..2c3eabd 100644 --- a/src/copy.c +++ b/src/copy.c @@ -36,6 +36,7 @@ #include "buffer-lcm.h" #include "copy.h" #include "cp-hash.h" +#include "extent-scan.h" #include "error.h" #include "fcntl--.h" #include "file-set.h" @@ -152,74 +153,79 @@ 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) +write_zeros (int fd, uint64_t n_bytes) { - 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); + static char *zeros; + static size_t nz = IO_BUFSIZE; + + if (zeros == NULL) + { + static char fallback[1024]; + zeros = calloc (nz, 1); + if (zeros == NULL) + { + zeros = fallback; + nz = sizeof fallback; + } + } + + while (n_bytes) + { + uint64_t n = MIN (sizeof nz, n_bytes); + if ((full_write (fd, zeros, n)) != n) + return false; + n_bytes -= n; + } + + return true; +} +/* Perform an efficient extent copy, if possible. This avoids + the overhead of detecting holes in hole-introducing/preserving + copy, and thus makes copying sparse files much more efficient. + Upon a successful copy, return true. If the initial extent scan + 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 +extent_copy (int src_fd, int dest_fd, size_t buf_size, + off_t src_total_size, bool make_holes, + char const *src_name, char const *dst_name, + bool *require_normal_copy) +{ + struct extent_scan scan; 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; + unsigned int i; + bool ok = true; - /* 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); + open_extent_scan (src_fd, &scan); 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) + ok = get_extents_info (&scan); + if (! ok) { - /* 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 (scan.hit_last_extent) + break; + + if (scan.initial_scan_failed) { - /* 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)); + close_extent_scan (&scan); + *require_normal_copy = true; + return false; } + + error (0, errno, _("failed to get extents info %s"), 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++) + for (i = 0; i < scan.ei_count; 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; + off_t ext_logical = scan.ext_info[i].ext_logical; + uint64_t ext_len = scan.ext_info[i].ext_length; if (lseek (src_fd, ext_logical, SEEK_SET) < 0) { @@ -227,27 +233,37 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size, return false; } - if (lseek (dest_fd, ext_logical, SEEK_SET) < 0) + if (make_holes) { - error (0, errno, _("cannot lseek %s"), quote (dst_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) + else { - last_ext_logical = ext_logical; - last_ext_len = ext_len; - last = true; + /* If not making a sparse file, write zeros to the destination + file if there is a hole between the last and current extent. */ + if (last_ext_logical + last_ext_len < ext_logical) + { + uint64_t holes_len = ext_logical - last_ext_logical - last_ext_len; + if (! write_zeros (dest_fd, holes_len)) + return false; + } } + last_ext_logical = ext_logical; + last_ext_len = ext_len; + last_read_size = 0; + 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; + buf_size = MIN (ext_len, buf_size); ssize_t n_read = read (src_fd, buf, buf_size); if (n_read < 0) @@ -257,12 +273,12 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size, continue; #endif error (0, errno, _("reading %s"), quote (src_name)); - return false; + return false; } if (n_read == 0) { - /* Figure out how many bytes read from the last extent. */ + /* Figure out how many bytes read from the previous extent. */ last_read_size = last_ext_len - ext_len; break; } @@ -277,27 +293,44 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size, } } - fiemap->fm_start = fm_ext[i - 1].fe_logical + fm_ext[i - 1].fe_length; + /* Release the space allocated to scan->ext_info. */ + free_extents_info (&scan); + } while (! scan.hit_last_extent); - } while (! last); + /* Do nothing now. */ + close_extent_scan (&scan); /* 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) + and the read-returned size or the last extent length will be shorter than + the actual size of the file. Use ftruncate to extend the length of the + destination file if make_holes, or write zeros up to the actual size of the + file. */ + if (make_holes) { - if (ftruncate (dest_fd, src_total_size) < 0) + if (last_ext_logical + last_read_size < src_total_size) { - error (0, errno, _("failed to extend %s"), quote (dst_name)); - return false; + if (ftruncate (dest_fd, src_total_size) < 0) + { + error (0, errno, _("failed to extend %s"), quote (dst_name)); + return false; + } + } + } + else + { + if (last_ext_logical + last_ext_len < src_total_size) + { + uint64_t holes_len = src_total_size - last_ext_logical - last_ext_len; + if (0 < holes_len) + { + if (! write_zeros (dest_fd, holes_len)) + 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 @@ -830,11 +863,13 @@ copy_reg (char const *src_name, char const *dst_name, 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)) + /* Perform efficient extent copy for sparse file, fall back to the + standard copy only if the initial extent scan fails. If the + '--sparse=never' option was specified, we writing all data but + use extent copy if available to efficiently read. */ + if (extent_copy (source_desc, dest_desc, buf_size, + src_open_sb.st_size, make_holes, + src_name, dst_name, &require_normal_copy)) goto preserve_metadata; else { diff --git a/src/extent-scan.c b/src/extent-scan.c new file mode 100644 index 0000000..f371b87 --- /dev/null +++ b/src/extent-scan.c @@ -0,0 +1,113 @@ +/* extent-scan.c -- core functions for scanning extents + Copyright (C) 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 . + + Written by Jie Liu (jeff.liu@oracle.com). */ + +#include +#include +#include +#include +#include + +#include "system.h" +#include "extent-scan.h" +#include "error.h" +#include "quote.h" + +#ifndef HAVE_FIEMAP +# include "fiemap.h" +#endif + +/* Allocate space for struct extent_scan, initialize the entries if + necessary and return it as the input argument of get_extents_info(). */ +extern void +open_extent_scan (int src_fd, struct extent_scan *scan) +{ + scan->fd = src_fd; + scan->ei_count = 0; + scan->scan_start = 0; + scan->initial_scan_failed = false; + scan->hit_last_extent = false; +} + +#ifdef __linux__ +# ifndef FS_IOC_FIEMAP +# define FS_IOC_FIEMAP _IOWR ('f', 11, struct fiemap) +# endif +/* Call ioctl(2) with FS_IOC_FIEMAP (available in linux 2.6.27) to + obtain a map of file extents excluding holes. */ +extern bool +get_extents_info (struct extent_scan *scan) +{ + union { struct fiemap f; char c[4096]; } fiemap_buf; + struct fiemap *fiemap = &fiemap_buf.f; + struct fiemap_extent *fm_extents = &fiemap->fm_extents[0]; + enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_extents }; + verify (count != 0); + unsigned int i; + + /* 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); + + fiemap->fm_start = scan->scan_start; + fiemap->fm_flags = FIEMAP_FLAG_SYNC; + fiemap->fm_extent_count = count; + fiemap->fm_length = FIEMAP_MAX_OFFSET - scan->scan_start; + + /* Fall back to the standard copy if call ioctl(2) failed for the + the first time. */ + if (ioctl (scan->fd, FS_IOC_FIEMAP, fiemap) < 0) + { + if (scan->scan_start == 0) + scan->initial_scan_failed = true; + return false; + } + + /* If 0 extents are returned, then more get_extent_table() are not needed. */ + if (fiemap->fm_mapped_extents == 0) + { + scan->hit_last_extent = true; + return false; + } + + scan->ei_count = fiemap->fm_mapped_extents; + scan->ext_info = xnmalloc (scan->ei_count, sizeof (struct extent_info)); + + for (i = 0; i < scan->ei_count; i++) + { + assert (fm_extents[i].fe_logical <= OFF_T_MAX); + + scan->ext_info[i].ext_logical = fm_extents[i].fe_logical; + scan->ext_info[i].ext_length = fm_extents[i].fe_length; + scan->ext_info[i].ext_flags = fm_extents[i].fe_flags; + } + + i--; + if (scan->ext_info[i].ext_flags & FIEMAP_EXTENT_LAST) + { + scan->hit_last_extent = true; + return true; + } + + scan->scan_start = fm_extents[i].fe_logical + fm_extents[i].fe_length; + + return true; +} +#else +extern bool get_extents_info (ignored) { errno = ENOTSUP; return false; } +#endif diff --git a/src/extent-scan.h b/src/extent-scan.h new file mode 100644 index 0000000..07c2e5b --- /dev/null +++ b/src/extent-scan.h @@ -0,0 +1,68 @@ +/* core functions for efficient reading sparse files + Copyright (C) 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 . + + Written by Jie Liu (jeff.liu@oracle.com). */ + +#ifndef EXTENT_SCAN_H +# define EXTENT_SCAN_H + +/* Structure used to reserve information of each extent. */ +struct extent_info +{ + /* Logical offset of an extent. */ + off_t ext_logical; + + /* Extent length. */ + uint64_t ext_length; + + /* Extent flags, use it for FIEMAP only, or set it to zero. */ + uint32_t ext_flags; +}; + +/* Structure used to reserve extent scan information per file. */ +struct extent_scan +{ + /* File descriptor of extent scan run against. */ + int fd; + + /* Next scan start offset. */ + off_t scan_start; + + /* How many extent info returned for a scan. */ + uint32_t ei_count; + + /* If true, fall back to a normal copy, either + set by the failure of ioctl(2) for FIEMAP or + lseek(2) with SEEK_DATA. */ + bool initial_scan_failed; + + /* If ture, the total extent scan per file has been finished. */ + bool hit_last_extent; + + /* Extent information. */ + struct extent_info *ext_info; +}; + +void +open_extent_scan (int src_fd, struct extent_scan *scan); + +bool +get_extents_info (struct extent_scan *scan); + +#define free_extents_info(ext_scan) free ((ext_scan)->ext_info) +#define close_extent_scan(ext_scan) /* empty */ + +#endif /* EXTENT_SCAN_H */ -- 2.7.4