| Message ID | 1729762134-380-1-git-send-email-zhiguo.niu@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs-tools: correct some confused desc about unit | expand |
On 2024/10/24 17:28, Zhiguo Niu wrote: > F2FS_BLKSIZE may be 4KB, 16KB, so use F2FS_BLKSIZE to replace > some hardcode desc about unit in some f2fs_io cmd, also > adjust "-c" parameters in mkfs man, to be consistent with > commit c35fa8cd75ac ("mkfs.f2fs: change -c option description"). > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > man/mkfs.f2fs.8 | 2 +- > tools/f2fs_io/f2fs_io.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/man/mkfs.f2fs.8 b/man/mkfs.f2fs.8 > index de885be..8b3b0cc 100644 > --- a/man/mkfs.f2fs.8 > +++ b/man/mkfs.f2fs.8 > @@ -122,7 +122,7 @@ block size matches the page size. > The default value is 4096. > .TP > .BI \-c " device-list" > -Build f2fs with these additional comma separated devices, so that the user can > +Build f2fs with these additional devices, so that the user can > see all the devices as one big volume. > Supports up to 7 devices except meta device. > .TP > diff --git a/tools/f2fs_io/f2fs_io.c b/tools/f2fs_io/f2fs_io.c > index 95f575f..ee4ed0e 100644 > --- a/tools/f2fs_io/f2fs_io.c > +++ b/tools/f2fs_io/f2fs_io.c > @@ -1013,7 +1013,7 @@ static void do_randread(int argc, char **argv, const struct cmd_desc *cmd) > > #define fiemap_desc "get block address in file" > #define fiemap_help \ > -"f2fs_io fiemap [offset in 4kb] [count in 4kb] [file_path]\n\n"\ > +"f2fs_io fiemap [offset in F2FS_BLKSIZE] [count in F2FS_BLKSIZE] [file_path]\n\n"\ > > #if defined(HAVE_LINUX_FIEMAP_H) && defined(HAVE_LINUX_FS_H) > static void do_fiemap(int argc, char **argv, const struct cmd_desc *cmd) > @@ -1617,8 +1617,8 @@ static void do_move_range(int argc, char **argv, const struct cmd_desc *cmd) > #define gc_range_desc "trigger filesystem gc_range" > #define gc_range_help "f2fs_io gc_range [sync_mode] [start] [length] [file_path]\n\n"\ > " sync_mode : 0: asynchronous, 1: synchronous\n" \ > -" start : start offset of defragment region, unit: 4kb\n" \ > -" length : bytes number of defragment region, unit: 4kb\n" \ > +" start : start offset of defragment region, unit: F2FS_BLKSIZE\n" \ > +" length : bytes number of defragment region, unit: F2FS_BLKSIZE\n" \ I think we'd better unify default block size to 4096 since in most of places in f2fs_io.c, we use 4096 as default IO/buffer size. git grep -n "4096" tools/f2fs_io/f2fs_io.c tools/f2fs_io/f2fs_io.c:212: args.block_size = 4096; tools/f2fs_io/f2fs_io.c:662: buf_size = bs * 4096; tools/f2fs_io/f2fs_io.c:666: buf = aligned_xalloc(4096, buf_size); tools/f2fs_io/f2fs_io.c:877: buf_size = bs * 4096; tools/f2fs_io/f2fs_io.c:881: buf = aligned_xalloc(4096, buf_size); tools/f2fs_io/f2fs_io.c:901: if (posix_fadvise(fd, 0, 4096, POSIX_FADV_SEQUENTIAL) != 0) tools/f2fs_io/f2fs_io.c:903: if (posix_fadvise(fd, 0, 4096, POSIX_FADV_WILLNEED) != 0) tools/f2fs_io/f2fs_io.c:979: buf_size = bs * 4096; tools/f2fs_io/f2fs_io.c:981: buf = aligned_xalloc(4096, buf_size); tools/f2fs_io/f2fs_io.c:994: aligned_size = (u64)stbuf.st_size & ~((u64)(4096 - 1)); tools/f2fs_io/f2fs_io.c:997: end_idx = (u64)(aligned_size - buf_size) / (u64)4096 + 1; tools/f2fs_io/f2fs_io.c:1004: ret = pread(fd, buf, buf_size, 4096 * idx); tools/f2fs_io/f2fs_io.c:1222: char *buf = aligned_xalloc(4096, 4096); tools/f2fs_io/f2fs_io.c:1224: while ((ret = xread(src_fd, buf, 4096)) > 0) git grep -n "F2FS_BLKSIZE" tools/f2fs_io/f2fs_io.c tools/f2fs_io/f2fs_io.c:1034: start = (u64)atoi(argv[1]) * F2FS_BLKSIZE; tools/f2fs_io/f2fs_io.c:1035: length = (u64)atoi(argv[2]) * F2FS_BLKSIZE; tools/f2fs_io/f2fs_io.c:1042: start / F2FS_BLKSIZE, length / F2FS_BLKSIZE); We can add a new macro F2FS_DEFAULT_BLKSIZE and use it instead of magic number and F2FS_BLKSIZE, what do you think? Thanks, > > static void do_gc_range(int argc, char **argv, const struct cmd_desc *cmd) > {
Chao Yu <chao@kernel.org> 于2024年10月24日周四 18:49写道： > > On 2024/10/24 17:28, Zhiguo Niu wrote: > > F2FS_BLKSIZE may be 4KB, 16KB, so use F2FS_BLKSIZE to replace > > some hardcode desc about unit in some f2fs_io cmd, also > > adjust "-c" parameters in mkfs man, to be consistent with > > commit c35fa8cd75ac ("mkfs.f2fs: change -c option description"). > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > man/mkfs.f2fs.8 | 2 +- > > tools/f2fs_io/f2fs_io.c | 6 +++--- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/man/mkfs.f2fs.8 b/man/mkfs.f2fs.8 > > index de885be..8b3b0cc 100644 > > --- a/man/mkfs.f2fs.8 > > +++ b/man/mkfs.f2fs.8 > > @@ -122,7 +122,7 @@ block size matches the page size. > > The default value is 4096. > > .TP > > .BI \-c " device-list" > > -Build f2fs with these additional comma separated devices, so that the user can > > +Build f2fs with these additional devices, so that the user can > > see all the devices as one big volume. > > Supports up to 7 devices except meta device. > > .TP > > diff --git a/tools/f2fs_io/f2fs_io.c b/tools/f2fs_io/f2fs_io.c > > index 95f575f..ee4ed0e 100644 > > --- a/tools/f2fs_io/f2fs_io.c > > +++ b/tools/f2fs_io/f2fs_io.c > > @@ -1013,7 +1013,7 @@ static void do_randread(int argc, char **argv, const struct cmd_desc *cmd) > > > > #define fiemap_desc "get block address in file" > > #define fiemap_help \ > > -"f2fs_io fiemap [offset in 4kb] [count in 4kb] [file_path]\n\n"\ > > +"f2fs_io fiemap [offset in F2FS_BLKSIZE] [count in F2FS_BLKSIZE] [file_path]\n\n"\ > > > > #if defined(HAVE_LINUX_FIEMAP_H) && defined(HAVE_LINUX_FS_H) > > static void do_fiemap(int argc, char **argv, const struct cmd_desc *cmd) > > @@ -1617,8 +1617,8 @@ static void do_move_range(int argc, char **argv, const struct cmd_desc *cmd) > > #define gc_range_desc "trigger filesystem gc_range" > > #define gc_range_help "f2fs_io gc_range [sync_mode] [start] [length] [file_path]\n\n"\ > > " sync_mode : 0: asynchronous, 1: synchronous\n" \ > > -" start : start offset of defragment region, unit: 4kb\n" \ > > -" length : bytes number of defragment region, unit: 4kb\n" \ > > +" start : start offset of defragment region, unit: F2FS_BLKSIZE\n" \ > > +" length : bytes number of defragment region, unit: F2FS_BLKSIZE\n" \ > > I think we'd better unify default block size to 4096 since in most of > places in f2fs_io.c, we use 4096 as default IO/buffer size. > > git grep -n "4096" tools/f2fs_io/f2fs_io.c > tools/f2fs_io/f2fs_io.c:212: args.block_size = 4096; > tools/f2fs_io/f2fs_io.c:662: buf_size = bs * 4096; > tools/f2fs_io/f2fs_io.c:666: buf = aligned_xalloc(4096, buf_size); > tools/f2fs_io/f2fs_io.c:877: buf_size = bs * 4096; > tools/f2fs_io/f2fs_io.c:881: buf = aligned_xalloc(4096, buf_size); > tools/f2fs_io/f2fs_io.c:901: if (posix_fadvise(fd, 0, 4096, POSIX_FADV_SEQUENTIAL) != 0) > tools/f2fs_io/f2fs_io.c:903: if (posix_fadvise(fd, 0, 4096, POSIX_FADV_WILLNEED) != 0) > tools/f2fs_io/f2fs_io.c:979: buf_size = bs * 4096; > tools/f2fs_io/f2fs_io.c:981: buf = aligned_xalloc(4096, buf_size); > tools/f2fs_io/f2fs_io.c:994: aligned_size = (u64)stbuf.st_size & ~((u64)(4096 - 1)); > tools/f2fs_io/f2fs_io.c:997: end_idx = (u64)(aligned_size - buf_size) / (u64)4096 + 1; > tools/f2fs_io/f2fs_io.c:1004: ret = pread(fd, buf, buf_size, 4096 * idx); > tools/f2fs_io/f2fs_io.c:1222: char *buf = aligned_xalloc(4096, 4096); > tools/f2fs_io/f2fs_io.c:1224: while ((ret = xread(src_fd, buf, 4096)) > 0) > > git grep -n "F2FS_BLKSIZE" tools/f2fs_io/f2fs_io.c > tools/f2fs_io/f2fs_io.c:1034: start = (u64)atoi(argv[1]) * F2FS_BLKSIZE; > tools/f2fs_io/f2fs_io.c:1035: length = (u64)atoi(argv[2]) * F2FS_BLKSIZE; > tools/f2fs_io/f2fs_io.c:1042: start / F2FS_BLKSIZE, length / F2FS_BLKSIZE); > > We can add a new macro F2FS_DEFAULT_BLKSIZE and use it instead of magic > number and F2FS_BLKSIZE, what do you think? Dear Chao, It is a good suggestions, will update it. now it is a little confused when I use f2fs_io fiemap in 16KB page system. ^^ thanks! > > Thanks, > > > > > static void do_gc_range(int argc, char **argv, const struct cmd_desc *cmd) > > { >
diff --git a/man/mkfs.f2fs.8 b/man/mkfs.f2fs.8 index de885be..8b3b0cc 100644 --- a/man/mkfs.f2fs.8 +++ b/man/mkfs.f2fs.8 @@ -122,7 +122,7 @@ block size matches the page size. The default value is 4096. .TP .BI \-c " device-list" -Build f2fs with these additional comma separated devices, so that the user can +Build f2fs with these additional devices, so that the user can see all the devices as one big volume. Supports up to 7 devices except meta device. .TP diff --git a/tools/f2fs_io/f2fs_io.c b/tools/f2fs_io/f2fs_io.c index 95f575f..ee4ed0e 100644 --- a/tools/f2fs_io/f2fs_io.c +++ b/tools/f2fs_io/f2fs_io.c @@ -1013,7 +1013,7 @@ static void do_randread(int argc, char **argv, const struct cmd_desc *cmd) #define fiemap_desc "get block address in file" #define fiemap_help \ -"f2fs_io fiemap [offset in 4kb] [count in 4kb] [file_path]\n\n"\ +"f2fs_io fiemap [offset in F2FS_BLKSIZE] [count in F2FS_BLKSIZE] [file_path]\n\n"\ #if defined(HAVE_LINUX_FIEMAP_H) && defined(HAVE_LINUX_FS_H) static void do_fiemap(int argc, char **argv, const struct cmd_desc *cmd) @@ -1617,8 +1617,8 @@ static void do_move_range(int argc, char **argv, const struct cmd_desc *cmd) #define gc_range_desc "trigger filesystem gc_range" #define gc_range_help "f2fs_io gc_range [sync_mode] [start] [length] [file_path]\n\n"\ " sync_mode : 0: asynchronous, 1: synchronous\n" \ -" start : start offset of defragment region, unit: 4kb\n" \ -" length : bytes number of defragment region, unit: 4kb\n" \ +" start : start offset of defragment region, unit: F2FS_BLKSIZE\n" \ +" length : bytes number of defragment region, unit: F2FS_BLKSIZE\n" \ static void do_gc_range(int argc, char **argv, const struct cmd_desc *cmd) {
F2FS_BLKSIZE may be 4KB, 16KB, so use F2FS_BLKSIZE to replace some hardcode desc about unit in some f2fs_io cmd, also adjust "-c" parameters in mkfs man, to be consistent with commit c35fa8cd75ac ("mkfs.f2fs: change -c option description"). Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> --- man/mkfs.f2fs.8 | 2 +- tools/f2fs_io/f2fs_io.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)