diff mbox series

[f2fs-dev,1/2] f2fs-tools: use pread and pwrite when they are available.

Message ID 20241210082801.4104194-1-weilongping@oppo.com (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev,1/2] f2fs-tools: use pread and pwrite when they are available. | expand

Commit Message

LongPing Wei Dec. 10, 2024, 8:28 a.m. UTC 
This patch want to reduce the number of system calls to improve performance.
pread and pwrite will only be used when the target platform support them.

Signed-off-by: LongPing Wei <weilongping@oppo.com>
---
 configure.ac     |  4 +++-
 lib/libf2fs_io.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

Comments

Sheng Yong Dec. 10, 2024, 1:43 p.m. UTC | #1 
On 2024/12/10 16:28, LongPing Wei wrote:
> This patch want to reduce the number of system calls to improve performance.
> pread and pwrite will only be used when the target platform support them.
> 
> Signed-off-by: LongPing Wei <weilongping@oppo.com>
> ---
>   configure.ac     |  4 +++-
>   lib/libf2fs_io.c | 29 +++++++++++++++++++++++++++++
>   2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 2053a65..439b97f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -182,6 +182,7 @@ AC_TYPE_SIZE_T
>   AC_FUNC_GETMNTENT
>   AC_CHECK_FUNCS_ONCE([
>   	add_key
> +	clock_gettime
>   	fallocate
>   	fsetxattr
>   	fstat
> @@ -193,7 +194,8 @@ AC_CHECK_FUNCS_ONCE([
>   	keyctl
>   	memset
>   	setmntent
> -	clock_gettime
> +	pread
> +	pwrite
>   ])
>   
>   AS_IF([test "$ac_cv_header_byteswap_h" = "yes"],
> diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
> index 520ae03..a8430c9 100644
> --- a/lib/libf2fs_io.c
> +++ b/lib/libf2fs_io.c
> @@ -279,6 +279,12 @@ static int dcache_io_read(long entry, __u64 offset, off_t blk)
>   	if (fd < 0)
>   		return fd;
>   
> +#ifdef HAVE_PREAD
> +	if (pread(fd, dcache_buf + entry * F2FS_BLKSIZE, F2FS_BLKSIZE, offset) < 0) {
> +		MSG(0, "\n pread() fail.\n");
> +		return -1;
> +	}
> +#else
>   	if (lseek(fd, offset, SEEK_SET) < 0) {
>   		MSG(0, "\n lseek fail.\n");
>   		return -1;
> @@ -287,6 +293,7 @@ static int dcache_io_read(long entry, __u64 offset, off_t blk)
>   		MSG(0, "\n read() fail.\n");
>   		return -1;
>   	}
> +#endif
>   	dcache_lastused[entry] = ++dcache_usetick;
>   	dcache_valid[entry] = true;
>   	dcache_blk[entry] = blk;
> @@ -393,10 +400,15 @@ int dev_read_version(void *buf, __u64 offset, size_t len)
>   {
>   	if (c.sparse_mode)
>   		return 0;
> +#ifdef HAVE_RPEAD
> +	if (pread(c.kd, buf, len, (off_t)offset) < 0)
> +		return -1;
> +#else
>   	if (lseek(c.kd, (off_t)offset, SEEK_SET) < 0)
>   		return -1;
>   	if (read(c.kd, buf, len) < 0)
>   		return -1;
> +#endif
>   	return 0;
>   }
>   
> @@ -535,10 +547,15 @@ int dev_read(void *buf, __u64 offset, size_t len)
>   	fd = __get_device_fd(&offset);
>   	if (fd < 0)
>   		return fd;
> +#ifdef HAVE_PREAD
> +	if (pread(fd, buf, len, (off_t)offset) < 0)
> +		return -1;
> +#else
>   	if (lseek(fd, (off_t)offset, SEEK_SET) < 0)
>   		return -1;
>   	if (read(fd, buf, len) < 0)
>   		return -1;
> +#endif
>   	return 0;
>   }
>   
> @@ -615,8 +632,10 @@ static int __dev_write(void *buf, __u64 offset, size_t len, enum rw_hint whint)
>   	if (fd < 0)
>   		return fd;
>   
> +#ifndef HAVE_PWRITE
>   	if (lseek(fd, (off_t)offset, SEEK_SET) < 0)
>   		return -1;
> +#endif
Hi, LongPing,

Can we move lseek and the following write together after writehint
operation? I think there is no need to place writehint between lseek
and write, because writehint affects the inode and does not affects
the pos in struct file.

However, I am wondering if writehint here works as expected when fd
is not opened with O_SYNC or O_DSYN, especially if multiple writes
occur in a short period of time. Please correct me if I miss something.

thanks,
shengyong
>   
>   #if ! defined(__MINGW32__)
>   	if (c.need_whint && (c.whint != whint)) {
> @@ -629,8 +648,13 @@ static int __dev_write(void *buf, __u64 offset, size_t len, enum rw_hint whint)
>   	}
>   #endif
>   
> +#ifdef HAVE_PWRITE
> +	if (pwrite(fd, buf, len, (off_t)offset) < 0)
> +		return -1;
> +#else
>   	if (write(fd, buf, len) < 0)
>   		return -1;
> +#endif
>   
>   	c.need_fsync = true;
>   
> @@ -663,10 +687,15 @@ int dev_write_block(void *buf, __u64 blk_addr, enum rw_hint whint)
>   
>   int dev_write_dump(void *buf, __u64 offset, size_t len)
>   {
> +#ifdef HAVE_PWRITE
> +	if (pwrite(c.dump_fd, buf, len, (off_t)offset) < 0)
> +		return -1;
> +#else
>   	if (lseek(c.dump_fd, (off_t)offset, SEEK_SET) < 0)
>   		return -1;
>   	if (write(c.dump_fd, buf, len) < 0)
>   		return -1;
> +#endif
>   	return 0;
>   }
>
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 2053a65..439b97f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,6 +182,7 @@  AC_TYPE_SIZE_T
 AC_FUNC_GETMNTENT
 AC_CHECK_FUNCS_ONCE([
 	add_key
+	clock_gettime
 	fallocate
 	fsetxattr
 	fstat
@@ -193,7 +194,8 @@  AC_CHECK_FUNCS_ONCE([
 	keyctl
 	memset
 	setmntent
-	clock_gettime
+	pread
+	pwrite
 ])
 
 AS_IF([test "$ac_cv_header_byteswap_h" = "yes"],
diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
index 520ae03..a8430c9 100644
--- a/lib/libf2fs_io.c
+++ b/lib/libf2fs_io.c
@@ -279,6 +279,12 @@  static int dcache_io_read(long entry, __u64 offset, off_t blk)
 	if (fd < 0)
 		return fd;
 
+#ifdef HAVE_PREAD
+	if (pread(fd, dcache_buf + entry * F2FS_BLKSIZE, F2FS_BLKSIZE, offset) < 0) {
+		MSG(0, "\n pread() fail.\n");
+		return -1;
+	}
+#else
 	if (lseek(fd, offset, SEEK_SET) < 0) {
 		MSG(0, "\n lseek fail.\n");
 		return -1;
@@ -287,6 +293,7 @@  static int dcache_io_read(long entry, __u64 offset, off_t blk)
 		MSG(0, "\n read() fail.\n");
 		return -1;
 	}
+#endif
 	dcache_lastused[entry] = ++dcache_usetick;
 	dcache_valid[entry] = true;
 	dcache_blk[entry] = blk;
@@ -393,10 +400,15 @@  int dev_read_version(void *buf, __u64 offset, size_t len)
 {
 	if (c.sparse_mode)
 		return 0;
+#ifdef HAVE_RPEAD
+	if (pread(c.kd, buf, len, (off_t)offset) < 0)
+		return -1;
+#else
 	if (lseek(c.kd, (off_t)offset, SEEK_SET) < 0)
 		return -1;
 	if (read(c.kd, buf, len) < 0)
 		return -1;
+#endif
 	return 0;
 }
 
@@ -535,10 +547,15 @@  int dev_read(void *buf, __u64 offset, size_t len)
 	fd = __get_device_fd(&offset);
 	if (fd < 0)
 		return fd;
+#ifdef HAVE_PREAD
+	if (pread(fd, buf, len, (off_t)offset) < 0)
+		return -1;
+#else
 	if (lseek(fd, (off_t)offset, SEEK_SET) < 0)
 		return -1;
 	if (read(fd, buf, len) < 0)
 		return -1;
+#endif
 	return 0;
 }
 
@@ -615,8 +632,10 @@  static int __dev_write(void *buf, __u64 offset, size_t len, enum rw_hint whint)
 	if (fd < 0)
 		return fd;
 
+#ifndef HAVE_PWRITE
 	if (lseek(fd, (off_t)offset, SEEK_SET) < 0)
 		return -1;
+#endif
 
 #if ! defined(__MINGW32__)
 	if (c.need_whint && (c.whint != whint)) {
@@ -629,8 +648,13 @@  static int __dev_write(void *buf, __u64 offset, size_t len, enum rw_hint whint)
 	}
 #endif
 
+#ifdef HAVE_PWRITE
+	if (pwrite(fd, buf, len, (off_t)offset) < 0)
+		return -1;
+#else
 	if (write(fd, buf, len) < 0)
 		return -1;
+#endif
 
 	c.need_fsync = true;
 
@@ -663,10 +687,15 @@  int dev_write_block(void *buf, __u64 blk_addr, enum rw_hint whint)
 
 int dev_write_dump(void *buf, __u64 offset, size_t len)
 {
+#ifdef HAVE_PWRITE
+	if (pwrite(c.dump_fd, buf, len, (off_t)offset) < 0)
+		return -1;
+#else
 	if (lseek(c.dump_fd, (off_t)offset, SEEK_SET) < 0)
 		return -1;
 	if (write(c.dump_fd, buf, len) < 0)
 		return -1;
+#endif
 	return 0;
 }