Subject: Re: [PATCH] block: return the correct bvec when checking for gaps
On Fri, Jun 04, 2021 at 06:38:45AM +0000, Long Li wrote:
Subject: Re: [PATCH] block: return the correct bvec when checking for gaps
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.
Hi Ming,
When bio_will_gap() calls into biovec_phys_mergeable (),
seg_boundary_mask (queue_segment_boundary()) is used to test if the two bio_vecs can be merged. This test can succeed if only the 1st page in bvec is used, but at the same time it can fail if all the pages in bvec are used. In other words, if the pages in bvec go across the seg_boundary_mask, the test can potentially succeed if only the 1st page is tested, but can fail if all the pages are tested.
Later, when SCSI builds the SG list from BIOs (that calls into
__blk_bios_map_sg), __blk_segment_map_sg_merge() calls biovec_phys_mergeable() doing the same test . This time it may fail if the pages in bvec go across the seg_boundary_mask (but tested okay in bio_will_gap() earlier, so those two BIOs were merged). If __blk_segment_map_sg_merge() fails, we end up with a broken SG list for drivers assuming the SG list not having offsets in intermediate pages.
OK, the reason is that both bio_will_gap() and __blk_segment_map_sg_merge() have to use same approach to check if two bvecs from two bios can be mergeable.
Now __blk_segment_map_sg_merge() won't merge the 1st bvec of next bio into previous bio if the 1st bvec of next bio crosses segment boundary, so bio_will_gap() has to take same way to check if the two bvecs can be merged.
Please add the segment boundary and map SG list story in commit log, then the patch looks fine.
Sure, I will send v2.
Long
Thanks, Ming