Re: [PATCH v3 1/4] btrfs: remove the dead branch in btrfs_io_needs_validation()

[David Sterba <dsterba@suse.cz>]

On Mon, May 03, 2021 at 10:08:53AM +0800, Qu Wenruo wrote:
> In function btrfs_io_needs_validation() we are ensured to get a
> non-cloned bio.
> The only caller, end_bio_extent_readpage(), already has an ASSERT() to
> make sure we only get non-cloned bios.
> 
> Thus the (bio_flagged(bio, BIO_CLONED)) branch will never get executed.
> 
> Remove the dead branch and updated the comment.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent_io.c | 29 +++++++----------------------
>  1 file changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 14ab11381d49..0787fae5f7f1 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2644,8 +2644,10 @@ static bool btrfs_check_repairable(struct inode *inode, bool needs_validation,
>  
>  static bool btrfs_io_needs_validation(struct inode *inode, struct bio *bio)
>  {
> +	struct bio_vec *bvec;
>  	u64 len = 0;
>  	const u32 blocksize = inode->i_sb->s_blocksize;
> +	int i;
>  
>  	/*
>  	 * If bi_status is BLK_STS_OK, then this was a checksum error, not an
> @@ -2669,30 +2671,13 @@ static bool btrfs_io_needs_validation(struct inode *inode, struct bio *bio)
>  	if (blocksize < PAGE_SIZE)
>  		return false;
>  	/*
> -	 * We need to validate each sector individually if the failed I/O was
> -	 * for multiple sectors.
> -	 *
> -	 * There are a few possible bios that can end up here:
> -	 * 1. A buffered read bio, which is not cloned.
> -	 * 2. A direct I/O read bio, which is cloned.

Ok, so the cloned bio was expected only due to direct io but now it's
done via iomap so it makes sense to simplify it.

> -	 * 3. A (buffered or direct) repair bio, which is not cloned.
> -	 *
> -	 * For cloned bios (case 2), we can get the size from
> -	 * btrfs_io_bio->iter; for non-cloned bios (cases 1 and 3), we can get
> -	 * it from the bvecs.
> +	 * We're ensured we won't get cloned bio in end_bio_extent_readpage(),
> +	 * thus we can get the length from the bvecs.
>  	 */
> -	if (bio_flagged(bio, BIO_CLONED)) {
> -		if (btrfs_io_bio(bio)->iter.bi_size > blocksize)
> +	bio_for_each_bvec_all(bvec, bio, i) {
> +		len += bvec->bv_len;
> +		if (len > blocksize)
>  			return true;
> -	} else {
> -		struct bio_vec *bvec;
> -		int i;
> -
> -		bio_for_each_bvec_all(bvec, bio, i) {
> -			len += bvec->bv_len;
> -			if (len > blocksize)
> -				return true;
> -		}
>  	}
>  	return false;
>  }
> -- 
> 2.31.1