[PATCH 0/2] block: two fixes on avoiding bio splitting

[PATCH 0/2] block: two fixes on avoiding bio splitting

[Ming Lei]

Hi,

The 1st patch fixes kernel panic issue which is caused by not
splitting bio.

The 2nd patch requets to change the limit as SZ_4K instead of
PAGE_SIZE which can be big enough to break some devies.

Thanks,

Ming Lei (2):
  block: still try to split bio if the bvec crosses pages
  block: split bio if the only bvec's length is > SZ_4K

 block/blk-merge.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Cc: Christoph Hellwig <hch@lst.de>

-- 
2.20.1

[PATCH 1/2] block: still try to split bio if the bvec crosses pages

[Ming Lei]

Some device may set segment boundary as PAGE_SIZE - 1. If the bvec
crosses pages, and meantime its length is <= PAGE_SIZE, we still need
to split the bvec into 2 segments.

Fixes this issue by still splitting bio if the single bvec crosses
pages.

Reported-by: kernel test robot <lkp@intel.com>
Fixes: fa5322872187 (block: avoid blk_bio_segment_split for small I/O operations)
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-merge.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index f22cb6251d06..d783bdc4559b 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -319,7 +319,8 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
 		 */
 		if (!q->limits.chunk_sectors &&
 		    (*bio)->bi_vcnt == 1 &&
-		    (*bio)->bi_io_vec[0].bv_len <= PAGE_SIZE) {
+		    ((*bio)->bi_io_vec[0].bv_len +
+		     (*bio)->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
 			*nr_segs = 1;
 			break;
 		}
-- 
2.20.1

[RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K

[Ming Lei]

64K PAGE_SIZE is popular on ARM64 or other ARCHs, and 64K has been big
enough to break some devices probably, so change the logic to split bio
if the only bvec's length is > SZ_4K instead of PAGE_SIZE.

Fixes: fa5322872187 (block: avoid blk_bio_segment_split for small I/O operations)
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-merge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index d783bdc4559b..f35327f63ef4 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -320,7 +320,7 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
 		if (!q->limits.chunk_sectors &&
 		    (*bio)->bi_vcnt == 1 &&
 		    ((*bio)->bi_io_vec[0].bv_len +
-		     (*bio)->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
+		     (*bio)->bi_io_vec[0].bv_offset) <= SZ_4K) {
 			*nr_segs = 1;
 			break;
 		}
-- 
2.20.1

Re: [PATCH 0/2] block: two fixes on avoiding bio splitting

[Jens Axboe]

On 11/8/19 3:15 AM, Ming Lei wrote:
> Hi,
> 
> The 1st patch fixes kernel panic issue which is caused by not
> splitting bio.
> 
> The 2nd patch requets to change the limit as SZ_4K instead of
> PAGE_SIZE which can be big enough to break some devies.

Both changes make sense to me, applied, thanks.

-- 
Jens Axboe

Re: [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K

[Christoph Hellwig]

On Fri, Nov 08, 2019 at 06:15:28PM +0800, Ming Lei wrote:
> 64K PAGE_SIZE is popular on ARM64 or other ARCHs, and 64K has been big
> enough to break some devices probably, so change the logic to split bio
> if the only bvec's length is > SZ_4K instead of PAGE_SIZE.

I don't think this makes sense as-is given that blk_queue_max_hw_sectors,
blk_queue_max_segment_size and co all check for a minimum of PAGE_SIZE
and warn otherwise, and blk_bio_segment_split uses PAGE_SIZE for
its short cut as well. So I don't think this has been a problem in
practice, and if it was this patch is not enough.

So either we leave things as is, or we need to do a real audit for
code using PAGE_SIZE as the minimum I/O granularity and replace it
everywhere a well as updating the documentation.  Which might be a good
thing given that variable sized limits are weird.