Re: [PATCH 1/8] btrfs: remove the alignment checks in end_bbio_data_read

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

在 2025/4/9 20:40, Christoph Hellwig 写道:
> end_bbio_data_read checks that each iterated folio fragment is aligned
> and justifies that with block drivers advancing the bio.  But block
> driver only advance bi_iter, while end_bbio_data_read uses
> bio_for_each_folio_all to iterate the immutable bi_io_vec array that
> can't be changed by drivers at all.

The old comment indeed is incorrect, but can we still leave an ASSERT()
just in case to case any unaligned submission?

It shouldn't cause anything different for end users, but should greatly
improve the life of quality for developers.

Thanks,
Qu>
> Remove the alignment checking and the misleading comment.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/extent_io.c | 27 +++------------------------
>   1 file changed, 3 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 197f5e51c474..193736b07a0b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -512,43 +512,22 @@ static void end_bbio_data_read(struct btrfs_bio *bbio)
>   	struct btrfs_fs_info *fs_info = bbio->fs_info;
>   	struct bio *bio = &bbio->bio;
>   	struct folio_iter fi;
> -	const u32 sectorsize = fs_info->sectorsize;
>
>   	ASSERT(!bio_flagged(bio, BIO_CLONED));
>   	bio_for_each_folio_all(fi, &bbio->bio) {
>   		bool uptodate = !bio->bi_status;
>   		struct folio *folio = fi.folio;
>   		struct inode *inode = folio->mapping->host;
> -		u64 start;
> -		u64 end;
> -		u32 len;
> +		u64 start = folio_pos(folio) + fi.offset;
>
>   		btrfs_debug(fs_info,
>   			"%s: bi_sector=%llu, err=%d, mirror=%u",
>   			__func__, bio->bi_iter.bi_sector, bio->bi_status,
>   			bbio->mirror_num);
>
> -		/*
> -		 * We always issue full-sector reads, but if some block in a
> -		 * folio fails to read, blk_update_request() will advance
> -		 * bv_offset and adjust bv_len to compensate.  Print a warning
> -		 * for unaligned offsets, and an error if they don't add up to
> -		 * a full sector.
> -		 */
> -		if (!IS_ALIGNED(fi.offset, sectorsize))
> -			btrfs_err(fs_info,
> -		"partial page read in btrfs with offset %zu and length %zu",
> -				  fi.offset, fi.length);
> -		else if (!IS_ALIGNED(fi.offset + fi.length, sectorsize))
> -			btrfs_info(fs_info,
> -		"incomplete page read with offset %zu and length %zu",
> -				   fi.offset, fi.length);
> -
> -		start = folio_pos(folio) + fi.offset;
> -		end = start + fi.length - 1;
> -		len = fi.length;
>
>   		if (likely(uptodate)) {
> +			u64 end = start + fi.length - 1;
>   			loff_t i_size = i_size_read(inode);
>
>   			/*
> @@ -573,7 +552,7 @@ static void end_bbio_data_read(struct btrfs_bio *bbio)
>   		}
>
>   		/* Update page status and unlock. */
> -		end_folio_read(folio, uptodate, start, len);
> +		end_folio_read(folio, uptodate, start, fi.length);
>   	}
>   	bio_put(bio);
>   }