public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup
@ 2023-05-09 11:48 Qu Wenruo
  2023-05-09 11:48 ` [PATCH 1/3] btrfs-progs: split btrfs_direct_pio() functions into two Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Qu Wenruo @ 2023-05-09 11:48 UTC (permalink / raw)
  To: linux-btrfs

During my convert fixes, I found a lot of locations allocating a dummy
extent buffer with a size which is not nodesize.

Then just read data into that dummy eb, and later call
write_and_map_eb() to write it back.

This behavior is a historic workaround, at a time where we only do
proper RAID56 writeback for metadata, but not data.

But now we have all raid56 handling done properly, and has proper
function to read/write any logical bytenr, read_data_from_disk() and
write_data_to_disk().

Thus there is no longer any need to use write_and_map_eb() as a
workaround.

This patchset would completely remove write_and_map_eb(), most call
sites are just abuse, only 3 call sites are valid but can easily be
converted to call write_data_to_disk().

Qu Wenruo (3):
  btrfs-progs: split btrfs_direct_pio() functions into two
  btrfs-progs: constify the buffer pointer for write functions
  btrfs-progs: remove write_and_map_eb()

 btrfs-corrupt-block.c     | 48 ++++++++++++++---------
 common/device-utils.c     | 82 ++++++++++++++++++++++++++-------------
 common/device-utils.h     | 13 ++++---
 convert/main.c            | 19 ++++-----
 convert/source-reiserfs.c | 10 +----
 kernel-shared/disk-io.c   | 23 +----------
 kernel-shared/disk-io.h   |  1 -
 kernel-shared/extent_io.c |  2 +-
 kernel-shared/extent_io.h |  2 +-
 mkfs/rootdir.c            | 41 +++++++-------------
 10 files changed, 118 insertions(+), 123 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] btrfs-progs: split btrfs_direct_pio() functions into two
  2023-05-09 11:48 [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup Qu Wenruo
@ 2023-05-09 11:48 ` Qu Wenruo
  2023-05-09 11:48 ` [PATCH 2/3] btrfs-progs: constify the buffer pointer for write functions Qu Wenruo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2023-05-09 11:48 UTC (permalink / raw)
  To: linux-btrfs

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 --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) \
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] btrfs-progs: constify the buffer pointer for write functions
  2023-05-09 11:48 [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup Qu Wenruo
  2023-05-09 11:48 ` [PATCH 1/3] btrfs-progs: split btrfs_direct_pio() functions into two Qu Wenruo
