Re: [PATCH RFC v2 02/12] btrfs: always save bio::bi_iter into btrfs_bio::iter before submitting

[Christoph Hellwig <hch@infradead.org>]

On Wed, Apr 27, 2022 at 03:18:48PM +0800, Qu Wenruo wrote:
> Lower level bio mapping, from driver to dm, even btrfs chunk mapping
> can modify bio::bi_iter.
> 
> This prevents us from doing two things:
> 
> - Iterate the bio range of a cloned bio
>   This is only utilized by direct IO, thus it's already using
>   btrfs_bio::iter, which is populated in btrfs_bio_clone_partial().
> 
> - Grab the original logical bytenr of a bio
>   This will be utilized by incoming read repair patches.
> 
> So to make sure all btrfs_bio submitted to own a proper iter, this patch
> will assigned btrfs_bio::iter in the following call sites:

Independent of what we want to do with the saved iter in the future
I don't think this is an improvement.  The place where we can start
advancing the iter is after the bio is submit by btrfs_map_bio, so that
is the one central place where it should be saved.  I actually had
a patch like that in one my my wip branches:

---
From: Christoph Hellwig <hch@lst.de>
Subject: btrfs: centralize stashing away ->bi_iter in btrfs_map_bio

Once a bio is submitted to a driver, the driver can advance bi_iter.
Save to the copy in the btrfs_bio just before that can happen rather
than in random places high up in the stack.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/extent_io.c | 7 +------
 fs/btrfs/volumes.c   | 3 +++
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 07888cce3bce6..002b1ea92e398 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2683,7 +2683,6 @@ int btrfs_repair_one_sector(struct inode *inode,
 	}
 
 	bio_add_page(repair_bio, page, failrec->len, pgoff);
-	repair_bbio->iter = repair_bio->bi_iter;
 
 	btrfs_debug(btrfs_sb(inode->i_sb),
 		    "repair read error: submitting new read to mirror %d",
@@ -3209,14 +3208,11 @@ struct bio *btrfs_bio_alloc(unsigned int nr_iovecs)
 
 struct bio *btrfs_bio_clone(struct block_device *bdev, struct bio *bio)
 {
-	struct btrfs_bio *bbio;
 	struct bio *new;
 
 	/* Bio allocation backed by a bioset does not fail */
 	new = bio_alloc_clone(bdev, bio, GFP_NOFS, &btrfs_bioset);
-	bbio = btrfs_bio(new);
-	btrfs_bio_init(bbio);
-	bbio->iter = bio->bi_iter;
+	btrfs_bio_init(btrfs_bio(new));
 	return new;
 }
 
@@ -3235,7 +3231,6 @@ struct bio *btrfs_bio_clone_partial(struct bio *orig, u64 offset, u64 size)
 	btrfs_bio_init(bbio);
 
 	bio_trim(bio, offset >> 9, size >> 9);
-	bbio->iter = bio->bi_iter;
 	return bio;
 }
 
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 748614b00ffa2..f282a58a12344 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6750,6 +6750,9 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
 	int total_devs;
 	struct btrfs_io_context *bioc = NULL;
 
+	/* stash away the iter as the low-level processing can advance it */
+	btrfs_bio(bio)->iter = bio->bi_iter;
+
 	length = bio->bi_iter.bi_size;
 	map_length = length;
 
-- 
2.30.2