[OE-core] [PATCH 2/6] e2fsprogs: let debugfs do sparse copy

Darren Hart dvhart at linux.intel.com
Fri Aug 23 17:06:24 UTC 2013


On Fri, 2013-08-23 at 14:45 +0800, Robert Yang wrote:
> 
> On 08/23/2013 10:33 AM, Rongqing Li wrote:
> >
> >
> > On 08/22/2013 09:13 PM, Robert Yang wrote:
> >> Let debugfs do sparse copy when src is a sparse file, just like
> >> "cp --sparse=auto"
> >>
> >> This patch has been reviewed by the linux-ext4 mailing list, but isn't
> >> merged atm.
> >>
> >> [YOCTO #3848]
> >>
> >> Signed-off-by: Robert Yang <liezhi.yang at windriver.com>
> >> ---
> >>   .../e2fsprogs-1.42.8/debugfs-sparse-copy.patch     |  147 ++++++++++++++++++++
> >>   .../recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb |    1 +
> >>   2 files changed, 148 insertions(+)
> >>   create mode 100644
> >> meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-sparse-copy.patch
> >>
> >> diff --git
> >> a/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-sparse-copy.patch
> >> b/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-sparse-copy.patch
> >> new file mode 100644
> >> index 0000000..c87bcbf
> >> --- /dev/null
> >> +++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-sparse-copy.patch
> >> @@ -0,0 +1,147 @@
> >> +debugfs.c: do sparse copy when src is a sparse file
> >> +
> >> +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.
> >> +
> >> +Upstream-Status: Submitted
> >> +
> >> +Signed-off-by: Robert Yang <liezhi.yang at windriver.com>
> >> +Acked-by: Darren Hart <dvhart at linux.intel.com>
> >> +---
> >> + debugfs/debugfs.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> >> + 1 file changed, 57 insertions(+), 4 deletions(-)
> >> +
> >> +diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
> >> +--- 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");
> >
> > should free(buf), or memory leak.
> >
> 
> Add a free(buf) and updated the PR:
> 
> git://git.pokylinux.org/poky-contrib robert/extX


Nice catch Rongqing, thank you.

Acked-by: Darren Hart <dvhart at linux.intel.com>

Robert, please resubmit a new version to the ext list with the memory
leak and whitespace fixes. It's a good way to the attention back on the
series.

--
Darren

> 
> // Robert
> 
> > -Roy
> >
> >> ++        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",
> >> +                 "<native file> <new file>", 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);
> >> +     }
> >> +--
> >> +1.8.1.2
> >> +
> >> diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
> >> b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
> >> index f97de7f..5d65bbc 100644
> >> --- a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
> >> +++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
> >> @@ -4,6 +4,7 @@ require e2fsprogs.inc
> >>   SRC_URI += "file://acinclude.m4 \
> >>               file://remove.ldconfig.call.patch \
> >>               file://debugfs-too-short.patch \
> >> +            file://debugfs-sparse-copy.patch \
> >>   "
> >>
> >>   SRC_URI[md5sum] = "8ef664b6eb698aa6b733df59b17b9ed4"
> >>
> >

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel





More information about the Openembedded-core mailing list