Hello Long,
On Thu, Jun 03, 2021 at 03:34:31PM -0700, longli@linuxonhyperv.com wrote:
From: Long Li longli@microsoft.com
After commit 07173c3ec276 ("block: enable multipage bvecs"), a bvec can have multiple pages. But bio_will_gap() still assumes one page bvec while checking for merging. This causes data corruption on drivers relying on the correct merging on virt_boundary_mask.
Can you explain the data corruption a bit?
IMO, either single page bvec or multipage bvec should be fine, because bio_will_gap() just checks if the last bvec of prev bio and the 1st bvec of next bio can be merged.
Fix this by returning the multi-page bvec for testing gaps for merging.
Cc: Jens Axboe axboe@kernel.dk Cc: Johannes Thumshirn johannes.thumshirn@wdc.com Cc: Pavel Begunkov asml.silence@gmail.com Cc: Ming Lei ming.lei@redhat.com Cc: Tejun Heo tj@kernel.org Cc: "Matthew Wilcox (Oracle)" willy@infradead.org Cc: Jeffle Xu jefflexu@linux.alibaba.com Cc: linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org Fixes: 07173c3ec276 ("block: enable multipage bvecs") Signed-off-by: Long Li longli@microsoft.com
include/linux/bio.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h index a0b4cfdf62a4..6b2f609ccfbf 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -44,9 +44,6 @@ static inline unsigned int bio_max_segs(unsigned int nr_segs) #define bio_offset(bio) bio_iter_offset((bio), (bio)->bi_iter) #define bio_iovec(bio) bio_iter_iovec((bio), (bio)->bi_iter) -#define bio_multiple_segments(bio) \
- ((bio)->bi_iter.bi_size != bio_iovec(bio).bv_len)
#define bvec_iter_sectors(iter) ((iter).bi_size >> 9) #define bvec_iter_end_sector(iter) ((iter).bi_sector + bvec_iter_sectors((iter))) @@ -271,7 +268,7 @@ static inline void bio_clear_flag(struct bio *bio, unsigned int bit) static inline void bio_get_first_bvec(struct bio *bio, struct bio_vec *bv) {
- *bv = bio_iovec(bio);
- *bv = mp_bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
} static inline void bio_get_last_bvec(struct bio *bio, struct bio_vec *bv) @@ -279,10 +276,10 @@ static inline void bio_get_last_bvec(struct bio *bio, struct bio_vec *bv) struct bvec_iter iter = bio->bi_iter; int idx;
- if (unlikely(!bio_multiple_segments(bio))) {
*bv = bio_iovec(bio);
- /* this bio has only one bvec */
- *bv = mp_bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
- if (bv->bv_len == bio->bi_iter.bi_size) return;
- }
bio_advance_iter(bio, &iter, iter.bi_size);
The patch itself looks fine, given both bio_get_first_bvec() and bio_get_last_bvec() are used in bio_will_gap() only.
Thanks, Ming