=== 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 | 60 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 6 deletions(-)
--
1.8.1.2
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 | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 53 insertions(+), 5 deletions(-)
diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index b77d0b5..0379f38 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -37,6 +37,16 @@ extern char *optarg;
#include "../version.h"
#include "jfs_user.h"
+/* 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;
@@ -1571,22 +1581,28 @@ 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 *zeromem;
+ int cmp;
retval = ext2fs_file_open(current_fs, newfile,
EXT2_FILE_WRITE, &e2_file);
if (retval)
return retval;
+ if (!(buf = (char *) malloc(bufsize))){
+ fprintf(stderr, "copy_file: can't allocate buffer\n");
+ return;
+ }
while (1) {
- got = read(fd, buf, sizeof(buf));
+ got = read(fd, buf, bufsize);
if (got == 0)
break;
if (got < 0) {
@@ -1594,20 +1610,42 @@ static errcode_t copy_file(int fd, ext2_ino_t newfile)
goto fail;
}
ptr = buf;
+
+ /* Sparse copy */
+ if (make_holes) {
+ if (!(zeromem = (char *) calloc(got, 1))) {
+ fprintf(stderr, "copy_file: can't allocate buffer\n");
+ return;
+ }
+ /* Check whether all is zero */
+ cmp = memcmp(ptr, zeromem, got);
+ /* Free it as early as possible */
+ free(zeromem);
+ 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);
if (retval)
goto fail;
-
got -= written;
ptr += written;
}
}
+ free(buf);
retval = ext2fs_file_close(e2_file);
return retval;
fail:
+ free(buf);
(void) ext2fs_file_close(e2_file);
return retval;
}
@@ -1620,6 +1658,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))
@@ -1684,7 +1724,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
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, use 2048 (4096 / 2) is a reasonable value.
Signed-off-by: Robert Yang <[email protected]>
Acked-by: Darren Hart <[email protected]>
---
debugfs/debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index dcf16e2..b77d0b5 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -2300,7 +2300,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[2048];
char *cp;
int exit_status = 0;
int retval;
--
1.8.1.2
On Fri, Jul 26, 2013 at 06:30:57PM +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 | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
> index b77d0b5..0379f38 100644
> --- a/debugfs/debugfs.c
> +++ b/debugfs/debugfs.c
> @@ -37,6 +37,16 @@ extern char *optarg;
> #include "../version.h"
> #include "jfs_user.h"
>
> +/* 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;
> @@ -1571,22 +1581,28 @@ 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 *zeromem;
> + int cmp;
>
> retval = ext2fs_file_open(current_fs, newfile,
> EXT2_FILE_WRITE, &e2_file);
> if (retval)
> return retval;
>
> + if (!(buf = (char *) malloc(bufsize))){
> + fprintf(stderr, "copy_file: can't allocate buffer\n");
> + return;
> + }
> while (1) {
> - got = read(fd, buf, sizeof(buf));
> + got = read(fd, buf, bufsize);
> if (got == 0)
> break;
> if (got < 0) {
> @@ -1594,20 +1610,42 @@ static errcode_t copy_file(int fd, ext2_ino_t newfile)
> goto fail;
> }
> ptr = buf;
> +
> + /* Sparse copy */
> + if (make_holes) {
> + if (!(zeromem = (char *) calloc(got, 1))) {
Maybe ext2fs_get_memzero()? And I suspect you can allocate a bufsize-sized
zeromem outside the while loop. Chances are pretty good that if make_holes,
then buf_size won't be greater than 4096 anyway.
--D
> + fprintf(stderr, "copy_file: can't allocate buffer\n");
> + return;
> + }
> + /* Check whether all is zero */
> + cmp = memcmp(ptr, zeromem, got);
> + /* Free it as early as possible */
> + free(zeromem);
> + 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);
> if (retval)
> goto fail;
> -
> got -= written;
> ptr += written;
> }
> }
> + free(buf);
> retval = ext2fs_file_close(e2_file);
> return retval;
>
> fail:
> + free(buf);
> (void) ext2fs_file_close(e2_file);
> return retval;
> }
> @@ -1620,6 +1658,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))
> @@ -1684,7 +1724,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 26, 2013 at 06:30:56PM +0800, Robert Yang wrote:
> 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, use 2048 (4096 / 2) is a reasonable value.
I'd just use BUFSIZ (which is 8192 on Linux systems). That's what the
ss library uses, and if you might have two paths and PATH_MAX is 4096,
2048 could easily be too small.
- Ted
On 07/27/2013 12:02 AM, Darrick J. Wong wrote:
> On Fri, Jul 26, 2013 at 06:30:57PM +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 | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 53 insertions(+), 5 deletions(-)
>>
>> diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
>> index b77d0b5..0379f38 100644
>> --- a/debugfs/debugfs.c
>> +++ b/debugfs/debugfs.c
>> @@ -37,6 +37,16 @@ extern char *optarg;
>> #include "../version.h"
>> #include "jfs_user.h"
>>
>> +/* 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;
>> @@ -1571,22 +1581,28 @@ 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 *zeromem;
>> + int cmp;
>>
>> retval = ext2fs_file_open(current_fs, newfile,
>> EXT2_FILE_WRITE, &e2_file);
>> if (retval)
>> return retval;
>>
>> + if (!(buf = (char *) malloc(bufsize))){
>> + fprintf(stderr, "copy_file: can't allocate buffer\n");
>> + return;
>> + }
>> while (1) {
>> - got = read(fd, buf, sizeof(buf));
>> + got = read(fd, buf, bufsize);
>> if (got == 0)
>> break;
>> if (got < 0) {
>> @@ -1594,20 +1610,42 @@ static errcode_t copy_file(int fd, ext2_ino_t newfile)
>> goto fail;
>> }
>> ptr = buf;
>> +
>> + /* Sparse copy */
>> + if (make_holes) {
>> + if (!(zeromem = (char *) calloc(got, 1))) {
>
> Maybe ext2fs_get_memzero()? And I suspect you can allocate a bufsize-sized
> zeromem outside the while loop. Chances are pretty good that if make_holes,
> then buf_size won't be greater than 4096 anyway.
Thanks, sounds great, my testing shows that it is faster for copying the
large sparse file after move the ext2fs_get_memzero() out of the while loop,
and didn't have any obvious impaction for non-sparse file, I will update it
and send a V3.
// Robert
>
> --D
>
>> + fprintf(stderr, "copy_file: can't allocate buffer\n");
>> + return;
>> + }
>> + /* Check whether all is zero */
>> + cmp = memcmp(ptr, zeromem, got);
>> + /* Free it as early as possible */
>> + free(zeromem);
>> + 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);
>> if (retval)
>> goto fail;
>> -
>> got -= written;
>> ptr += written;
>> }
>> }
>> + free(buf);
>> retval = ext2fs_file_close(e2_file);
>> return retval;
>>
>> fail:
>> + free(buf);
>> (void) ext2fs_file_close(e2_file);
>> return retval;
>> }
>> @@ -1620,6 +1658,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))
>> @@ -1684,7 +1724,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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
Hi Ted,
Thanks, I will update it and send a V3.
// Robert
On 07/29/2013 10:37 AM, Theodore Ts'o wrote:
> On Fri, Jul 26, 2013 at 06:30:56PM +0800, Robert Yang wrote:
>> 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, use 2048 (4096 / 2) is a reasonable value.
>
> I'd just use BUFSIZ (which is 8192 on Linux systems). That's what the
> ss library uses, and if you might have two paths and PATH_MAX is 4096,
> 2048 could easily be too small.
>
> - Ted
>
>