2013-07-29 09:08:50

by Robert Yang

[permalink] [raw]
Subject: [PATCH 0/2 V3] e2fsprogs/debugfs: do sparse copy when src is a sparse file

=== V3:
* Use BUFSIZ (which is 8192 on Linux systems) for the max argument
length as Ted suggested.

* Use ext2fs_get_memzero() and move it out of the while loop to get a
better performance as Darrick suggested.

=== V2:
* Use 64K for the IO_BUFSIZE and use malloc() to allocate the memory for
the buffer respect to Darrick's comments.

* Use calloc() and memcmp() to check the sparse block as Darrick
suggested.

* Adjust the frame and remove a few un-needed code as Darrick
suggested.

=== V1:
* There are two patches, one is used for fixing the max length of the
argument, the other one is for sparsing copy when src is a sparse
file.

* BTW., we are trying to use the debugfs to create an ext2/ext3/ext4
image based on a given directory or file, which is similar to genext2fs,
please vist here for the script you are interested in it:

https://gist.github.com/robertlinux/6034499

It is an external shell script at the moment, the performance is not good,
so we are going to:
- Integrate this function into mke2fs in C language.
- Add a [-d <directory>] option to specify the initial directory in mke2fs

I'm not sure whether such a patch is acceptable or not, please feel free to
give your comments.

Robert Yang
Wind River System

Robert Yang (2):
debugfs.c: the max length of debugfs argument is too short
debugfs.c: do sparse copy when src is a sparse file

debugfs/debugfs.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 62 insertions(+), 5 deletions(-)

--
1.8.1.2



2013-07-29 09:08:53

by Robert Yang

[permalink] [raw]
Subject: [PATCH 1/2] debugfs.c: the max length of debugfs argument is too short

The max length of debugfs argument is 256 which is too short, the
arguments are two paths, the PATH_MAX is 4096 according to
/usr/include/linux/limits.h, so use BUFSIZ (which is 8192 on Linux
systems), that's also what the ss library uses.

Signed-off-by: Robert Yang <[email protected]>
Acked-by: Darren Hart <[email protected]>
---
debugfs/debugfs.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 2660218..a6bc932 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -37,6 +37,10 @@ extern char *optarg;
#include "../version.h"
#include "jfs_user.h"

+#ifndef BUFSIZ
+#define BUFSIZ 8192
+#endif
+
ss_request_table *extra_cmds;
const char *debug_prog_name;
int sci_idx;
@@ -2311,7 +2315,7 @@ void do_dump_mmp(int argc EXT2FS_ATTR((unused)), char *argv[])
static int source_file(const char *cmd_file, int ss_idx)
{
FILE *f;
- char buf[256];
+ char buf[BUFSIZ];
char *cp;
int exit_status = 0;
int retval;
--
1.8.1.2


2013-07-29 09:08:55

by Robert Yang

[permalink] [raw]
Subject: [PATCH 2/2] 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.

Signed-off-by: Robert Yang <[email protected]>
Acked-by: Darren Hart <[email protected]>
---
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",
"<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


2013-08-06 01:46:42

by Robert Yang

[permalink] [raw]
Subject: Re: [PATCH 0/2 V3] e2fsprogs/debugfs: do sparse copy when src is a sparse file


Any comments, please ?

// Robert

On 07/29/2013 05:06 PM, Robert Yang wrote:
> === V3:
> * Use BUFSIZ (which is 8192 on Linux systems) for the max argument
> length as Ted suggested.
>
> * Use ext2fs_get_memzero() and move it out of the while loop to get a
> better performance as Darrick suggested.
>
> === V2:
> * Use 64K for the IO_BUFSIZE and use malloc() to allocate the memory for
> the buffer respect to Darrick's comments.
>
> * Use calloc() and memcmp() to check the sparse block as Darrick
> suggested.
>
> * Adjust the frame and remove a few un-needed code as Darrick
> suggested.
>
> === V1:
> * There are two patches, one is used for fixing the max length of the
> argument, the other one is for sparsing copy when src is a sparse
> file.
>
> * BTW., we are trying to use the debugfs to create an ext2/ext3/ext4
> image based on a given directory or file, which is similar to genext2fs,
> please vist here for the script you are interested in it:
>
> https://gist.github.com/robertlinux/6034499
>
> It is an external shell script at the moment, the performance is not good,
> so we are going to:
> - Integrate this function into mke2fs in C language.
> - Add a [-d <directory>] option to specify the initial directory in mke2fs
>
> I'm not sure whether such a patch is acceptable or not, please feel free to
> give your comments.
>
> Robert Yang
> Wind River System
>
> Robert Yang (2):
> debugfs.c: the max length of debugfs argument is too short
> debugfs.c: do sparse copy when src is a sparse file
>
> debugfs/debugfs.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 62 insertions(+), 5 deletions(-)
>

2013-08-19 22:43:09

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH 2/2] debugfs.c: do sparse copy when src is a sparse file

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 <[email protected]>
> Acked-by: Darren Hart <[email protected]>
> ---
> 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",
> "<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);
> }

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