diff mbox series

[1/3] btrfs-progs: split btrfs_direct_pio() functions into two

Message ID 86c88bc4e7d0500ae22317546420704e57a0ee88.1683632614.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: write_and_map_eb() cleanup | expand

Commit Message

Qu Wenruo May 9, 2023, 11:48 a.m. UTC
It's not a common practice to use the same io function for both read and
write (we have pread() and pwrite(), not pio()).

Furthermore the original function has the following problems:

- Not returning proper error number
  If we had ioctl/stat errors we just return 0 with errno set.
  Thus caller would treat it as a short read, not a proper error.

- Unnecessary @ret_rw
  This is not that obvious if we have different handling for read and
  write, but if we split them it's super obvious we can reuse @ret.

- No proper copy back for short read

- Unable to constify the @buf pointer for write operation

All those problems would be addressed in this patch.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/device-utils.c | 82 +++++++++++++++++++++++++++++--------------
 common/device-utils.h |  7 ++--
 2 files changed, 60 insertions(+), 29 deletions(-)
diff mbox series

Patch

diff --git a/common/device-utils.c b/common/device-utils.c
index 30b66ca8646f..c911a014dbb3 100644
--- a/common/device-utils.c
+++ b/common/device-utils.c
@@ -529,7 +529,7 @@  int device_get_rotational(const char *file)
 	return (rotational == '0');
 }
 
-ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
+ssize_t btrfs_direct_pread(int fd, void *buf, size_t count, off_t offset)
 {
 	int alignment;
 	size_t iosize;
@@ -537,13 +537,10 @@  ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
 	struct stat stat_buf;
 	unsigned long req;
 	int ret;
-	ssize_t ret_rw;
-
-	UASSERT(rw == READ || rw == WRITE);
 
 	if (fstat(fd, &stat_buf) == -1) {
 		error("fstat failed: %m");
-		return 0;
+		return -errno;
 	}
 
 	if ((stat_buf.st_mode & S_IFMT) == S_IFBLK)
@@ -553,20 +550,61 @@  ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
 
 	if (ioctl(fd, req, &alignment)) {
 		error("failed to get block size: %m");
-		return 0;
+		return -errno;
 	}
 
-	if (IS_ALIGNED((size_t)buf, alignment) && IS_ALIGNED(count, alignment)) {
-		if (rw == WRITE)
-			return pwrite(fd, buf, count, offset);
-		else
-			return pread(fd, buf, count, offset);
+	if (IS_ALIGNED((size_t)buf, alignment) && IS_ALIGNED(count, alignment))
+		return pread(fd, buf, count, offset);
+
+	iosize = round_up(count, alignment);
+
+	ret = posix_memalign(&bounce_buf, alignment, iosize);
+	if (ret) {
+		error_msg(ERROR_MSG_MEMORY, "bounce buffer");
+		errno = ret;
+		return -ret;
 	}
 
+	ret = pread(fd, bounce_buf, iosize, offset);
+	if (ret >= count)
+		ret = count;
+	memcpy(buf, bounce_buf, count);
+
+	free(bounce_buf);
+	return ret;
+}
+
+ssize_t btrfs_direct_pwrite(int fd, const void *buf, size_t count, off_t offset)
+{
+	int alignment;
+	size_t iosize;
+	void *bounce_buf = NULL;
+	struct stat stat_buf;
+	unsigned long req;
+	int ret;
+
+	if (fstat(fd, &stat_buf) == -1) {
+		error("fstat failed: %m");
+		return -errno;
+	}
+
+	if ((stat_buf.st_mode & S_IFMT) == S_IFBLK)
+		req = BLKSSZGET;
+	else
+		req = FIGETBSZ;
+
+	if (ioctl(fd, req, &alignment)) {
+		error("failed to get block size: %m");
+		return -errno;
+	}
+
+	if (IS_ALIGNED((size_t)buf, alignment) && IS_ALIGNED(count, alignment))
+		return pwrite(fd, buf, count, offset);
+
 	/* Cannot do anything if the write size is not aligned */
-	if (rw == WRITE && !IS_ALIGNED(count, alignment)) {
+	if (!IS_ALIGNED(count, alignment)) {
 		error("%zu is not aligned to %d", count, alignment);
-		return 0;
+		return -EINVAL;
 	}
 
 	iosize = round_up(count, alignment);
@@ -575,21 +613,13 @@  ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset)
 	if (ret) {
 		error_msg(ERROR_MSG_MEMORY, "bounce buffer");
 		errno = ret;
-		return 0;
+		return -ret;
 	}
 
-	if (rw == WRITE) {
-		UASSERT(iosize == count);
-		memcpy(bounce_buf, buf, count);
-		ret_rw = pwrite(fd, bounce_buf, iosize, offset);
-	} else {
-		ret_rw = pread(fd, bounce_buf, iosize, offset);
-		if (ret_rw >= count) {
-			ret_rw = count;
-			memcpy(buf, bounce_buf, count);
-		}
-	}
+	UASSERT(iosize == count);
+	memcpy(bounce_buf, buf, count);
+	ret = pwrite(fd, bounce_buf, iosize, offset);
 
 	free(bounce_buf);
-	return ret_rw;
+	return ret;
 }
diff --git a/common/device-utils.h b/common/device-utils.h
index 064d7cd09927..6390a72951eb 100644
--- a/common/device-utils.h
+++ b/common/device-utils.h
@@ -50,7 +50,8 @@  int device_get_rotational(const char *file);
  */
 int btrfs_prepare_device(int fd, const char *file, u64 *block_count_ret,
 		u64 max_block_count, unsigned opflags);
-ssize_t btrfs_direct_pio(int rw, int fd, void *buf, size_t count, off_t offset);
+ssize_t btrfs_direct_pread(int fd, void *buf, size_t count, off_t offset);
+ssize_t btrfs_direct_pwrite(int fd, const void *buf, size_t count, off_t offset);
 
 #ifdef BTRFS_ZONED
 static inline ssize_t btrfs_pwrite(int fd, void *buf, size_t count,
@@ -59,7 +60,7 @@  static inline ssize_t btrfs_pwrite(int fd, void *buf, size_t count,
 	if (!direct)
 		return pwrite(fd, buf, count, offset);
 
-	return btrfs_direct_pio(WRITE, fd, buf, count, offset);
+	return btrfs_direct_pwrite(fd, buf, count, offset);
 }
 static inline ssize_t btrfs_pread(int fd, void *buf, size_t count, off_t offset,
 				  bool direct)
@@ -67,7 +68,7 @@  static inline ssize_t btrfs_pread(int fd, void *buf, size_t count, off_t offset,
 	if (!direct)
 		return pread(fd, buf, count, offset);
 
-	return btrfs_direct_pio(READ, fd, buf, count, offset);
+	return btrfs_direct_pread(fd, buf, count, offset);
 }
 #else
 #define btrfs_pwrite(fd, buf, count, offset, direct) \