Re: [PATCH 5/8] btrfs: simplify bvec iteration in index_one_bio

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2025/4/9 20:40, Christoph Hellwig 写道:
> Flatten the two loops by open coding bio_for_each_segment and advancing
> the iterator one sector at a time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/raid56.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index 28dbded86cc2..703e713bac03 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1195,23 +1195,20 @@ static int rbio_add_io_sector(struct btrfs_raid_bio *rbio,
>   static void index_one_bio(struct btrfs_raid_bio *rbio, struct bio *bio)
>   {
>   	const u32 sectorsize = rbio->bioc->fs_info->sectorsize;
> -	struct bio_vec bvec;
> -	struct bvec_iter iter;
> +	struct bvec_iter iter = bio->bi_iter;
>   	u32 offset = (bio->bi_iter.bi_sector << SECTOR_SHIFT) -
>   		     rbio->bioc->full_stripe_logical;
>   
> -	bio_for_each_segment(bvec, bio, iter) {
> -		u32 bvec_offset;
> +	while (iter.bi_size) {
> +		int index = offset / sectorsize;

The value @offset is no longer updated, this means only the first sector 
of the current bio will be updated, and it's updated several times, 
resulting incorrect location.

Furthermore, for full stripe write since we do not allocate pages for 
the stripes_pages[], we will got incorrect bio_sectors[] pointing to NULL.

This will crash rmw_rbio(), easily reproduced by btrfs/011.

I'll also fix it in my local branch.

Thanks,
Qu

> +		struct sector_ptr *sector = &rbio->bio_sectors[index];
> +		struct bio_vec bv = bio_iter_iovec(bio, iter);
>   
> -		for (bvec_offset = 0; bvec_offset < bvec.bv_len;
> -		     bvec_offset += sectorsize, offset += sectorsize) {
> -			int index = offset / sectorsize;
> -			struct sector_ptr *sector = &rbio->bio_sectors[index];
> +		sector->page = bv.bv_page;
> +		sector->pgoff = bv.bv_offset;
> +		ASSERT(sector->pgoff < PAGE_SIZE);
>   
> -			sector->page = bvec.bv_page;
> -			sector->pgoff = bvec.bv_offset + bvec_offset;
> -			ASSERT(sector->pgoff < PAGE_SIZE);
> -		}
> +		bio_advance_iter_single(bio, &iter, sectorsize);
>   	}
>   }
>