@ 2023-05-09 11:48 ` Qu Wenruo
  2023-05-09 11:48 ` [PATCH 3/3] btrfs-progs: remove write_and_map_eb() Qu Wenruo
  2023-05-10 16:25 ` [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup David Sterba
  3 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2023-05-09 11:48 UTC (permalink / raw)
  To: linux-btrfs

The following functions accept a buffer for write, which can be marked
as const:

- btrfs_pwrite()
- write_data_to_disk()

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/device-utils.h     | 6 +++---
 kernel-shared/extent_io.c | 2 +-
 kernel-shared/extent_io.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/common/device-utils.h b/common/device-utils.h
index 6390a72951eb..21955c2835f2 100644
--- a/common/device-utils.h
+++ b/common/device-utils.h
@@ -54,7 +54,7 @@ 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,
+static inline ssize_t btrfs_pwrite(int fd, const void *buf, size_t count,
 				   off_t offset, bool direct)
 {
 	if (!direct)
@@ -62,8 +62,8 @@ static inline ssize_t btrfs_pwrite(int fd, void *buf, size_t count,
 
 	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)
+static inline ssize_t btrfs_pread(int fd, void *buf, size_t count,
+				  off_t offset, bool direct)
 {
 	if (!direct)
 		return pread(fd, buf, count, offset);
diff --git a/kernel-shared/extent_io.c b/kernel-shared/extent_io.c
index 172ba66a1cfa..7f6d592ab641 100644
--- a/kernel-shared/extent_io.c
+++ b/kernel-shared/extent_io.c
@@ -477,7 +477,7 @@ int read_data_from_disk(struct btrfs_fs_info *info, void *buf, u64 logical,
  * Such data will be written to all mirrors and RAID56 P/Q will also be
  * properly handled.
  */
-int write_data_to_disk(struct btrfs_fs_info *info, void *buf, u64 offset,
+int write_data_to_disk(struct btrfs_fs_info *info, const void *buf, u64 offset,
 		       u64 bytes)
 {
 	struct btrfs_multi_bio *multi = NULL;
diff --git a/kernel-shared/extent_io.h b/kernel-shared/extent_io.h
index a1cda3a53351..b6c30eef67c2 100644
--- a/kernel-shared/extent_io.h
+++ b/kernel-shared/extent_io.h
@@ -128,7 +128,7 @@ int set_extent_buffer_dirty(struct extent_buffer *eb);
 int btrfs_clear_buffer_dirty(struct extent_buffer *eb);
 int read_data_from_disk(struct btrfs_fs_info *info, void *buf, u64 logical,
 			u64 *len, int mirror);
-int write_data_to_disk(struct btrfs_fs_info *info, void *buf, u64 offset,
+int write_data_to_disk(struct btrfs_fs_info *info, const void *buf, u64 offset,
 		       u64 bytes);
 void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long start,
                                 unsigned long pos, unsigned long len);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] btrfs-progs: remove write_and_map_eb()
  2023-05-09 11:48 [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup Qu Wenruo
  2023-05-09 11:48 ` [PATCH 1/3] btrfs-progs: split btrfs_direct_pio() functions into two Qu Wenruo
  2023-05-09 11:48 ` [PATCH 2/3] btrfs-progs: constify the buffer pointer for write functions Qu Wenruo
@ 2023-05-09 11:48 ` Qu Wenruo
  2023-05-10 16:25 ` [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup David Sterba
  3 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2023-05-09 11:48 UTC (permalink / raw)
  To: linux-btrfs

The function write_and_map_eb() is quite abused as a way to write any
generic buffer back to disk.

But we have a more situable function already, write_data_to_disk().

This patch would remove the abused write_data_to_disk() calls, and
convert the only three valid call sites to write_data_to_disk() instead.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-corrupt-block.c     | 48 +++++++++++++++++++++++----------------
 convert/main.c            | 19 +++++++---------
 convert/source-reiserfs.c | 10 +-------
 kernel-shared/disk-io.c   | 23 +------------------
 kernel-shared/disk-io.h   |  1 -
 mkfs/rootdir.c            | 41 ++++++++++++---------------------
 6 files changed, 53 insertions(+), 89 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 98cfe598320a..9bbebf25d4b7 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -38,40 +38,47 @@
 
 #define FIELD_BUF_LEN 80
 
-static int debug_corrupt_block(struct extent_buffer *eb,
-		struct btrfs_root *root, u64 bytenr, u32 blocksize, u64 copy)
+static int debug_corrupt_sector(struct btrfs_root *root, u64 logical, int mirror)
 {
+	const u32 sectorsize = root->fs_info->sectorsize;
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	int ret;
 	int num_copies;
 	int mirror_num = 1;
+	void *buf;
+
+	buf = malloc(root->fs_info->sectorsize);
+	if (!buf) {
+		error_msg(ERROR_MSG_MEMORY, "allocating memory for bytenr %llu",
+			  logical);
+		return -ENOMEM;
+	}
 
 	while (1) {
-		if (!copy || mirror_num == copy) {
-			u64 read_len = eb->len;
+		if (!mirror || mirror_num == mirror) {
+			u64 read_len = sectorsize;
 
-			ret = read_data_from_disk(eb->fs_info, eb->data,
-						  eb->start, &read_len,
-						  mirror_num);
-			if (read_len < eb->len)
+			ret = read_data_from_disk(fs_info, buf, logical,
+						  &read_len, mirror_num);
+			if (read_len < sectorsize)
 				ret = -EIO;
 			if (ret < 0) {
 				errno = -ret;
-				error("cannot read eb bytenr %llu: %m", eb->start);
+				error("cannot read bytenr %llu: %m", logical);
 				return ret;
 			}
-			printf("corrupting %llu copy %d\n", eb->start,
+			printf("corrupting %llu copy %d\n", logical,
 			       mirror_num);
-			memset(eb->data, 0, eb->len);
-			ret = write_and_map_eb(eb->fs_info, eb);
+			memset(buf, 0, sectorsize);
+			ret = write_data_to_disk(fs_info, buf, logical, sectorsize);
 			if (ret < 0) {
 				errno = -ret;
-				error("cannot write eb bytenr %llu: %m", eb->start);
+				error("cannot write bytenr %llu: %m", logical);
 				return ret;
 			}
 		}
 
-		num_copies = btrfs_num_copies(root->fs_info, eb->start,
-					      eb->len);
+		num_copies = btrfs_num_copies(root->fs_info, logical, sectorsize);
 		if (num_copies == 1)
 			break;
 
@@ -157,7 +164,7 @@ static void corrupt_keys(struct btrfs_trans_handle *trans,
 		u16 csum_type = fs_info->csum_type;
 
 		csum_tree_block_size(eb, csum_size, 0, csum_type);
-		write_and_map_eb(eb->fs_info, eb);
+		write_data_to_disk(eb->fs_info, eb->data, eb->start, eb->len);
 	}
 }
 
@@ -878,7 +885,7 @@ static int corrupt_metadata_block(struct btrfs_fs_info *fs_info, u64 block,
 		btrfs_set_header_generation(eb, bogus);
 		csum_tree_block_size(eb, fs_info->csum_size, 0,
 				     fs_info->csum_type);
-		ret = write_and_map_eb(fs_info, eb);
+		ret = write_data_to_disk(fs_info, eb->data, eb->start, eb->len);
 		free_extent_buffer(eb);
 		if (ret < 0) {
 			errno = -ret;
@@ -1607,8 +1614,11 @@ int main(int argc, char **argv)
 				goto out_close;
 			}
 
-			debug_corrupt_block(eb, root, logical,
-					    root->fs_info->sectorsize, copy);
+			ret = debug_corrupt_sector(root, logical, (int)copy);
+			if (ret < 0) {
+				ret = 1;
+				goto out_close;
+			}
 			free_extent_buffer(eb);
 		}
 		logical += root->fs_info->sectorsize;
diff --git a/convert/main.c b/convert/main.c
index 46c6dfc571ff..134b44d55f2c 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -367,7 +367,6 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans,
 	u64 hole_len;
 	struct cache_extent *cache;
 	struct btrfs_key key;
-	struct extent_buffer *eb;
 	int ret = 0;
 
 	/*
@@ -378,6 +377,8 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans,
 	 * migrate ranges that covered by old fs data.
 	 */
 	while (cur_off < range_end(range)) {
+		void *buf;
+
 		cache = search_cache_extent(used, cur_off);
 		if (!cache)
 			break;
@@ -399,25 +400,21 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans,
 		if (ret < 0)
 			break;
 
-		eb = malloc(sizeof(*eb) + cur_len);
-		if (!eb) {
+		buf = malloc(cur_len);
+		if (!buf) {
 			ret = -ENOMEM;
 			break;
 		}
 
-		ret = pread(fd, eb->data, cur_len, cur_off);
+		ret = pread(fd, buf, cur_len, cur_off);
 		if (ret < cur_len) {
 			ret = (ret < 0 ? ret : -EIO);
-			free(eb);
+			free(buf);
 			break;
 		}
-		eb->start = key.objectid;
-		eb->len = key.offset;
-		eb->fs_info = root->fs_info;
-
+		ret = write_data_to_disk(root->fs_info, buf, key.objectid, key.offset);
 		/* Write the data */
-		ret = write_and_map_eb(root->fs_info, eb);
-		free(eb);
+		free(buf);
 		if (ret < 0)
 			break;
 
diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
index ceb50b6af093..35fd01050c7a 100644
--- a/convert/source-reiserfs.c
+++ b/convert/source-reiserfs.c
@@ -352,7 +352,6 @@ static int convert_direct(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	u32 sectorsize = root->fs_info->sectorsize;
 	int ret;
-	struct extent_buffer *eb;
 
 	BUG_ON(length > sectorsize);
 	ret = btrfs_reserve_extent(trans, root, sectorsize,
@@ -360,14 +359,7 @@ static int convert_direct(struct btrfs_trans_handle *trans,
 	if (ret)
 		return ret;
 
-	eb = alloc_extent_buffer(root->fs_info, key.objectid, sectorsize);
-
-	if (!eb)
-		return -ENOMEM;
-
-	write_extent_buffer(eb, body, 0, length);
-	ret = write_and_map_eb(root->fs_info, eb);
-	free_extent_buffer(eb);
+	ret = write_data_to_disk(root->fs_info, body, key.objectid, sectorsize);
 	if (ret)
 		return ret;
 
diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
index 51fa4fdca7a7..f3b7631c385f 100644
--- a/kernel-shared/disk-io.c
+++ b/kernel-shared/disk-io.c
@@ -462,27 +462,6 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
 	return eb;
 }
 
-int write_and_map_eb(struct btrfs_fs_info *fs_info, struct extent_buffer *eb)
-{
-	int ret;
-	u64 *raid_map = NULL;
-	struct btrfs_multi_bio *multi = NULL;
-
-	/* write_data_to_disk() will handle all mirrors and RAID56. */
-	ret = write_data_to_disk(fs_info, eb->data, eb->start, eb->len);
-	if (ret < 0) {
-		errno = -ret;
-		error("failed to write bytenr %llu length %u: %m",
-			eb->start, eb->len);
-		goto out;
-	}
-
-out:
-	kfree(raid_map);
-	kfree(multi);
-	return ret;
-}
-
 int write_tree_block(struct btrfs_trans_handle *trans,
 		     struct btrfs_fs_info *fs_info,
 		     struct extent_buffer *eb)
@@ -500,7 +479,7 @@ int write_tree_block(struct btrfs_trans_handle *trans,
 	btrfs_set_header_flag(eb, BTRFS_HEADER_FLAG_WRITTEN);
 	csum_tree_block(fs_info, eb, 0);
 
-	return write_and_map_eb(fs_info, eb);
+	return write_data_to_disk(fs_info, eb->data, eb->start, eb->len);
 }
 
 void btrfs_setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
diff --git a/kernel-shared/disk-io.h b/kernel-shared/disk-io.h
index 6baa4a806b78..3a31667967cc 100644
--- a/kernel-shared/disk-io.h
+++ b/kernel-shared/disk-io.h
@@ -222,7 +222,6 @@ int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid);
 int write_tree_block(struct btrfs_trans_handle *trans,
 		     struct btrfs_fs_info *fs_info,
 		     struct extent_buffer *eb);
-int write_and_map_eb(struct btrfs_fs_info *fs_info, struct extent_buffer *eb);
 int btrfs_fs_roots_compare_roots(struct rb_node *node1, struct rb_node *node2);
 struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
 				     struct btrfs_fs_info *fs_info,
diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index aa2577addc0f..5fd3c6feea5c 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -316,7 +316,7 @@ static int add_file_items(struct btrfs_trans_handle *trans,
 	u64 file_pos = 0;
 	u64 cur_bytes;
 	u64 total_bytes;
-	struct extent_buffer *eb = NULL;
+	void *buf = NULL;
 	int fd;
 
 	if (st->st_size == 0)
@@ -358,12 +358,8 @@ static int add_file_items(struct btrfs_trans_handle *trans,
 	/* round up our st_size to the FS blocksize */
 	total_bytes = (u64)blocks * sectorsize;
 
-	/*
-	 * do our IO in extent buffers so it can work
-	 * against any raid type
-	 */
-	eb = calloc(1, sizeof(*eb) + sectorsize);
-	if (!eb) {
+	buf = malloc(sectorsize);
+	if (!buf) {
 		ret = -ENOMEM;
 		goto end;
 	}
@@ -385,37 +381,28 @@ again:
 
 	while (bytes_read < cur_bytes) {
 
-		memset(eb->data, 0, sectorsize);
+		memset(buf, 0, sectorsize);
 
-		ret_read = pread(fd, eb->data, sectorsize, file_pos +
-				   bytes_read);
+		ret_read = pread(fd, buf, sectorsize, file_pos + bytes_read);
 		if (ret_read == -1) {
 			error("cannot read %s at offset %llu length %u: %m",
 				path_name, file_pos + bytes_read, sectorsize);
 			goto end;
 		}
 
-		eb->start = first_block + bytes_read;
-		eb->len = sectorsize;
-		eb->fs_info = root->fs_info;
-
-		/*
-		 * we're doing the csum before we record the extent, but
-		 * that's ok
-		 */
-		ret = btrfs_csum_file_block(trans,
-				first_block + bytes_read + sectorsize,
-				first_block + bytes_read,
-				eb->data, sectorsize);
-		if (ret)
-			goto end;
-
-		ret = write_and_map_eb(root->fs_info, eb);
+		ret = write_data_to_disk(root->fs_info, buf,
+					 first_block + bytes_read, sectorsize);
 		if (ret) {
 			error("failed to write %s", path_name);
 			goto end;
 		}
 
+		ret = btrfs_csum_file_block(trans,
+				first_block + bytes_read + sectorsize,
+				first_block + bytes_read, buf, sectorsize);
+		if (ret)
+			goto end;
+
 		bytes_read += sectorsize;
 	}
 
@@ -434,7 +421,7 @@ again:
 		goto again;
 
 end:
-	free(eb);
+	free(buf);
 	close(fd);
 	return ret;
 }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup
  2023-05-09 11:48 [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup Qu Wenruo
                   ` (2 preceding siblings ...)
  2023-05-09 11:48 ` [PATCH 3/3] btrfs-progs: remove write_and_map_eb() Qu Wenruo
@ 2023-05-10 16:25 ` David Sterba
  3 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2023-05-10 16:25 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Tue, May 09, 2023 at 07:48:37PM +0800, Qu Wenruo wrote:
> During my convert fixes, I found a lot of locations allocating a dummy
> extent buffer with a size which is not nodesize.
> 
> Then just read data into that dummy eb, and later call
> write_and_map_eb() to write it back.
> 
> This behavior is a historic workaround, at a time where we only do
> proper RAID56 writeback for metadata, but not data.
> 
> But now we have all raid56 handling done properly, and has proper
> function to read/write any logical bytenr, read_data_from_disk() and
> write_data_to_disk().
> 
> Thus there is no longer any need to use write_and_map_eb() as a
> workaround.
> 
> This patchset would completely remove write_and_map_eb(), most call
> sites are just abuse, only 3 call sites are valid but can easily be
> converted to call write_data_to_disk().
> 
> Qu Wenruo (3):
>   btrfs-progs: split btrfs_direct_pio() functions into two
>   btrfs-progs: constify the buffer pointer for write functions
>   btrfs-progs: remove write_and_map_eb()

Added to devel, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-05-10 16:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-09 11:48 [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup Qu Wenruo
2023-05-09 11:48 ` [PATCH 1/3] btrfs-progs: split btrfs_direct_pio() functions into two Qu Wenruo
2023-05-09 11:48 ` [PATCH 2/3] btrfs-progs: constify the buffer pointer for write functions Qu Wenruo
2023-05-09 11:48 ` [PATCH 3/3] btrfs-progs: remove write_and_map_eb() Qu Wenruo
2023-05-10 16:25 ` [PATCH 0/3] btrfs-progs: write_and_map_eb() cleanup David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox