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:
signature.asc
Description: OpenPGP digital signature
