From: Darren Hart Subject: Re: [PATCH 2/2] debugfs.c: do sparse copy when src is a sparse file Date: Mon, 19 Aug 2013 15:43:07 -0700 Message-ID: <1376952187.12131.105.camel@dvhart-mobl4.amr.corp.intel.com> References: <1375088785-30653-1-git-send-email-liezhi.yang@windriver.com> <1375088785-30653-3-git-send-email-liezhi.yang@windriver.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: "tom.zanussi@intel.com" , Robert Yang To: linux-ext4@vger.kernel.org, tytso@mit.edu, darrick.wong@oracle.com Return-path: Received: from mga02.intel.com ([134.134.136.20]:34259 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751575Ab3HSWnJ (ORCPT ); Mon, 19 Aug 2013 18:43:09 -0400 In-Reply-To: <1375088785-30653-3-git-send-email-liezhi.yang@windriver.com> Sender: linux-ext4-owner@vger.kernel.org List-ID: Ted, Darrick, Any more thoughts on this series from Robert? We'd like to pull this in to the Yocto Project, but are trying hard not to include patches that are not at least acked by the upstream maintainers. I believe Robert as addressed the concerns raised? Thanks! Darren On Mon, 2013-07-29 at 17:06 +0800, Robert Yang wrote: > Let debugfs do sparse copy when src is a sparse file, just like > "cp --sparse=auto" > > * For the: > #define IO_BUFSIZE 64*1024 > this is a suggested value from gnu coreutils: > http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=blob;f=src/ioblksize.h;h=1ae93255e7d0ccf0855208c7ae5888209997bf16;hb=HEAD > > * Use malloc() to allocate memory for the buffer since put 64K (or > more) on the stack seems not a good idea. > > Signed-off-by: Robert Yang > Acked-by: Darren Hart > --- > debugfs/debugfs.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 57 insertions(+), 4 deletions(-) > > diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c > index a6bc932..d35fecc 100644 > --- a/debugfs/debugfs.c > +++ b/debugfs/debugfs.c > @@ -41,6 +41,16 @@ extern char *optarg; > #define BUFSIZ 8192 > #endif > > +/* 64KiB is the minimium blksize to best minimize system call overhead. */ > +#ifndef IO_BUFSIZE > +#define IO_BUFSIZE 64*1024 > +#endif > + > +/* Block size for `st_blocks' */ > +#ifndef S_BLKSIZE > +#define S_BLKSIZE 512 > +#endif > + > ss_request_table *extra_cmds; > const char *debug_prog_name; > int sci_idx; > @@ -1575,22 +1585,36 @@ void do_find_free_inode(int argc, char *argv[]) > } > > #ifndef READ_ONLY > -static errcode_t copy_file(int fd, ext2_ino_t newfile) > +static errcode_t copy_file(int fd, ext2_ino_t newfile, int bufsize, int make_holes) > { > ext2_file_t e2_file; > errcode_t retval; > int got; > unsigned int written; > - char buf[8192]; > + char *buf; > char *ptr; > + char *zero_buf; > + int cmp; > > retval = ext2fs_file_open(current_fs, newfile, > EXT2_FILE_WRITE, &e2_file); > if (retval) > return retval; > > + if (!(buf = (char *) malloc(bufsize))){ > + com_err("copy_file", errno, "can't allocate buffer\n"); > + return; > + } > + > + /* This is used for checking whether the whole block is zero */ > + retval = ext2fs_get_memzero(bufsize, &zero_buf); > + if (retval) { > + com_err("copy_file", retval, "can't allocate buffer\n"); > + return retval; > + } > + > while (1) { > - got = read(fd, buf, sizeof(buf)); > + got = read(fd, buf, bufsize); > if (got == 0) > break; > if (got < 0) { > @@ -1598,6 +1622,21 @@ static errcode_t copy_file(int fd, ext2_ino_t newfile) > goto fail; > } > ptr = buf; > + > + /* Sparse copy */ > + if (make_holes) { > + /* Check whether all is zero */ > + cmp = memcmp(ptr, zero_buf, got); > + if (cmp == 0) { > + /* The whole block is zero, make a hole */ > + retval = ext2fs_file_lseek(e2_file, got, EXT2_SEEK_CUR, NULL); > + if (retval) > + goto fail; > + got = 0; > + } > + } > + > + /* Normal copy */ > while (got > 0) { > retval = ext2fs_file_write(e2_file, ptr, > got, &written); > @@ -1608,10 +1647,14 @@ static errcode_t copy_file(int fd, ext2_ino_t newfile) > ptr += written; > } > } > + free(buf); > + ext2fs_free_mem(&zero_buf); > retval = ext2fs_file_close(e2_file); > return retval; > > fail: > + free(buf); > + ext2fs_free_mem(&zero_buf); > (void) ext2fs_file_close(e2_file); > return retval; > } > @@ -1624,6 +1667,8 @@ void do_write(int argc, char *argv[]) > ext2_ino_t newfile; > errcode_t retval; > struct ext2_inode inode; > + int bufsize = IO_BUFSIZE; > + int make_holes = 0; > > if (common_args_process(argc, argv, 3, 3, "write", > " ", CHECK_FS_RW)) > @@ -1699,7 +1744,15 @@ void do_write(int argc, char *argv[]) > return; > } > if (LINUX_S_ISREG(inode.i_mode)) { > - retval = copy_file(fd, newfile); > + if (statbuf.st_blocks < statbuf.st_size / S_BLKSIZE) { > + make_holes = 1; > + /* > + * Use I/O blocksize as buffer size when > + * copying sparse files. > + */ > + bufsize = statbuf.st_blksize; > + } > + retval = copy_file(fd, newfile, bufsize, make_holes); > if (retval) > com_err("copy_file", retval, 0); > } -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel