improve type safety by passing struct btrfs_bio around

improve type safety by passing struct btrfs_bio around

[Christoph Hellwig]

Hi all,

much of the btrfs I/O path work on a struct btrfs_bio but passes pointers
to the bio embedded into that around.  This series improves type safety
by always passing the actual btrfs_bio instead.

Diffstat:
 bio.c         |   53 ++++++++++++++++++++++--------------------
 bio.h         |    8 +++---
 compression.c |   33 ++++++++++++++------------
 compression.h |    4 +--
 extent_io.c   |   72 ++++++++++++++++++++++++++--------------------------------
 inode.c       |   57 +++++++++++++++++++--------------------------
 lzo.c         |   14 ++++-------
 zlib.c        |    2 -
 zstd.c        |    1 
 9 files changed, 114 insertions(+), 130 deletions(-)

[PATCH 01/10] btrfs: remove unused members from struct btrfs_encoded_read_private

[Christoph Hellwig]

The inode and file_offset members in truct btrfs_encoded_read_private
are unused, so remove them.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ecd0aa83927949..9ad0d181c7082a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9926,8 +9926,6 @@ static ssize_t btrfs_encoded_read_inline(
 }
 
 struct btrfs_encoded_read_private {
-	struct btrfs_inode *inode;
-	u64 file_offset;
 	wait_queue_head_t wait;
 	atomic_t pending;
 	blk_status_t status;
@@ -9958,8 +9956,6 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 					  u64 disk_io_size, struct page **pages)
 {
 	struct btrfs_encoded_read_private priv = {
-		.inode = inode,
-		.file_offset = file_offset,
 		.pending = ATOMIC_INIT(1),
 	};
 	unsigned long i = 0;
-- 
2.39.1

[PATCH 02/10] btrfs: cleanup btrfs_encoded_read_regular_fill_pages

[Christoph Hellwig]

btrfs_encoded_read_regular_fill_pages has a pretty odd control flow.
Unwind it so that there is a single loop over the pages array.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c | 51 ++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9ad0d181c7082a..431b6082ab3d83 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9959,39 +9959,34 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 		.pending = ATOMIC_INIT(1),
 	};
 	unsigned long i = 0;
-	u64 cur = 0;
+	struct bio *bio;
 
 	init_waitqueue_head(&priv.wait);
-	/* Submit bios for the extent, splitting due to bio limits as necessary. */
-	while (cur < disk_io_size) {
-		struct bio *bio = NULL;
-		u64 remaining = disk_io_size - cur;
-
-		while (bio || remaining) {
-			size_t bytes = min_t(u64, remaining, PAGE_SIZE);
-
-			if (!bio) {
-				bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ,
-						      inode,
-						      btrfs_encoded_read_endio,
-						      &priv);
-				bio->bi_iter.bi_sector =
-					(disk_bytenr + cur) >> SECTOR_SHIFT;
-			}
 
-			if (!bytes ||
-			    bio_add_page(bio, pages[i], bytes, 0) < bytes) {
-				atomic_inc(&priv.pending);
-				btrfs_submit_bio(bio, 0);
-				bio = NULL;
-				continue;
-			}
+	bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
+			      btrfs_encoded_read_endio, &priv);
+	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 
-			i++;
-			cur += bytes;
-			remaining -= bytes;
+	do {
+		size_t bytes = min_t(u64, disk_io_size, PAGE_SIZE);
+
+		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
+			atomic_inc(&priv.pending);
+			btrfs_submit_bio(bio, 0);
+
+			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
+					      btrfs_encoded_read_endio, &priv);
+			bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+			continue;
 		}
-	}
+
+		i++;
+		disk_bytenr += bytes;
+		disk_io_size -= bytes;
+	} while (disk_io_size);
+
+	atomic_inc(&priv.pending);
+	btrfs_submit_bio(bio, 0);
 
 	if (atomic_dec_return(&priv.pending))
 		io_wait_event(priv.wait, !atomic_read(&priv.pending));
-- 
2.39.1

[PATCH 03/10] btrfs: move zero filling of compressed read bios into common code

[Christoph Hellwig]

All algorithms have to fill the remainder of the orig_bio with zeroes,
so do it in common code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/compression.c |  2 ++
 fs/btrfs/lzo.c         | 14 +++++---------
 fs/btrfs/zlib.c        |  2 --
 fs/btrfs/zstd.c        |  1 -
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 5b1de1c19991e9..64c804dc3962f6 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -965,6 +965,8 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
 	ret = compression_decompress_bio(workspace, cb);
 	put_workspace(type, workspace);
 
+	if (!ret)
+		zero_fill_bio(cb->orig_bio);
 	return ret;
 }
 
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index dc66ee98989e90..3a095b9c6373ea 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -389,8 +389,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 			 */
 			btrfs_err(fs_info, "unexpectedly large lzo segment len %u",
 					seg_len);
-			ret = -EIO;
-			goto out;
+			return -EIO;
 		}
 
 		/* Copy the compressed segment payload into workspace */
@@ -401,8 +400,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 					    workspace->buf, &out_len);
 		if (ret != LZO_E_OK) {
 			btrfs_err(fs_info, "failed to decompress");
-			ret = -EIO;
-			goto out;
+			return -EIO;
 		}
 
 		/* Copy the data into inode pages */
@@ -411,7 +409,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 
 		/* All data read, exit */
 		if (ret == 0)
-			goto out;
+			return 0;
 		ret = 0;
 
 		/* Check if the sector has enough space for a segment header */
@@ -422,10 +420,8 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 		/* Skip the padding zeros */
 		cur_in += sector_bytes_left;
 	}
-out:
-	if (!ret)
-		zero_fill_bio(cb->orig_bio);
-	return ret;
+
+	return 0;
 }
 
 int lzo_decompress(struct list_head *ws, const u8 *data_in,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index da7bb9187b68a3..8acb05e176c540 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -350,8 +350,6 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	zlib_inflateEnd(&workspace->strm);
 	if (data_in)
 		kunmap_local(data_in);
-	if (!ret)
-		zero_fill_bio(cb->orig_bio);
 	return ret;
 }
 
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index e34f1ab99d56fe..f798da267590d4 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -609,7 +609,6 @@ int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 		}
 	}
 	ret = 0;
-	zero_fill_bio(cb->orig_bio);
 done:
 	if (workspace->in_buf.src)
 		kunmap_local(workspace->in_buf.src);
-- 
2.39.1

[PATCH 04/10] btrfs: pass a btrfs_bio to btrfs_submit_bio

[Christoph Hellwig]

btrfs_submit_bio expects the bio passed to it to be embedded into a
btrfs_bio structure.  Pass the btrfs_bio directly to inrease type
safety and make the code self-documenting.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/bio.c         | 14 +++++++-------
 fs/btrfs/bio.h         |  2 +-
 fs/btrfs/compression.c |  4 ++--
 fs/btrfs/extent_io.c   |  2 +-
 fs/btrfs/inode.c       |  6 +++---
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 726592868e9c5c..c04e103f876853 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -164,7 +164,7 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
 			goto done;
 		}
 
-		btrfs_submit_bio(&repair_bbio->bio, mirror);
+		btrfs_submit_bio(repair_bbio, mirror);
 		return;
 	}
 
@@ -232,7 +232,7 @@ static struct btrfs_failed_bio *repair_one_sector(struct btrfs_bio *failed_bbio,
 
 	mirror = next_repair_mirror(fbio, failed_bbio->mirror_num);
 	btrfs_debug(fs_info, "submitting repair read to mirror %d", mirror);
-	btrfs_submit_bio(repair_bio, mirror);
+	btrfs_submit_bio(repair_bbio, mirror);
 	return fbio;
 }
 
@@ -603,12 +603,12 @@ static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
 	return true;
 }
 
-static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
+static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 {
-	struct btrfs_bio *bbio = btrfs_bio(bio);
 	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_bio *orig_bbio = bbio;
+	struct bio *bio = &bbio->bio;
 	u64 logical = bio->bi_iter.bi_sector << 9;
 	u64 length = bio->bi_iter.bi_size;
 	u64 map_length = length;
@@ -650,7 +650,7 @@ static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
 		if (use_append) {
 			bio->bi_opf &= ~REQ_OP_WRITE;
 			bio->bi_opf |= REQ_OP_ZONE_APPEND;
-			ret = btrfs_extract_ordered_extent(btrfs_bio(bio));
+			ret = btrfs_extract_ordered_extent(bbio);
 			if (ret)
 				goto fail_put_bio;
 		}
@@ -686,9 +686,9 @@ static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
 	return true;
 }
 
-void btrfs_submit_bio(struct bio *bio, int mirror_num)
+void btrfs_submit_bio(struct btrfs_bio *bbio, int mirror_num)
 {
-	while (!btrfs_submit_chunk(bio, mirror_num))
+	while (!btrfs_submit_chunk(bbio, mirror_num))
 		;
 }
 
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index 873ff85817f0b2..b4e7d5ab7d236d 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -88,7 +88,7 @@ static inline void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 /* Bio only refers to one ordered extent. */
 #define REQ_BTRFS_ONE_ORDERED			REQ_DRV
 
-void btrfs_submit_bio(struct bio *bio, int mirror_num);
+void btrfs_submit_bio(struct btrfs_bio *bbio, int mirror_num);
 int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,
 			    u64 length, u64 logical, struct page *page,
 			    unsigned int pg_offset, int mirror_num);
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 64c804dc3962f6..27bea05cab1a1b 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -333,7 +333,7 @@ void btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
 	cb->nr_pages = nr_pages;
 
 	btrfs_add_compressed_bio_pages(cb, disk_start);
-	btrfs_submit_bio(&cb->bbio.bio, 0);
+	btrfs_submit_bio(&cb->bbio, 0);
 
 	if (blkcg_css)
 		kthread_associate_blkcg(NULL);
@@ -565,7 +565,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
 	if (memstall)
 		psi_memstall_leave(&pflags);
 
-	btrfs_submit_bio(&cb->bbio.bio, mirror_num);
+	btrfs_submit_bio(&cb->bbio, mirror_num);
 	return;
 
 out_free_compressed_pages:
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d53bd452b9ec30..77de129db364c9 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -157,7 +157,7 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
 	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
 		btrfs_submit_compressed_read(bio, mirror_num);
 	else
-		btrfs_submit_bio(bio, mirror_num);
+		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
 
 	/* The bio is owned by the end_io handler now */
 	bio_ctrl->bio = NULL;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 431b6082ab3d83..ed96c84f5be71d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7787,7 +7787,7 @@ static void btrfs_dio_submit_io(const struct iomap_iter *iter, struct bio *bio,
 	dip->bytes = bio->bi_iter.bi_size;
 
 	dio_data->submitted += bio->bi_iter.bi_size;
-	btrfs_submit_bio(bio, 0);
+	btrfs_submit_bio(bbio, 0);
 }
 
 static const struct iomap_ops btrfs_dio_iomap_ops = {
@@ -9972,7 +9972,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 
 		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
 			atomic_inc(&priv.pending);
-			btrfs_submit_bio(bio, 0);
+			btrfs_submit_bio(btrfs_bio(bio), 0);
 
 			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
 					      btrfs_encoded_read_endio, &priv);
@@ -9986,7 +9986,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	} while (disk_io_size);
 
 	atomic_inc(&priv.pending);
-	btrfs_submit_bio(bio, 0);
+	btrfs_submit_bio(btrfs_bio(bio), 0);
 
 	if (atomic_dec_return(&priv.pending))
 		io_wait_event(priv.wait, !atomic_read(&priv.pending));
-- 
2.39.1

[PATCH 05/10] btrfs: pass a btrfs_bio to btrfs_submit_compressed_read

[Christoph Hellwig]

btrfs_submit_compressed_read expects the bio passed to it to be embedded
into a btrfs_bio structure.  Pass the btrfs_bio directly to inrease type
safety and make the code self-documenting.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/compression.c | 16 ++++++++--------
 fs/btrfs/compression.h |  2 +-
 fs/btrfs/extent_io.c   |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 27bea05cab1a1b..c12e317e133624 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -498,15 +498,15 @@ static noinline int add_ra_bio_pages(struct inode *inode,
  * After the compressed pages are read, we copy the bytes into the
  * bio we were passed and then call the bio end_io calls
  */
-void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
+void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num)
 {
-	struct btrfs_inode *inode = btrfs_bio(bio)->inode;
+	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct extent_map_tree *em_tree = &inode->extent_tree;
 	struct compressed_bio *cb;
 	unsigned int compressed_len;
-	const u64 disk_bytenr = bio->bi_iter.bi_sector << SECTOR_SHIFT;
-	u64 file_offset = btrfs_bio(bio)->file_offset;
+	const u64 disk_bytenr = bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT;
+	u64 file_offset = bbio->file_offset;
 	u64 em_len;
 	u64 em_start;
 	struct extent_map *em;
@@ -534,10 +534,10 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
 	em_len = em->len;
 	em_start = em->start;
 
-	cb->len = bio->bi_iter.bi_size;
+	cb->len = bbio->bio.bi_iter.bi_size;
 	cb->compressed_len = compressed_len;
 	cb->compress_type = em->compress_type;
-	cb->orig_bio = bio;
+	cb->orig_bio = &bbio->bio;
 
 	free_extent_map(em);
 
@@ -558,7 +558,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
 			 &pflags);
 
 	/* include any pages we added in add_ra-bio_pages */
-	cb->len = bio->bi_iter.bi_size;
+	cb->len = bbio->bio.bi_iter.bi_size;
 
 	btrfs_add_compressed_bio_pages(cb, disk_bytenr);
 
@@ -573,7 +573,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
 out_free_bio:
 	bio_put(&cb->bbio.bio);
 out:
-	btrfs_bio_end_io(btrfs_bio(bio), ret);
+	btrfs_bio_end_io(bbio, ret);
 }
 
 /*
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 95d2e85c6e4eea..692bafa1050e8e 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -94,7 +94,7 @@ void btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
 				  blk_opf_t write_flags,
 				  struct cgroup_subsys_state *blkcg_css,
 				  bool writeback);
-void btrfs_submit_compressed_read(struct bio *bio, int mirror_num);
+void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num);
 
 unsigned int btrfs_compress_str2level(unsigned int type, const char *str);
 
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 77de129db364c9..6ea6f2c057ac3e 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -155,7 +155,7 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
 
 	if (btrfs_op(bio) == BTRFS_MAP_READ &&
 	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
-		btrfs_submit_compressed_read(bio, mirror_num);
+		btrfs_submit_compressed_read(btrfs_bio(bio), mirror_num);
 	else
 		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
 
-- 
2.39.1

[PATCH 06/10] btrfs: store a pointer to the original btrfs_bio in struct compressed_bio

[Christoph Hellwig]

The original bio must be a btrfs_bio, so store a pointer to the
btrfs_bio for better type checking.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/compression.c | 15 ++++++++-------
 fs/btrfs/compression.h |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index c12e317e133624..c5839d04690d67 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -177,7 +177,7 @@ static void end_compressed_bio_read(struct btrfs_bio *bbio)
 		status = errno_to_blk_status(btrfs_decompress_bio(cb));
 
 	btrfs_free_compressed_pages(cb);
-	btrfs_bio_end_io(btrfs_bio(cb->orig_bio), status);
+	btrfs_bio_end_io(cb->orig_bbio, status);
 	bio_put(&bbio->bio);
 }
 
@@ -357,7 +357,8 @@ static noinline int add_ra_bio_pages(struct inode *inode,
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	unsigned long end_index;
-	u64 cur = btrfs_bio(cb->orig_bio)->file_offset + cb->orig_bio->bi_iter.bi_size;
+	struct bio *orig_bio = &cb->orig_bbio->bio;
+	u64 cur = cb->orig_bbio->file_offset + orig_bio->bi_iter.bi_size;
 	u64 isize = i_size_read(inode);
 	int ret;
 	struct page *page;
@@ -447,7 +448,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
 		 */
 		if (!em || cur < em->start ||
 		    (cur + fs_info->sectorsize > extent_map_end(em)) ||
-		    (em->block_start >> 9) != cb->orig_bio->bi_iter.bi_sector) {
+		    (em->block_start >> 9) != orig_bio->bi_iter.bi_sector) {
 			free_extent_map(em);
 			unlock_extent(tree, cur, page_end, NULL);
 			unlock_page(page);
@@ -467,7 +468,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
 		}
 
 		add_size = min(em->start + em->len, page_end + 1) - cur;
-		ret = bio_add_page(cb->orig_bio, page, add_size, offset_in_page(cur));
+		ret = bio_add_page(orig_bio, page, add_size, offset_in_page(cur));
 		if (ret != add_size) {
 			unlock_extent(tree, cur, page_end, NULL);
 			unlock_page(page);
@@ -537,7 +538,7 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num)
 	cb->len = bbio->bio.bi_iter.bi_size;
 	cb->compressed_len = compressed_len;
 	cb->compress_type = em->compress_type;
-	cb->orig_bio = &bbio->bio;
+	cb->orig_bbio = bbio;
 
 	free_extent_map(em);
 
@@ -966,7 +967,7 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
 	put_workspace(type, workspace);
 
 	if (!ret)
-		zero_fill_bio(cb->orig_bio);
+		zero_fill_bio(&cb->orig_bbio->bio);
 	return ret;
 }
 
@@ -1044,7 +1045,7 @@ void __cold btrfs_exit_compress(void)
 int btrfs_decompress_buf2page(const char *buf, u32 buf_len,
 			      struct compressed_bio *cb, u32 decompressed)
 {
-	struct bio *orig_bio = cb->orig_bio;
+	struct bio *orig_bio = &cb->orig_bbio->bio;
 	/* Offset inside the full decompressed extent */
 	u32 cur_offset;
 
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 692bafa1050e8e..5d5146e72a860b 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -55,7 +55,7 @@ struct compressed_bio {
 
 	union {
 		/* For reads, this is the bio we are copying the data into */
-		struct bio *orig_bio;
+		struct btrfs_bio *orig_bbio;
 		struct work_struct write_end_work;
 	};
 
-- 
2.39.1

[PATCH 07/10] btrfs: simplify finding the inode in submit_one_bio

[Christoph Hellwig]

struct btrfs_bio now has an always valid inode pointer that can be used
to find the inode in submit_one_bio, so use that and initialize all
variables for which it is possible at declaration time.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 6ea6f2c057ac3e..05b96a77fba104 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -123,23 +123,16 @@ struct btrfs_bio_ctrl {
 
 static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
 {
-	struct bio *bio;
-	struct bio_vec *bv;
-	struct inode *inode;
-	int mirror_num;
+	struct bio *bio = bio_ctrl->bio;
+	int mirror_num = bio_ctrl->mirror_num;
 
-	if (!bio_ctrl->bio)
+	if (!bio)
 		return;
 
-	bio = bio_ctrl->bio;
-	bv = bio_first_bvec_all(bio);
-	inode = bv->bv_page->mapping->host;
-	mirror_num = bio_ctrl->mirror_num;
-
 	/* Caller should ensure the bio has at least some range added */
 	ASSERT(bio->bi_iter.bi_size);
 
-	if (!is_data_inode(inode)) {
+	if (!is_data_inode(&btrfs_bio(bio)->inode->vfs_inode)) {
 		if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
 			/*
 			 * For metadata read, we should have the parent_check,
-- 
2.39.1

[PATCH 08/10] btrfs: store a pointer to a btrfs_bio in struct btrfs_bio_ctrl

[Christoph Hellwig]

The bio in struct btrfs_bio_ctrl must be a btrfs_bio, so store a pointer
to the btrfs_bio for better type checking.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 47 ++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 05b96a77fba104..df143c5267e61b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -97,7 +97,7 @@ void btrfs_extent_buffer_leak_debug_check(struct btrfs_fs_info *fs_info)
  * how many bytes are there before stripe/ordered extent boundary.
  */
 struct btrfs_bio_ctrl {
-	struct bio *bio;
+	struct btrfs_bio *bbio;
 	int mirror_num;
 	enum btrfs_compression_type compress_type;
 	u32 len_to_oe_boundary;
@@ -123,37 +123,37 @@ struct btrfs_bio_ctrl {
 
 static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
 {
-	struct bio *bio = bio_ctrl->bio;
+	struct btrfs_bio *bbio = bio_ctrl->bbio;
 	int mirror_num = bio_ctrl->mirror_num;
 
-	if (!bio)
+	if (!bbio)
 		return;
 
 	/* Caller should ensure the bio has at least some range added */
-	ASSERT(bio->bi_iter.bi_size);
+	ASSERT(bbio->bio.bi_iter.bi_size);
 
-	if (!is_data_inode(&btrfs_bio(bio)->inode->vfs_inode)) {
-		if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
+	if (!is_data_inode(&bbio->inode->vfs_inode)) {
+		if (btrfs_op(&bbio->bio) != BTRFS_MAP_WRITE) {
 			/*
 			 * For metadata read, we should have the parent_check,
 			 * and copy it to bbio for metadata verification.
 			 */
 			ASSERT(bio_ctrl->parent_check);
-			memcpy(&btrfs_bio(bio)->parent_check,
+			memcpy(&bbio->parent_check,
 			       bio_ctrl->parent_check,
 			       sizeof(struct btrfs_tree_parent_check));
 		}
-		bio->bi_opf |= REQ_META;
+		bbio->bio.bi_opf |= REQ_META;
 	}
 
-	if (btrfs_op(bio) == BTRFS_MAP_READ &&
+	if (btrfs_op(&bbio->bio) == BTRFS_MAP_READ &&
 	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
-		btrfs_submit_compressed_read(btrfs_bio(bio), mirror_num);
+		btrfs_submit_compressed_read(bbio, mirror_num);
 	else
-		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
+		btrfs_submit_bio(bbio, mirror_num);
 
 	/* The bio is owned by the end_io handler now */
-	bio_ctrl->bio = NULL;
+	bio_ctrl->bbio = NULL;
 }
 
 /*
@@ -161,16 +161,16 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
  */
 static void submit_write_bio(struct btrfs_bio_ctrl *bio_ctrl, int ret)
 {
-	struct bio *bio = bio_ctrl->bio;
+	struct btrfs_bio *bbio = bio_ctrl->bbio;
 
-	if (!bio)
+	if (!bbio)
 		return;
 
 	if (ret) {
 		ASSERT(ret < 0);
-		btrfs_bio_end_io(btrfs_bio(bio), errno_to_blk_status(ret));
+		btrfs_bio_end_io(bbio, errno_to_blk_status(ret));
 		/* The bio is owned by the end_io handler now */
-		bio_ctrl->bio = NULL;
+		bio_ctrl->bbio = NULL;
 	} else {
 		submit_one_bio(bio_ctrl);
 	}
@@ -863,7 +863,7 @@ static bool btrfs_bio_is_contig(struct btrfs_bio_ctrl *bio_ctrl,
 				struct page *page, u64 disk_bytenr,
 				unsigned int pg_offset)
 {
-	struct bio *bio = bio_ctrl->bio;
+	struct bio *bio = &bio_ctrl->bbio->bio;
 	struct bio_vec *bvec = bio_last_bvec_all(bio);
 	const sector_t sector = disk_bytenr >> SECTOR_SHIFT;
 
@@ -902,7 +902,7 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 			      bio_ctrl->end_io_func, NULL);
 	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 	btrfs_bio(bio)->file_offset = file_offset;
-	bio_ctrl->bio = bio;
+	bio_ctrl->bbio = btrfs_bio(bio);
 	bio_ctrl->len_to_oe_boundary = U32_MAX;
 
 	/*
@@ -942,8 +942,8 @@ static void alloc_new_bio(struct btrfs_inode *inode,
  * @pg_offset:	offset of the new bio or to check whether we are adding
  *              a contiguous page to the previous one
  *
- * The will either add the page into the existing @bio_ctrl->bio, or allocate a
- * new one in @bio_ctrl->bio.
+ * The will either add the page into the existing @bio_ctrl->bbio, or allocate a
+ * new one in @bio_ctrl->bbio.
  * The mirror number for this IO should already be initizlied in
  * @bio_ctrl->mirror_num.
  */
@@ -956,7 +956,7 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl,
 	ASSERT(pg_offset + size <= PAGE_SIZE);
 	ASSERT(bio_ctrl->end_io_func);
 
-	if (bio_ctrl->bio &&
+	if (bio_ctrl->bbio &&
 	    !btrfs_bio_is_contig(bio_ctrl, page, disk_bytenr, pg_offset))
 		submit_one_bio(bio_ctrl);
 
@@ -964,7 +964,7 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl,
 		u32 len = size;
 
 		/* Allocate new bio if needed */
-		if (!bio_ctrl->bio) {
+		if (!bio_ctrl->bbio) {
 			alloc_new_bio(inode, bio_ctrl, disk_bytenr,
 				      page_offset(page) + pg_offset);
 		}
@@ -976,7 +976,8 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl,
 			len = bio_ctrl->len_to_oe_boundary;
 		}
 
-		if (bio_add_page(bio_ctrl->bio, page, len, pg_offset) != len) {
+		if (bio_add_page(&bio_ctrl->bbio->bio, page, len, pg_offset) !=
+				len) {
 			/* bio full: move on to a new one */
 			submit_one_bio(bio_ctrl);
 			continue;
-- 
2.39.1

[PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Christoph Hellwig]

Return the conaining struct btrfs_bio instead of the less type safe
struct bio from btrfs_bio_alloc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/bio.c       | 12 +++++++-----
 fs/btrfs/bio.h       |  6 +++---
 fs/btrfs/extent_io.c | 18 +++++++++---------
 fs/btrfs/inode.c     | 18 +++++++++---------
 4 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index c04e103f876853..527081abca026f 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -48,15 +48,17 @@ void btrfs_bio_init(struct btrfs_bio *bbio, struct btrfs_inode *inode,
  * Just like the underlying bio_alloc_bioset it will not fail as it is backed by
  * a mempool.
  */
-struct bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
-			    struct btrfs_inode *inode,
-			    btrfs_bio_end_io_t end_io, void *private)
+struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
+				  struct btrfs_inode *inode,
+				  btrfs_bio_end_io_t end_io, void *private)
 {
+	struct btrfs_bio *bbio;
 	struct bio *bio;
 
 	bio = bio_alloc_bioset(NULL, nr_vecs, opf, GFP_NOFS, &btrfs_bioset);
-	btrfs_bio_init(btrfs_bio(bio), inode, end_io, private);
-	return bio;
+	bbio = btrfs_bio(bio);
+	btrfs_bio_init(bbio, inode, end_io, private);
+	return bbio;
 }
 
 static struct bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index b4e7d5ab7d236d..dbf125f6fa336d 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -75,9 +75,9 @@ void __cold btrfs_bioset_exit(void);
 
 void btrfs_bio_init(struct btrfs_bio *bbio, struct btrfs_inode *inode,
 		    btrfs_bio_end_io_t end_io, void *private);
-struct bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
-			    struct btrfs_inode *inode,
-			    btrfs_bio_end_io_t end_io, void *private);
+struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
+				  struct btrfs_inode *inode,
+				  btrfs_bio_end_io_t end_io, void *private);
 
 static inline void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 {
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index df143c5267e61b..2d5e4df3419b0f 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -896,13 +896,13 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 			  u64 disk_bytenr, u64 file_offset)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
-	struct bio *bio;
+	struct btrfs_bio *bbio;
 
-	bio = btrfs_bio_alloc(BIO_MAX_VECS, bio_ctrl->opf, inode,
-			      bio_ctrl->end_io_func, NULL);
-	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
-	btrfs_bio(bio)->file_offset = file_offset;
-	bio_ctrl->bbio = btrfs_bio(bio);
+	bbio = btrfs_bio_alloc(BIO_MAX_VECS, bio_ctrl->opf, inode,
+			       bio_ctrl->end_io_func, NULL);
+	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+	bbio->file_offset = file_offset;
+	bio_ctrl->bbio = bbio;
 	bio_ctrl->len_to_oe_boundary = U32_MAX;
 
 	/*
@@ -911,7 +911,7 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 	 * them.
 	 */
 	if (bio_ctrl->compress_type == BTRFS_COMPRESS_NONE &&
-	    btrfs_use_zone_append(btrfs_bio(bio))) {
+	    btrfs_use_zone_append(bbio)) {
 		struct btrfs_ordered_extent *ordered;
 
 		ordered = btrfs_lookup_ordered_extent(inode, file_offset);
@@ -930,8 +930,8 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 		 * to always be set on the last added/replaced device.
 		 * This is a bit odd but has been like that for a long time.
 		 */
-		bio_set_dev(bio, fs_info->fs_devices->latest_dev->bdev);
-		wbc_init_bio(bio_ctrl->wbc, bio);
+		bio_set_dev(&bbio->bio, fs_info->fs_devices->latest_dev->bdev);
+		wbc_init_bio(bio_ctrl->wbc, &bbio->bio);
 	}
 }
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ed96c84f5be71d..7e691bab72dffa 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9959,24 +9959,24 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 		.pending = ATOMIC_INIT(1),
 	};
 	unsigned long i = 0;
-	struct bio *bio;
+	struct btrfs_bio *bbio;
 
 	init_waitqueue_head(&priv.wait);
 
-	bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
+	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
 			      btrfs_encoded_read_endio, &priv);
-	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 
 	do {
 		size_t bytes = min_t(u64, disk_io_size, PAGE_SIZE);
 
-		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
+		if (bio_add_page(&bbio->bio, pages[i], bytes, 0) < bytes) {
 			atomic_inc(&priv.pending);
-			btrfs_submit_bio(btrfs_bio(bio), 0);
+			btrfs_submit_bio(bbio, 0);
 
-			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
-					      btrfs_encoded_read_endio, &priv);
-			bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+			bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
+					       btrfs_encoded_read_endio, &priv);
+			bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 			continue;
 		}
 
@@ -9986,7 +9986,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	} while (disk_io_size);
 
 	atomic_inc(&priv.pending);
-	btrfs_submit_bio(btrfs_bio(bio), 0);
+	btrfs_submit_bio(bbio, 0);
 
 	if (atomic_dec_return(&priv.pending))
 		io_wait_event(priv.wait, !atomic_read(&priv.pending));
-- 
2.39.1

[PATCH 10/10] btrfs: make btrfs_split_bio work on struct btrfs_bio

[Christoph Hellwig]

btrfs_split_bio expects a btrfs_bio as argument and always allocates one.
Type both the orig_bio argument and the return value as struct btrfs_bio
to improve type safety.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/bio.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 527081abca026f..cf09c6271edbee 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -61,30 +61,31 @@ struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
 	return bbio;
 }
 
-static struct bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
-				   struct bio *orig, u64 map_length,
-				   bool use_append)
+static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
+					 struct btrfs_bio *orig_bbio,
+					 u64 map_length, bool use_append)
 {
-	struct btrfs_bio *orig_bbio = btrfs_bio(orig);
+	struct btrfs_bio *bbio;
 	struct bio *bio;
 
 	if (use_append) {
 		unsigned int nr_segs;
 
-		bio = bio_split_rw(orig, &fs_info->limits, &nr_segs,
+		bio = bio_split_rw(&orig_bbio->bio, &fs_info->limits, &nr_segs,
 				   &btrfs_clone_bioset, map_length);
 	} else {
-		bio = bio_split(orig, map_length >> SECTOR_SHIFT, GFP_NOFS,
-				&btrfs_clone_bioset);
+		bio = bio_split(&orig_bbio->bio, map_length >> SECTOR_SHIFT,
+				GFP_NOFS, &btrfs_clone_bioset);
 	}
-	btrfs_bio_init(btrfs_bio(bio), orig_bbio->inode, NULL, orig_bbio);
+	bbio = btrfs_bio(bio);
+	btrfs_bio_init(bbio, orig_bbio->inode, NULL, orig_bbio);
 
-	btrfs_bio(bio)->file_offset = orig_bbio->file_offset;
-	if (!(orig->bi_opf & REQ_BTRFS_ONE_ORDERED))
+	bbio->file_offset = orig_bbio->file_offset;
+	if (!(orig_bbio->bio.bi_opf & REQ_BTRFS_ONE_ORDERED))
 		orig_bbio->file_offset += map_length;
 
 	atomic_inc(&orig_bbio->pending_ios);
-	return bio;
+	return bbio;
 }
 
 static void btrfs_orig_write_end_io(struct bio *bio);
@@ -633,8 +634,8 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 		map_length = min(map_length, fs_info->max_zone_append_size);
 
 	if (map_length < length) {
-		bio = btrfs_split_bio(fs_info, bio, map_length, use_append);
-		bbio = btrfs_bio(bio);
+		bbio = btrfs_split_bio(fs_info, bbio, map_length, use_append);
+		bio = &bbio->bio;
 	}
 
 	/*
-- 
2.39.1

Re: [PATCH 01/10] btrfs: remove unused members from struct btrfs_encoded_read_private

[Johannes Thumshirn]

On 01.03.23 14:42, Christoph Hellwig wrote:
> The inode and file_offset members in truct btrfs_encoded_read_private
                               struct ~^

Otherwise
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 02/10] btrfs: cleanup btrfs_encoded_read_regular_fill_pages

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 03/10] btrfs: move zero filling of compressed read bios into common code

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 04/10] btrfs: pass a btrfs_bio to btrfs_submit_bio

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 05/10] btrfs: pass a btrfs_bio to btrfs_submit_compressed_read

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 06/10] btrfs: store a pointer to the original btrfs_bio in struct compressed_bio

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 07/10] btrfs: simplify finding the inode in submit_one_bio

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 08/10] btrfs: store a pointer to a btrfs_bio in struct btrfs_bio_ctrl

[Johannes Thumshirn]

On 01.03.23 14:43, Christoph Hellwig wrote:
> The bio in struct btrfs_bio_ctrl must be a btrfs_bio, so store a pointer
> to the btrfs_bio for better type checking.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/btrfs/extent_io.c | 47 ++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 05b96a77fba104..df143c5267e61b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -97,7 +97,7 @@ void btrfs_extent_buffer_leak_debug_check(struct btrfs_fs_info *fs_info)
>   * how many bytes are there before stripe/ordered extent boundary.
>   */
>  struct btrfs_bio_ctrl {
> -	struct bio *bio;
> +	struct btrfs_bio *bbio;
>  	int mirror_num;
>  	enum btrfs_compression_type compress_type;
>  	u32 len_to_oe_boundary;
> @@ -123,37 +123,37 @@ struct btrfs_bio_ctrl {
>  
>  static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>  {
> -	struct bio *bio = bio_ctrl->bio;
> +	struct btrfs_bio *bbio = bio_ctrl->bbio;
>  	int mirror_num = bio_ctrl->mirror_num;
>  

Can we keep a local bio in here? so we don't have to do
bbio->bio every other line?

> -	if (!bio)
> +	if (!bbio)
>  		return;
>  
>  	/* Caller should ensure the bio has at least some range added */
> -	ASSERT(bio->bi_iter.bi_size);
> +	ASSERT(bbio->bio.bi_iter.bi_size);
>  
> -	if (!is_data_inode(&btrfs_bio(bio)->inode->vfs_inode)) {
> -		if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
> +	if (!is_data_inode(&bbio->inode->vfs_inode)) {
> +		if (btrfs_op(&bbio->bio) != BTRFS_MAP_WRITE) {
>  			/*
>  			 * For metadata read, we should have the parent_check,
>  			 * and copy it to bbio for metadata verification.
>  			 */
>  			ASSERT(bio_ctrl->parent_check);
> -			memcpy(&btrfs_bio(bio)->parent_check,
> +			memcpy(&bbio->parent_check,
>  			       bio_ctrl->parent_check,
>  			       sizeof(struct btrfs_tree_parent_check));
>  		}
> -		bio->bi_opf |= REQ_META;
> +		bbio->bio.bi_opf |= REQ_META;
>  	}
>  
> -	if (btrfs_op(bio) == BTRFS_MAP_READ &&
> +	if (btrfs_op(&bbio->bio) == BTRFS_MAP_READ &&
>  	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
> -		btrfs_submit_compressed_read(btrfs_bio(bio), mirror_num);
> +		btrfs_submit_compressed_read(bbio, mirror_num);
>  	else
> -		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
> +		btrfs_submit_bio(bbio, mirror_num);
>  
>  	/* The bio is owned by the end_io handler now */
	   The bbio? 


Otherwise,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 10/10] btrfs: make btrfs_split_bio work on struct btrfs_bio

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 01/10] btrfs: remove unused members from struct btrfs_encoded_read_private

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> The inode and file_offset members in truct btrfs_encoded_read_private
> are unused, so remove them.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/inode.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index ecd0aa83927949..9ad0d181c7082a 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9926,8 +9926,6 @@ static ssize_t btrfs_encoded_read_inline(
>   }
>   
>   struct btrfs_encoded_read_private {
> -	struct btrfs_inode *inode;
> -	u64 file_offset;
>   	wait_queue_head_t wait;
>   	atomic_t pending;
>   	blk_status_t status;
> @@ -9958,8 +9956,6 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   					  u64 disk_io_size, struct page **pages)
>   {
>   	struct btrfs_encoded_read_private priv = {
> -		.inode = inode,
> -		.file_offset = file_offset,
>   		.pending = ATOMIC_INIT(1),
>   	};
>   	unsigned long i = 0;

Re: [PATCH 02/10] btrfs: cleanup btrfs_encoded_read_regular_fill_pages

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> btrfs_encoded_read_regular_fill_pages has a pretty odd control flow.
> Unwind it so that there is a single loop over the pages array.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/inode.c | 51 ++++++++++++++++++++++--------------------------
>   1 file changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 9ad0d181c7082a..431b6082ab3d83 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9959,39 +9959,34 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   		.pending = ATOMIC_INIT(1),
>   	};
>   	unsigned long i = 0;
> -	u64 cur = 0;
> +	struct bio *bio;
>   
>   	init_waitqueue_head(&priv.wait);
> -	/* Submit bios for the extent, splitting due to bio limits as necessary. */
> -	while (cur < disk_io_size) {
> -		struct bio *bio = NULL;
> -		u64 remaining = disk_io_size - cur;
> -
> -		while (bio || remaining) {
> -			size_t bytes = min_t(u64, remaining, PAGE_SIZE);
> -
> -			if (!bio) {
> -				bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ,
> -						      inode,
> -						      btrfs_encoded_read_endio,
> -						      &priv);
> -				bio->bi_iter.bi_sector =
> -					(disk_bytenr + cur) >> SECTOR_SHIFT;
> -			}
>   
> -			if (!bytes ||
> -			    bio_add_page(bio, pages[i], bytes, 0) < bytes) {
> -				atomic_inc(&priv.pending);
> -				btrfs_submit_bio(bio, 0);
> -				bio = NULL;
> -				continue;
> -			}
> +	bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
> +			      btrfs_encoded_read_endio, &priv);
> +	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;

Can't we even merge the bio allocation into the main loop?

Maybe something like this instead?

struct bio *bio = NULL;

while (cur < len) {
	if (!bio) {
		bio = btrfs_io_alloc();
		bio->bi_iter.bi_sector = (cur + orig_disk_bytenr) >>
					 SECTOR_SHIFT;
	}
	if (bio_add_page() < bytes) {
		btrfs_submit_bio();
		bio = NULL;
	}
	cur += bytes;
}

Thanks,
Qu
>   
> -			i++;
> -			cur += bytes;
> -			remaining -= bytes;
> +	do {
> +		size_t bytes = min_t(u64, disk_io_size, PAGE_SIZE);
> +
> +		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
> +			atomic_inc(&priv.pending);
> +			btrfs_submit_bio(bio, 0);
> +
> +			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
> +					      btrfs_encoded_read_endio, &priv);
> +			bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
> +			continue;
>   		}
> -	}
> +
> +		i++;
> +		disk_bytenr += bytes;
> +		disk_io_size -= bytes;
> +	} while (disk_io_size);
> +
> +	atomic_inc(&priv.pending);
> +	btrfs_submit_bio(bio, 0);
>   
>   	if (atomic_dec_return(&priv.pending))
>   		io_wait_event(priv.wait, !atomic_read(&priv.pending));

Re: [PATCH 03/10] btrfs: move zero filling of compressed read bios into common code

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> All algorithms have to fill the remainder of the orig_bio with zeroes,
> so do it in common code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/compression.c |  2 ++
>   fs/btrfs/lzo.c         | 14 +++++---------
>   fs/btrfs/zlib.c        |  2 --
>   fs/btrfs/zstd.c        |  1 -
>   4 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 5b1de1c19991e9..64c804dc3962f6 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -965,6 +965,8 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
>   	ret = compression_decompress_bio(workspace, cb);
>   	put_workspace(type, workspace);
>   
> +	if (!ret)
> +		zero_fill_bio(cb->orig_bio);
>   	return ret;
>   }
>   
> diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
> index dc66ee98989e90..3a095b9c6373ea 100644
> --- a/fs/btrfs/lzo.c
> +++ b/fs/btrfs/lzo.c
> @@ -389,8 +389,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>   			 */
>   			btrfs_err(fs_info, "unexpectedly large lzo segment len %u",
>   					seg_len);
> -			ret = -EIO;
> -			goto out;
> +			return -EIO;
>   		}
>   
>   		/* Copy the compressed segment payload into workspace */
> @@ -401,8 +400,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>   					    workspace->buf, &out_len);
>   		if (ret != LZO_E_OK) {
>   			btrfs_err(fs_info, "failed to decompress");
> -			ret = -EIO;
> -			goto out;
> +			return -EIO;
>   		}
>   
>   		/* Copy the data into inode pages */
> @@ -411,7 +409,7 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>   
>   		/* All data read, exit */
>   		if (ret == 0)
> -			goto out;
> +			return 0;
>   		ret = 0;
>   
>   		/* Check if the sector has enough space for a segment header */
> @@ -422,10 +420,8 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>   		/* Skip the padding zeros */
>   		cur_in += sector_bytes_left;
>   	}
> -out:
> -	if (!ret)
> -		zero_fill_bio(cb->orig_bio);
> -	return ret;
> +
> +	return 0;
>   }
>   
>   int lzo_decompress(struct list_head *ws, const u8 *data_in,
> diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
> index da7bb9187b68a3..8acb05e176c540 100644
> --- a/fs/btrfs/zlib.c
> +++ b/fs/btrfs/zlib.c
> @@ -350,8 +350,6 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>   	zlib_inflateEnd(&workspace->strm);
>   	if (data_in)
>   		kunmap_local(data_in);
> -	if (!ret)
> -		zero_fill_bio(cb->orig_bio);
>   	return ret;
>   }
>   
> diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
> index e34f1ab99d56fe..f798da267590d4 100644
> --- a/fs/btrfs/zstd.c
> +++ b/fs/btrfs/zstd.c
> @@ -609,7 +609,6 @@ int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>   		}
>   	}
>   	ret = 0;
> -	zero_fill_bio(cb->orig_bio);
>   done:
>   	if (workspace->in_buf.src)
>   		kunmap_local(workspace->in_buf.src);

Re: [PATCH 04/10] btrfs: pass a btrfs_bio to btrfs_submit_bio

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> btrfs_submit_bio expects the bio passed to it to be embedded into a
> btrfs_bio structure.  Pass the btrfs_bio directly to inrease type
> safety and make the code self-documenting.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/bio.c         | 14 +++++++-------
>   fs/btrfs/bio.h         |  2 +-
>   fs/btrfs/compression.c |  4 ++--
>   fs/btrfs/extent_io.c   |  2 +-
>   fs/btrfs/inode.c       |  6 +++---
>   5 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 726592868e9c5c..c04e103f876853 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -164,7 +164,7 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
>   			goto done;
>   		}
>   
> -		btrfs_submit_bio(&repair_bbio->bio, mirror);
> +		btrfs_submit_bio(repair_bbio, mirror);
>   		return;
>   	}
>   
> @@ -232,7 +232,7 @@ static struct btrfs_failed_bio *repair_one_sector(struct btrfs_bio *failed_bbio,
>   
>   	mirror = next_repair_mirror(fbio, failed_bbio->mirror_num);
>   	btrfs_debug(fs_info, "submitting repair read to mirror %d", mirror);
> -	btrfs_submit_bio(repair_bio, mirror);
> +	btrfs_submit_bio(repair_bbio, mirror);
>   	return fbio;
>   }
>   
> @@ -603,12 +603,12 @@ static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
>   	return true;
>   }
>   
> -static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
> +static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>   {
> -	struct btrfs_bio *bbio = btrfs_bio(bio);
>   	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	struct btrfs_bio *orig_bbio = bbio;
> +	struct bio *bio = &bbio->bio;
>   	u64 logical = bio->bi_iter.bi_sector << 9;
>   	u64 length = bio->bi_iter.bi_size;
>   	u64 map_length = length;
> @@ -650,7 +650,7 @@ static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
>   		if (use_append) {
>   			bio->bi_opf &= ~REQ_OP_WRITE;
>   			bio->bi_opf |= REQ_OP_ZONE_APPEND;
> -			ret = btrfs_extract_ordered_extent(btrfs_bio(bio));
> +			ret = btrfs_extract_ordered_extent(bbio);
>   			if (ret)
>   				goto fail_put_bio;
>   		}
> @@ -686,9 +686,9 @@ static bool btrfs_submit_chunk(struct bio *bio, int mirror_num)
>   	return true;
>   }
>   
> -void btrfs_submit_bio(struct bio *bio, int mirror_num)
> +void btrfs_submit_bio(struct btrfs_bio *bbio, int mirror_num)
>   {
> -	while (!btrfs_submit_chunk(bio, mirror_num))
> +	while (!btrfs_submit_chunk(bbio, mirror_num))
>   		;
>   }
>   
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index 873ff85817f0b2..b4e7d5ab7d236d 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -88,7 +88,7 @@ static inline void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
>   /* Bio only refers to one ordered extent. */
>   #define REQ_BTRFS_ONE_ORDERED			REQ_DRV
>   
> -void btrfs_submit_bio(struct bio *bio, int mirror_num);
> +void btrfs_submit_bio(struct btrfs_bio *bbio, int mirror_num);
>   int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,
>   			    u64 length, u64 logical, struct page *page,
>   			    unsigned int pg_offset, int mirror_num);
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 64c804dc3962f6..27bea05cab1a1b 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -333,7 +333,7 @@ void btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
>   	cb->nr_pages = nr_pages;
>   
>   	btrfs_add_compressed_bio_pages(cb, disk_start);
> -	btrfs_submit_bio(&cb->bbio.bio, 0);
> +	btrfs_submit_bio(&cb->bbio, 0);
>   
>   	if (blkcg_css)
>   		kthread_associate_blkcg(NULL);
> @@ -565,7 +565,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
>   	if (memstall)
>   		psi_memstall_leave(&pflags);
>   
> -	btrfs_submit_bio(&cb->bbio.bio, mirror_num);
> +	btrfs_submit_bio(&cb->bbio, mirror_num);
>   	return;
>   
>   out_free_compressed_pages:
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index d53bd452b9ec30..77de129db364c9 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -157,7 +157,7 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>   	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
>   		btrfs_submit_compressed_read(bio, mirror_num);
>   	else
> -		btrfs_submit_bio(bio, mirror_num);
> +		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
>   
>   	/* The bio is owned by the end_io handler now */
>   	bio_ctrl->bio = NULL;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 431b6082ab3d83..ed96c84f5be71d 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7787,7 +7787,7 @@ static void btrfs_dio_submit_io(const struct iomap_iter *iter, struct bio *bio,
>   	dip->bytes = bio->bi_iter.bi_size;
>   
>   	dio_data->submitted += bio->bi_iter.bi_size;
> -	btrfs_submit_bio(bio, 0);
> +	btrfs_submit_bio(bbio, 0);
>   }
>   
>   static const struct iomap_ops btrfs_dio_iomap_ops = {
> @@ -9972,7 +9972,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   
>   		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
>   			atomic_inc(&priv.pending);
> -			btrfs_submit_bio(bio, 0);
> +			btrfs_submit_bio(btrfs_bio(bio), 0);
>   
>   			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
>   					      btrfs_encoded_read_endio, &priv);
> @@ -9986,7 +9986,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   	} while (disk_io_size);
>   
>   	atomic_inc(&priv.pending);
> -	btrfs_submit_bio(bio, 0);
> +	btrfs_submit_bio(btrfs_bio(bio), 0);
>   
>   	if (atomic_dec_return(&priv.pending))
>   		io_wait_event(priv.wait, !atomic_read(&priv.pending));

Re: [PATCH 05/10] btrfs: pass a btrfs_bio to btrfs_submit_compressed_read

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> btrfs_submit_compressed_read expects the bio passed to it to be embedded
> into a btrfs_bio structure.  Pass the btrfs_bio directly to inrease type
> safety and make the code self-documenting.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/compression.c | 16 ++++++++--------
>   fs/btrfs/compression.h |  2 +-
>   fs/btrfs/extent_io.c   |  2 +-
>   3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 27bea05cab1a1b..c12e317e133624 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -498,15 +498,15 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>    * After the compressed pages are read, we copy the bytes into the
>    * bio we were passed and then call the bio end_io calls
>    */
> -void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
> +void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num)
>   {
> -	struct btrfs_inode *inode = btrfs_bio(bio)->inode;
> +	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	struct extent_map_tree *em_tree = &inode->extent_tree;
>   	struct compressed_bio *cb;
>   	unsigned int compressed_len;
> -	const u64 disk_bytenr = bio->bi_iter.bi_sector << SECTOR_SHIFT;
> -	u64 file_offset = btrfs_bio(bio)->file_offset;
> +	const u64 disk_bytenr = bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT;
> +	u64 file_offset = bbio->file_offset;
>   	u64 em_len;
>   	u64 em_start;
>   	struct extent_map *em;
> @@ -534,10 +534,10 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
>   	em_len = em->len;
>   	em_start = em->start;
>   
> -	cb->len = bio->bi_iter.bi_size;
> +	cb->len = bbio->bio.bi_iter.bi_size;
>   	cb->compressed_len = compressed_len;
>   	cb->compress_type = em->compress_type;
> -	cb->orig_bio = bio;
> +	cb->orig_bio = &bbio->bio;
>   
>   	free_extent_map(em);
>   
> @@ -558,7 +558,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
>   			 &pflags);
>   
>   	/* include any pages we added in add_ra-bio_pages */
> -	cb->len = bio->bi_iter.bi_size;
> +	cb->len = bbio->bio.bi_iter.bi_size;
>   
>   	btrfs_add_compressed_bio_pages(cb, disk_bytenr);
>   
> @@ -573,7 +573,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
>   out_free_bio:
>   	bio_put(&cb->bbio.bio);
>   out:
> -	btrfs_bio_end_io(btrfs_bio(bio), ret);
> +	btrfs_bio_end_io(bbio, ret);
>   }
>   
>   /*
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 95d2e85c6e4eea..692bafa1050e8e 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -94,7 +94,7 @@ void btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
>   				  blk_opf_t write_flags,
>   				  struct cgroup_subsys_state *blkcg_css,
>   				  bool writeback);
> -void btrfs_submit_compressed_read(struct bio *bio, int mirror_num);
> +void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num);
>   
>   unsigned int btrfs_compress_str2level(unsigned int type, const char *str);
>   
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 77de129db364c9..6ea6f2c057ac3e 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -155,7 +155,7 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>   
>   	if (btrfs_op(bio) == BTRFS_MAP_READ &&
>   	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
> -		btrfs_submit_compressed_read(bio, mirror_num);
> +		btrfs_submit_compressed_read(btrfs_bio(bio), mirror_num);
>   	else
>   		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
>   

Re: [PATCH 06/10] btrfs: store a pointer to the original btrfs_bio in struct compressed_bio

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> The original bio must be a btrfs_bio, so store a pointer to the
> btrfs_bio for better type checking.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/compression.c | 15 ++++++++-------
>   fs/btrfs/compression.h |  2 +-
>   2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index c12e317e133624..c5839d04690d67 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -177,7 +177,7 @@ static void end_compressed_bio_read(struct btrfs_bio *bbio)
>   		status = errno_to_blk_status(btrfs_decompress_bio(cb));
>   
>   	btrfs_free_compressed_pages(cb);
> -	btrfs_bio_end_io(btrfs_bio(cb->orig_bio), status);
> +	btrfs_bio_end_io(cb->orig_bbio, status);
>   	bio_put(&bbio->bio);
>   }
>   
> @@ -357,7 +357,8 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>   {
>   	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>   	unsigned long end_index;
> -	u64 cur = btrfs_bio(cb->orig_bio)->file_offset + cb->orig_bio->bi_iter.bi_size;
> +	struct bio *orig_bio = &cb->orig_bbio->bio;
> +	u64 cur = cb->orig_bbio->file_offset + orig_bio->bi_iter.bi_size;
>   	u64 isize = i_size_read(inode);
>   	int ret;
>   	struct page *page;
> @@ -447,7 +448,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>   		 */
>   		if (!em || cur < em->start ||
>   		    (cur + fs_info->sectorsize > extent_map_end(em)) ||
> -		    (em->block_start >> 9) != cb->orig_bio->bi_iter.bi_sector) {
> +		    (em->block_start >> 9) != orig_bio->bi_iter.bi_sector) {
>   			free_extent_map(em);
>   			unlock_extent(tree, cur, page_end, NULL);
>   			unlock_page(page);
> @@ -467,7 +468,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>   		}
>   
>   		add_size = min(em->start + em->len, page_end + 1) - cur;
> -		ret = bio_add_page(cb->orig_bio, page, add_size, offset_in_page(cur));
> +		ret = bio_add_page(orig_bio, page, add_size, offset_in_page(cur));
>   		if (ret != add_size) {
>   			unlock_extent(tree, cur, page_end, NULL);
>   			unlock_page(page);
> @@ -537,7 +538,7 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num)
>   	cb->len = bbio->bio.bi_iter.bi_size;
>   	cb->compressed_len = compressed_len;
>   	cb->compress_type = em->compress_type;
> -	cb->orig_bio = &bbio->bio;
> +	cb->orig_bbio = bbio;
>   
>   	free_extent_map(em);
>   
> @@ -966,7 +967,7 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
>   	put_workspace(type, workspace);
>   
>   	if (!ret)
> -		zero_fill_bio(cb->orig_bio);
> +		zero_fill_bio(&cb->orig_bbio->bio);
>   	return ret;
>   }
>   
> @@ -1044,7 +1045,7 @@ void __cold btrfs_exit_compress(void)
>   int btrfs_decompress_buf2page(const char *buf, u32 buf_len,
>   			      struct compressed_bio *cb, u32 decompressed)
>   {
> -	struct bio *orig_bio = cb->orig_bio;
> +	struct bio *orig_bio = &cb->orig_bbio->bio;
>   	/* Offset inside the full decompressed extent */
>   	u32 cur_offset;
>   
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 692bafa1050e8e..5d5146e72a860b 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -55,7 +55,7 @@ struct compressed_bio {
>   
>   	union {
>   		/* For reads, this is the bio we are copying the data into */
> -		struct bio *orig_bio;
> +		struct btrfs_bio *orig_bbio;
>   		struct work_struct write_end_work;
>   	};
>   

Re: [PATCH 07/10] btrfs: simplify finding the inode in submit_one_bio

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> struct btrfs_bio now has an always valid inode pointer that can be used
> to find the inode in submit_one_bio, so use that and initialize all
> variables for which it is possible at declaration time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Initially I want to recommend to also make bio_ctrl->bio to be 
btrfs_bio, but that would be done in the next patch already.

Thanks,
Qu
> ---
>   fs/btrfs/extent_io.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 6ea6f2c057ac3e..05b96a77fba104 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -123,23 +123,16 @@ struct btrfs_bio_ctrl {
>   
>   static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>   {
> -	struct bio *bio;
> -	struct bio_vec *bv;
> -	struct inode *inode;
> -	int mirror_num;
> +	struct bio *bio = bio_ctrl->bio;
> +	int mirror_num = bio_ctrl->mirror_num;
>   
> -	if (!bio_ctrl->bio)
> +	if (!bio)
>   		return;
>   
> -	bio = bio_ctrl->bio;
> -	bv = bio_first_bvec_all(bio);
> -	inode = bv->bv_page->mapping->host;
> -	mirror_num = bio_ctrl->mirror_num;
> -
>   	/* Caller should ensure the bio has at least some range added */
>   	ASSERT(bio->bi_iter.bi_size);
>   
> -	if (!is_data_inode(inode)) {
> +	if (!is_data_inode(&btrfs_bio(bio)->inode->vfs_inode)) {
>   		if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
>   			/*
>   			 * For metadata read, we should have the parent_check,

Re: [PATCH 08/10] btrfs: store a pointer to a btrfs_bio in struct btrfs_bio_ctrl

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> The bio in struct btrfs_bio_ctrl must be a btrfs_bio, so store a pointer
> to the btrfs_bio for better type checking.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/extent_io.c | 47 ++++++++++++++++++++++----------------------
>   1 file changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 05b96a77fba104..df143c5267e61b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -97,7 +97,7 @@ void btrfs_extent_buffer_leak_debug_check(struct btrfs_fs_info *fs_info)
>    * how many bytes are there before stripe/ordered extent boundary.
>    */
>   struct btrfs_bio_ctrl {
> -	struct bio *bio;
> +	struct btrfs_bio *bbio;
>   	int mirror_num;
>   	enum btrfs_compression_type compress_type;
>   	u32 len_to_oe_boundary;
> @@ -123,37 +123,37 @@ struct btrfs_bio_ctrl {
>   
>   static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>   {
> -	struct bio *bio = bio_ctrl->bio;
> +	struct btrfs_bio *bbio = bio_ctrl->bbio;
>   	int mirror_num = bio_ctrl->mirror_num;
>   
> -	if (!bio)
> +	if (!bbio)
>   		return;
>   
>   	/* Caller should ensure the bio has at least some range added */
> -	ASSERT(bio->bi_iter.bi_size);
> +	ASSERT(bbio->bio.bi_iter.bi_size);
>   
> -	if (!is_data_inode(&btrfs_bio(bio)->inode->vfs_inode)) {
> -		if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
> +	if (!is_data_inode(&bbio->inode->vfs_inode)) {
> +		if (btrfs_op(&bbio->bio) != BTRFS_MAP_WRITE) {
>   			/*
>   			 * For metadata read, we should have the parent_check,
>   			 * and copy it to bbio for metadata verification.
>   			 */
>   			ASSERT(bio_ctrl->parent_check);
> -			memcpy(&btrfs_bio(bio)->parent_check,
> +			memcpy(&bbio->parent_check,
>   			       bio_ctrl->parent_check,
>   			       sizeof(struct btrfs_tree_parent_check));
>   		}
> -		bio->bi_opf |= REQ_META;
> +		bbio->bio.bi_opf |= REQ_META;
>   	}
>   
> -	if (btrfs_op(bio) == BTRFS_MAP_READ &&
> +	if (btrfs_op(&bbio->bio) == BTRFS_MAP_READ &&
>   	    bio_ctrl->compress_type != BTRFS_COMPRESS_NONE)
> -		btrfs_submit_compressed_read(btrfs_bio(bio), mirror_num);
> +		btrfs_submit_compressed_read(bbio, mirror_num);
>   	else
> -		btrfs_submit_bio(btrfs_bio(bio), mirror_num);
> +		btrfs_submit_bio(bbio, mirror_num);
>   
>   	/* The bio is owned by the end_io handler now */
> -	bio_ctrl->bio = NULL;
> +	bio_ctrl->bbio = NULL;
>   }
>   
>   /*
> @@ -161,16 +161,16 @@ static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
>    */
>   static void submit_write_bio(struct btrfs_bio_ctrl *bio_ctrl, int ret)
>   {
> -	struct bio *bio = bio_ctrl->bio;
> +	struct btrfs_bio *bbio = bio_ctrl->bbio;
>   
> -	if (!bio)
> +	if (!bbio)
>   		return;
>   
>   	if (ret) {
>   		ASSERT(ret < 0);
> -		btrfs_bio_end_io(btrfs_bio(bio), errno_to_blk_status(ret));
> +		btrfs_bio_end_io(bbio, errno_to_blk_status(ret));
>   		/* The bio is owned by the end_io handler now */
> -		bio_ctrl->bio = NULL;
> +		bio_ctrl->bbio = NULL;
>   	} else {
>   		submit_one_bio(bio_ctrl);
>   	}
> @@ -863,7 +863,7 @@ static bool btrfs_bio_is_contig(struct btrfs_bio_ctrl *bio_ctrl,
>   				struct page *page, u64 disk_bytenr,
>   				unsigned int pg_offset)
>   {
> -	struct bio *bio = bio_ctrl->bio;
> +	struct bio *bio = &bio_ctrl->bbio->bio;
>   	struct bio_vec *bvec = bio_last_bvec_all(bio);
>   	const sector_t sector = disk_bytenr >> SECTOR_SHIFT;
>   
> @@ -902,7 +902,7 @@ static void alloc_new_bio(struct btrfs_inode *inode,
>   			      bio_ctrl->end_io_func, NULL);
>   	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
>   	btrfs_bio(bio)->file_offset = file_offset;
> -	bio_ctrl->bio = bio;
> +	bio_ctrl->bbio = btrfs_bio(bio);
>   	bio_ctrl->len_to_oe_boundary = U32_MAX;
>   
>   	/*
> @@ -942,8 +942,8 @@ static void alloc_new_bio(struct btrfs_inode *inode,
>    * @pg_offset:	offset of the new bio or to check whether we are adding
>    *              a contiguous page to the previous one
>    *
> - * The will either add the page into the existing @bio_ctrl->bio, or allocate a
> - * new one in @bio_ctrl->bio.
> + * The will either add the page into the existing @bio_ctrl->bbio, or allocate a
> + * new one in @bio_ctrl->bbio.
>    * The mirror number for this IO should already be initizlied in
>    * @bio_ctrl->mirror_num.
>    */
> @@ -956,7 +956,7 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl,
>   	ASSERT(pg_offset + size <= PAGE_SIZE);
>   	ASSERT(bio_ctrl->end_io_func);
>   
> -	if (bio_ctrl->bio &&
> +	if (bio_ctrl->bbio &&
>   	    !btrfs_bio_is_contig(bio_ctrl, page, disk_bytenr, pg_offset))
>   		submit_one_bio(bio_ctrl);
>   
> @@ -964,7 +964,7 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl,
>   		u32 len = size;
>   
>   		/* Allocate new bio if needed */
> -		if (!bio_ctrl->bio) {
> +		if (!bio_ctrl->bbio) {
>   			alloc_new_bio(inode, bio_ctrl, disk_bytenr,
>   				      page_offset(page) + pg_offset);
>   		}
> @@ -976,7 +976,8 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl,
>   			len = bio_ctrl->len_to_oe_boundary;
>   		}
>   
> -		if (bio_add_page(bio_ctrl->bio, page, len, pg_offset) != len) {
> +		if (bio_add_page(&bio_ctrl->bbio->bio, page, len, pg_offset) !=
> +				len) {
>   			/* bio full: move on to a new one */
>   			submit_one_bio(bio_ctrl);
>   			continue;

Re: [PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> Return the conaining struct btrfs_bio instead of the less type safe
> struct bio from btrfs_bio_alloc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/bio.c       | 12 +++++++-----
>   fs/btrfs/bio.h       |  6 +++---
>   fs/btrfs/extent_io.c | 18 +++++++++---------
>   fs/btrfs/inode.c     | 18 +++++++++---------
>   4 files changed, 28 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index c04e103f876853..527081abca026f 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -48,15 +48,17 @@ void btrfs_bio_init(struct btrfs_bio *bbio, struct btrfs_inode *inode,
>    * Just like the underlying bio_alloc_bioset it will not fail as it is backed by
>    * a mempool.
>    */
> -struct bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
> -			    struct btrfs_inode *inode,
> -			    btrfs_bio_end_io_t end_io, void *private)
> +struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
> +				  struct btrfs_inode *inode,
> +				  btrfs_bio_end_io_t end_io, void *private)
>   {
> +	struct btrfs_bio *bbio;
>   	struct bio *bio;
>   
>   	bio = bio_alloc_bioset(NULL, nr_vecs, opf, GFP_NOFS, &btrfs_bioset);
> -	btrfs_bio_init(btrfs_bio(bio), inode, end_io, private);
> -	return bio;
> +	bbio = btrfs_bio(bio);
> +	btrfs_bio_init(bbio, inode, end_io, private);
> +	return bbio;
>   }
>   
>   static struct bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index b4e7d5ab7d236d..dbf125f6fa336d 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -75,9 +75,9 @@ void __cold btrfs_bioset_exit(void);
>   
>   void btrfs_bio_init(struct btrfs_bio *bbio, struct btrfs_inode *inode,
>   		    btrfs_bio_end_io_t end_io, void *private);
> -struct bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
> -			    struct btrfs_inode *inode,
> -			    btrfs_bio_end_io_t end_io, void *private);
> +struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
> +				  struct btrfs_inode *inode,
> +				  btrfs_bio_end_io_t end_io, void *private);
>   
>   static inline void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
>   {
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index df143c5267e61b..2d5e4df3419b0f 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -896,13 +896,13 @@ static void alloc_new_bio(struct btrfs_inode *inode,
>   			  u64 disk_bytenr, u64 file_offset)
>   {
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
> -	struct bio *bio;
> +	struct btrfs_bio *bbio;
>   
> -	bio = btrfs_bio_alloc(BIO_MAX_VECS, bio_ctrl->opf, inode,
> -			      bio_ctrl->end_io_func, NULL);
> -	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
> -	btrfs_bio(bio)->file_offset = file_offset;
> -	bio_ctrl->bbio = btrfs_bio(bio);
> +	bbio = btrfs_bio_alloc(BIO_MAX_VECS, bio_ctrl->opf, inode,
> +			       bio_ctrl->end_io_func, NULL);
> +	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
> +	bbio->file_offset = file_offset;
> +	bio_ctrl->bbio = bbio;
>   	bio_ctrl->len_to_oe_boundary = U32_MAX;
>   
>   	/*
> @@ -911,7 +911,7 @@ static void alloc_new_bio(struct btrfs_inode *inode,
>   	 * them.
>   	 */
>   	if (bio_ctrl->compress_type == BTRFS_COMPRESS_NONE &&
> -	    btrfs_use_zone_append(btrfs_bio(bio))) {
> +	    btrfs_use_zone_append(bbio)) {
>   		struct btrfs_ordered_extent *ordered;
>   
>   		ordered = btrfs_lookup_ordered_extent(inode, file_offset);
> @@ -930,8 +930,8 @@ static void alloc_new_bio(struct btrfs_inode *inode,
>   		 * to always be set on the last added/replaced device.
>   		 * This is a bit odd but has been like that for a long time.
>   		 */
> -		bio_set_dev(bio, fs_info->fs_devices->latest_dev->bdev);
> -		wbc_init_bio(bio_ctrl->wbc, bio);
> +		bio_set_dev(&bbio->bio, fs_info->fs_devices->latest_dev->bdev);
> +		wbc_init_bio(bio_ctrl->wbc, &bbio->bio);
>   	}
>   }
>   
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index ed96c84f5be71d..7e691bab72dffa 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9959,24 +9959,24 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   		.pending = ATOMIC_INIT(1),
>   	};
>   	unsigned long i = 0;
> -	struct bio *bio;
> +	struct btrfs_bio *bbio;
>   
>   	init_waitqueue_head(&priv.wait);
>   
> -	bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
> +	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
>   			      btrfs_encoded_read_endio, &priv);
> -	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
> +	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
>   
>   	do {
>   		size_t bytes = min_t(u64, disk_io_size, PAGE_SIZE);
>   
> -		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
> +		if (bio_add_page(&bbio->bio, pages[i], bytes, 0) < bytes) {
>   			atomic_inc(&priv.pending);
> -			btrfs_submit_bio(btrfs_bio(bio), 0);
> +			btrfs_submit_bio(bbio, 0);
>   
> -			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
> -					      btrfs_encoded_read_endio, &priv);
> -			bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
> +			bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
> +					       btrfs_encoded_read_endio, &priv);
> +			bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
>   			continue;
>   		}
>   
> @@ -9986,7 +9986,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   	} while (disk_io_size);
>   
>   	atomic_inc(&priv.pending);
> -	btrfs_submit_bio(btrfs_bio(bio), 0);
> +	btrfs_submit_bio(bbio, 0);
>   
>   	if (atomic_dec_return(&priv.pending))
>   		io_wait_event(priv.wait, !atomic_read(&priv.pending));

Re: [PATCH 10/10] btrfs: make btrfs_split_bio work on struct btrfs_bio

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> btrfs_split_bio expects a btrfs_bio as argument and always allocates one.
> Type both the orig_bio argument and the return value as struct btrfs_bio
> to improve type safety.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   fs/btrfs/bio.c | 27 ++++++++++++++-------------
>   1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 527081abca026f..cf09c6271edbee 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -61,30 +61,31 @@ struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
>   	return bbio;
>   }
>   
> -static struct bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
> -				   struct bio *orig, u64 map_length,
> -				   bool use_append)
> +static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
> +					 struct btrfs_bio *orig_bbio,
> +					 u64 map_length, bool use_append)
>   {
> -	struct btrfs_bio *orig_bbio = btrfs_bio(orig);
> +	struct btrfs_bio *bbio;
>   	struct bio *bio;
>   
>   	if (use_append) {
>   		unsigned int nr_segs;
>   
> -		bio = bio_split_rw(orig, &fs_info->limits, &nr_segs,
> +		bio = bio_split_rw(&orig_bbio->bio, &fs_info->limits, &nr_segs,
>   				   &btrfs_clone_bioset, map_length);
>   	} else {
> -		bio = bio_split(orig, map_length >> SECTOR_SHIFT, GFP_NOFS,
> -				&btrfs_clone_bioset);
> +		bio = bio_split(&orig_bbio->bio, map_length >> SECTOR_SHIFT,
> +				GFP_NOFS, &btrfs_clone_bioset);
>   	}
> -	btrfs_bio_init(btrfs_bio(bio), orig_bbio->inode, NULL, orig_bbio);
> +	bbio = btrfs_bio(bio);
> +	btrfs_bio_init(bbio, orig_bbio->inode, NULL, orig_bbio);
>   
> -	btrfs_bio(bio)->file_offset = orig_bbio->file_offset;
> -	if (!(orig->bi_opf & REQ_BTRFS_ONE_ORDERED))
> +	bbio->file_offset = orig_bbio->file_offset;
> +	if (!(orig_bbio->bio.bi_opf & REQ_BTRFS_ONE_ORDERED))
>   		orig_bbio->file_offset += map_length;
>   
>   	atomic_inc(&orig_bbio->pending_ios);
> -	return bio;
> +	return bbio;
>   }
>   
>   static void btrfs_orig_write_end_io(struct bio *bio);
> @@ -633,8 +634,8 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>   		map_length = min(map_length, fs_info->max_zone_append_size);
>   
>   	if (map_length < length) {
> -		bio = btrfs_split_bio(fs_info, bio, map_length, use_append);
> -		bbio = btrfs_bio(bio);
> +		bbio = btrfs_split_bio(fs_info, bbio, map_length, use_append);
> +		bio = &bbio->bio;
>   	}
>   
>   	/*

Re: improve type safety by passing struct btrfs_bio around

[Qu Wenruo]

On 2023/3/1 21:42, Christoph Hellwig wrote:
> Hi all,
> 
> much of the btrfs I/O path work on a struct btrfs_bio but passes pointers
> to the bio embedded into that around.  This series improves type safety
> by always passing the actual btrfs_bio instead.

The whole series looks good and commented for each one.

But I'm still a little concerned about possible rogue non-btrfs bios.
Is it possible to add one magic number member for btrfs_bio, and do 
extra magic number check in btrfs_bio() macro?

Or am I over concerning?

Thanks,
Qu

> 
> Diffstat:
>   bio.c         |   53 ++++++++++++++++++++++--------------------
>   bio.h         |    8 +++---
>   compression.c |   33 ++++++++++++++------------
>   compression.h |    4 +--
>   extent_io.c   |   72 ++++++++++++++++++++++++++--------------------------------
>   inode.c       |   57 +++++++++++++++++++--------------------------
>   lzo.c         |   14 ++++-------
>   zlib.c        |    2 -
>   zstd.c        |    1
>   9 files changed, 114 insertions(+), 130 deletions(-)

Re: [PATCH 01/10] btrfs: remove unused members from struct btrfs_encoded_read_private

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> The inode and file_offset members in truct btrfs_encoded_read_private
> are unused, so remove them.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks like commits 7959bd441176 and 7609afac6775 and removed their use.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 02/10] btrfs: cleanup btrfs_encoded_read_regular_fill_pages

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> btrfs_encoded_read_regular_fill_pages has a pretty odd control flow.
> Unwind it so that there is a single loop over the pages array.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 03/10] btrfs: move zero filling of compressed read bios into common code

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> All algorithms have to fill the remainder of the orig_bio with zeroes,
> so do it in common code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 04/10] btrfs: pass a btrfs_bio to btrfs_submit_bio

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> btrfs_submit_bio expects the bio passed to it to be embedded into a
> btrfs_bio structure.  Pass the btrfs_bio directly to inrease type

Typo: inrease -> increase

> safety and make the code self-documenting.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks
Anand

Re: [PATCH 05/10] btrfs: pass a btrfs_bio to btrfs_submit_compressed_read

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> btrfs_submit_compressed_read expects the bio passed to it to be embedded
> into a btrfs_bio structure.  Pass the btrfs_bio directly to inrease type

Typo: inrease->increase

> safety and make the code self-documenting.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

A nit is below..

> ---
>   fs/btrfs/compression.c | 16 ++++++++--------
>   fs/btrfs/compression.h |  2 +-
>   fs/btrfs/extent_io.c   |  2 +-
>   3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 27bea05cab1a1b..c12e317e133624 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -498,15 +498,15 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>    * After the compressed pages are read, we copy the bytes into the
>    * bio we were passed and then call the bio end_io calls
>    */
> -void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
> +void btrfs_submit_compressed_read(struct btrfs_bio *bbio, int mirror_num)
>   {
> -	struct btrfs_inode *inode = btrfs_bio(bio)->inode;
> +	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	struct extent_map_tree *em_tree = &inode->extent_tree;
>   	struct compressed_bio *cb;
>   	unsigned int compressed_len;

Simplify by declaring and assigning bio directly:
         struct bio *bio = bbio->bio;
This eliminates the need to dereference bbio->bio three times
later in the code.

1.
> -	const u64 disk_bytenr = bio->bi_iter.bi_sector << SECTOR_SHIFT;
> -	u64 file_offset = btrfs_bio(bio)->file_offset;
> +	const u64 disk_bytenr = bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT;
> +	u64 file_offset = bbio->file_offset;


>   	u64 em_len;
>   	u64 em_start;
>   	struct extent_map *em;
> @@ -534,10 +534,10 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
>   	em_len = em->len;
>   	em_start = em->start;
>   

2.
> -	cb->len = bio->bi_iter.bi_size;
> +	cb->len = bbio->bio.bi_iter.bi_size;


>   	cb->compressed_len = compressed_len;
>   	cb->compress_type = em->compress_type > -	cb->orig_bio = bio;
> +	cb->orig_bio = &bbio->bio;
>   
>   	free_extent_map(em);
>   
> @@ -558,7 +558,7 @@ void btrfs_submit_compressed_read(struct bio *bio, int mirror_num)
>   			 &pflags);
>   
>   	/* include any pages we added in add_ra-bio_pages */

3.
> -	cb->len = bio->bi_iter.bi_size;
> +	cb->len = bbio->bio.bi_iter.bi_size;


Thanks.
Anand

Re: [PATCH 06/10] btrfs: store a pointer to the original btrfs_bio in struct compressed_bio

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> The original bio must be a btrfs_bio, so store a pointer to the
> btrfs_bio for better type checking.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>


Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 07/10] btrfs: simplify finding the inode in submit_one_bio

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> struct btrfs_bio now has an always valid inode pointer that can be used
> to find the inode in submit_one_bio, so use that and initialize all
> variables for which it is possible at declaration time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 08/10] btrfs: store a pointer to a btrfs_bio in struct btrfs_bio_ctrl

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> The bio in struct btrfs_bio_ctrl must be a btrfs_bio, so store a pointer
> to the btrfs_bio for better type checking.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 02/10] btrfs: cleanup btrfs_encoded_read_regular_fill_pages

[Christoph Hellwig]

On Fri, Mar 03, 2023 at 07:24:29AM +0800, Qu Wenruo wrote:
> Can't we even merge the bio allocation into the main loop?
>
> Maybe something like this instead?

That should work.  I personally prefer the version that I sent out,
though.

Re: [PATCH 08/10] btrfs: store a pointer to a btrfs_bio in struct btrfs_bio_ctrl

[Christoph Hellwig]

On Thu, Mar 02, 2023 at 02:20:19PM +0000, Johannes Thumshirn wrote:
> >  {
> > -	struct bio *bio = bio_ctrl->bio;
> > +	struct btrfs_bio *bbio = bio_ctrl->bbio;
> >  	int mirror_num = bio_ctrl->mirror_num;
> >  
> 
> Can we keep a local bio in here? so we don't have to do
> bbio->bio every other line?

About half of these is going away in follow on patches, and in the
next series or two most of the remaining onces will disappear as well.

Re: improve type safety by passing struct btrfs_bio around

[Christoph Hellwig]

On Fri, Mar 03, 2023 at 07:34:08AM +0800, Qu Wenruo wrote:
> But I'm still a little concerned about possible rogue non-btrfs bios.
> Is it possible to add one magic number member for btrfs_bio, and do extra 
> magic number check in btrfs_bio() macro?

A magic number adds runtime overhead.  The container_of on the bio
frontpad is very common all over the kernel, and we don't do that
anywhere else.  After this series the btrfs_bio() only really happens
on allocation and the end_io handler which is fairly easy to keep
straight.

Re: [PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> Return the conaining struct btrfs_bio instead of the less type safe
> struct bio from btrfs_bio_alloc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>


Reviewed-by: Anand Jain <anand.jain@oracle.com>


-	struct bio *bio;
+	struct btrfs_bio *bbio;

Here, dereferenced for bio from %bbio appears more than once.
I am curious why you didn't choose to initialize the bio instead.

Re: [PATCH 10/10] btrfs: make btrfs_split_bio work on struct btrfs_bio

[Anand Jain]

On 01/03/2023 21:42, Christoph Hellwig wrote:
> btrfs_split_bio expects a btrfs_bio as argument and always allocates one.
> Type both the orig_bio argument and the return value as struct btrfs_bio
> to improve type safety.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Anand Jain]

On 04/03/2023 06:48, Anand Jain wrote:
> On 01/03/2023 21:42, Christoph Hellwig wrote:
>> Return the conaining struct btrfs_bio instead of the less type safe

Nit:
s/conaining/containing

>> struct bio from btrfs_bio_alloc.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> 
> Reviewed-by: Anand Jain <anand.jain@oracle.com>
> 
> 
> -    struct bio *bio;
> +    struct btrfs_bio *bbio;
> 
> Here, dereferenced for bio from %bbio appears more than once.
> I am curious why you didn't choose to initialize the bio instead.

Re: [PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Christoph Hellwig]

On Sat, Mar 04, 2023 at 06:48:54AM +0800, Anand Jain wrote:
> Reviewed-by: Anand Jain <anand.jain@oracle.com>
>
>
> -	struct bio *bio;
> +	struct btrfs_bio *bbio;
>
> Here, dereferenced for bio from %bbio appears more than once.
> I am curious why you didn't choose to initialize the bio instead.

As this is a bit of a theme here:  I prefer to not add a local variable
if there's less than about half a dozen dereference in the code, and
the dereferences aren't too long.  If there is a general consensus to
add a more local variables I can do that, but it doesn't feel helpful
to me.

[PATCH 09/10] btrfs: return a btrfs_bio from btrfs_bio_alloc

[Christoph Hellwig]

Return the containing struct btrfs_bio instead of the less type safe
struct bio from btrfs_bio_alloc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/bio.c       | 12 +++++++-----
 fs/btrfs/bio.h       |  6 +++---
 fs/btrfs/extent_io.c | 18 +++++++++---------
 fs/btrfs/inode.c     | 18 +++++++++---------
 4 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index c04e103f876853..527081abca026f 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -48,15 +48,17 @@ void btrfs_bio_init(struct btrfs_bio *bbio, struct btrfs_inode *inode,
  * Just like the underlying bio_alloc_bioset it will not fail as it is backed by
  * a mempool.
  */
-struct bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
-			    struct btrfs_inode *inode,
-			    btrfs_bio_end_io_t end_io, void *private)
+struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
+				  struct btrfs_inode *inode,
+				  btrfs_bio_end_io_t end_io, void *private)
 {
+	struct btrfs_bio *bbio;
 	struct bio *bio;
 
 	bio = bio_alloc_bioset(NULL, nr_vecs, opf, GFP_NOFS, &btrfs_bioset);
-	btrfs_bio_init(btrfs_bio(bio), inode, end_io, private);
-	return bio;
+	bbio = btrfs_bio(bio);
+	btrfs_bio_init(bbio, inode, end_io, private);
+	return bbio;
 }
 
 static struct bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index b4e7d5ab7d236d..dbf125f6fa336d 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -75,9 +75,9 @@ void __cold btrfs_bioset_exit(void);
 
 void btrfs_bio_init(struct btrfs_bio *bbio, struct btrfs_inode *inode,
 		    btrfs_bio_end_io_t end_io, void *private);
-struct bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
-			    struct btrfs_inode *inode,
-			    btrfs_bio_end_io_t end_io, void *private);
+struct btrfs_bio *btrfs_bio_alloc(unsigned int nr_vecs, blk_opf_t opf,
+				  struct btrfs_inode *inode,
+				  btrfs_bio_end_io_t end_io, void *private);
 
 static inline void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 {
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index aece6f020d1473..3150bc45306520 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -896,13 +896,13 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 			  u64 disk_bytenr, u64 file_offset)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
-	struct bio *bio;
+	struct btrfs_bio *bbio;
 
-	bio = btrfs_bio_alloc(BIO_MAX_VECS, bio_ctrl->opf, inode,
-			      bio_ctrl->end_io_func, NULL);
-	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
-	btrfs_bio(bio)->file_offset = file_offset;
-	bio_ctrl->bbio = btrfs_bio(bio);
+	bbio = btrfs_bio_alloc(BIO_MAX_VECS, bio_ctrl->opf, inode,
+			       bio_ctrl->end_io_func, NULL);
+	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+	bbio->file_offset = file_offset;
+	bio_ctrl->bbio = bbio;
 	bio_ctrl->len_to_oe_boundary = U32_MAX;
 
 	/*
@@ -911,7 +911,7 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 	 * them.
 	 */
 	if (bio_ctrl->compress_type == BTRFS_COMPRESS_NONE &&
-	    btrfs_use_zone_append(btrfs_bio(bio))) {
+	    btrfs_use_zone_append(bbio)) {
 		struct btrfs_ordered_extent *ordered;
 
 		ordered = btrfs_lookup_ordered_extent(inode, file_offset);
@@ -930,8 +930,8 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 		 * to always be set on the last added/replaced device.
 		 * This is a bit odd but has been like that for a long time.
 		 */
-		bio_set_dev(bio, fs_info->fs_devices->latest_dev->bdev);
-		wbc_init_bio(bio_ctrl->wbc, bio);
+		bio_set_dev(&bbio->bio, fs_info->fs_devices->latest_dev->bdev);
+		wbc_init_bio(bio_ctrl->wbc, &bbio->bio);
 	}
 }
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index cb3c387e57993b..8d86134598f56b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9959,24 +9959,24 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 		.pending = ATOMIC_INIT(1),
 	};
 	unsigned long i = 0;
-	struct bio *bio;
+	struct btrfs_bio *bbio;
 
 	init_waitqueue_head(&priv.wait);
 
-	bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
+	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
 			      btrfs_encoded_read_endio, &priv);
-	bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 
 	do {
 		size_t bytes = min_t(u64, disk_io_size, PAGE_SIZE);
 
-		if (bio_add_page(bio, pages[i], bytes, 0) < bytes) {
+		if (bio_add_page(&bbio->bio, pages[i], bytes, 0) < bytes) {
 			atomic_inc(&priv.pending);
-			btrfs_submit_bio(btrfs_bio(bio), 0);
+			btrfs_submit_bio(bbio, 0);
 
-			bio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
-					      btrfs_encoded_read_endio, &priv);
-			bio->bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
+			bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, inode,
+					       btrfs_encoded_read_endio, &priv);
+			bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 			continue;
 		}
 
@@ -9986,7 +9986,7 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	} while (disk_io_size);
 
 	atomic_inc(&priv.pending);
-	btrfs_submit_bio(btrfs_bio(bio), 0);
+	btrfs_submit_bio(bbio, 0);
 
 	if (atomic_dec_return(&priv.pending))
 		io_wait_event(priv.wait, !atomic_read(&priv.pending));
-- 
2.39.1