[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a224e560-2937-4edd-93d8-8077de6054b1@kernel.org>
Date: Thu, 24 Oct 2024 18:48:55 +0800
From: Chao Yu <chao@...nel.org>
To: Zhiguo Niu <zhiguo.niu@...soc.com>, jaegeuk@...nel.org
Cc: Chao Yu <chao@...nel.org>, linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, niuzhiguo84@...il.com, ke.wang@...soc.com,
Hao_hao.Wang@...soc.com
Subject: Re: [PATCH] f2fs-tools: correct some confused desc about unit
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@...soc.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)
> {
Powered by blists - more mailing lists