Re: No way to break bio_for_each_segment_all() macro?

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

[-- Attachment #1.1: Type: text/plain, Size: 3562 bytes --]



On 2019/4/6 下午9:47, Ming Lei wrote:
> On Sat, Apr 06, 2019 at 09:53:07AM +0800, Qu Wenruo wrote:
>> Hi,
>>
>> I'm looking into a strange behavior that we can't break
>> bio_for_each_segment_all() after commit 6dc4f100c175 ("block: allow
>> bio_for_each_segment_all() to iterate over multi-page bvec").
>>
>> It's screwing up all bio_for_each_segment_all() call with error out branch.
>>
> 
> Please test the following patch:

It works!

Without touching the handling in btrfs part, error can break out of that
bio_for_each_segment_all() loop, just as it used to do.

Thanks,
Qu

> --
> 
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index bb6090aa165d..7bd7e64e02f8 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -120,19 +120,15 @@ static inline bool bio_full(struct bio *bio)
>  	return bio->bi_vcnt >= bio->bi_max_vecs;
>  }
>  
> -#define mp_bvec_for_each_segment(bv, bvl, i, iter_all)			\
> -	for (bv = bvec_init_iter_all(&iter_all);			\
> -		(iter_all.done < (bvl)->bv_len) &&			\
> -		(mp_bvec_next_segment((bvl), &iter_all), 1);		\
> -		iter_all.done += bv->bv_len, i += 1)
> -
>  /*
>   * drivers should _never_ use the all version - the bio may have been split
>   * before it got to the driver and the driver won't own all of it
>   */
> -#define bio_for_each_segment_all(bvl, bio, i, iter_all)		\
> -	for (i = 0, iter_all.idx = 0; iter_all.idx < (bio)->bi_vcnt; iter_all.idx++)	\
> -		mp_bvec_for_each_segment(bvl, &((bio)->bi_io_vec[iter_all.idx]), i, iter_all)
> +#define bio_for_each_segment_all(bvl, bio, i, iter_all)			\
> +	for (i = 0, bvl = bvec_init_iter_all(&iter_all);		\
> +		iter_all.idx < (bio)->bi_vcnt &&			\
> +		(mp_bvec_advance(&((bio)->bi_io_vec[iter_all.idx]),	\
> +				 &iter_all), 1); i++)
>  
>  static inline void bio_advance_iter(struct bio *bio, struct bvec_iter *iter,
>  				    unsigned bytes)
> diff --git a/include/linux/bvec.h b/include/linux/bvec.h
> index f6275c4da13a..6e4996dfc847 100644
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -48,7 +48,7 @@ struct bvec_iter {
>  struct bvec_iter_all {
>  	struct bio_vec	bv;
>  	int		idx;
> -	unsigned	done;
> +	unsigned	bv_done;
>  };
>  
>  static inline struct page *bvec_nth_page(struct page *page, int idx)
> @@ -145,18 +145,18 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv,
>  
>  static inline struct bio_vec *bvec_init_iter_all(struct bvec_iter_all *iter_all)
>  {
> -	iter_all->bv.bv_page = NULL;
> -	iter_all->done = 0;
> +	iter_all->bv_done = 0;
> +	iter_all->idx = 0;
>  
>  	return &iter_all->bv;
>  }
>  
> -static inline void mp_bvec_next_segment(const struct bio_vec *bvec,
> -					struct bvec_iter_all *iter_all)
> +static inline void mp_bvec_advance(const struct bio_vec *bvec,
> +				   struct bvec_iter_all *iter_all)
>  {
>  	struct bio_vec *bv = &iter_all->bv;
>  
> -	if (bv->bv_page) {
> +	if (iter_all->bv_done) {
>  		bv->bv_page = nth_page(bv->bv_page, 1);
>  		bv->bv_offset = 0;
>  	} else {
> @@ -164,7 +164,13 @@ static inline void mp_bvec_next_segment(const struct bio_vec *bvec,
>  		bv->bv_offset = bvec->bv_offset;
>  	}
>  	bv->bv_len = min_t(unsigned int, PAGE_SIZE - bv->bv_offset,
> -			   bvec->bv_len - iter_all->done);
> +			   bvec->bv_len - iter_all->bv_done);
> +	iter_all->bv_done += bv->bv_len;
> +
> +	if (iter_all->bv_done == bvec->bv_len) {
> +		iter_all->idx++;
> +		iter_all->bv_done = 0;
> +	}
>  }
>  
>  /*
> 
> Thanks,
> Ming
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]