Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] block: two fixes on avoiding bio splitting
@ 2019-11-08 10:15 Ming Lei
  2019-11-08 10:15 ` [PATCH 1/2] block: still try to split bio if the bvec crosses pages Ming Lei
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ming Lei @ 2019-11-08 10:15 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Ming Lei, Christoph Hellwig

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] block: still try to split bio if the bvec crosses pages
  2019-11-08 10:15 [PATCH 0/2] block: two fixes on avoiding bio splitting Ming Lei
@ 2019-11-08 10:15 ` Ming Lei
  2019-11-08 10:15 ` [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K Ming Lei
  2019-11-08 14:00 ` [PATCH 0/2] block: two fixes on avoiding bio splitting Jens Axboe
  2 siblings, 0 replies; 5+ messages in thread
From: Ming Lei @ 2019-11-08 10:15 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Ming Lei, kernel test robot, Christoph Hellwig

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K
  2019-11-08 10:15 [PATCH 0/2] block: two fixes on avoiding bio splitting Ming Lei
  2019-11-08 10:15 ` [PATCH 1/2] block: still try to split bio if the bvec crosses pages Ming Lei
@ 2019-11-08 10:15 ` Ming Lei
  2019-11-11 10:21   ` Christoph Hellwig
  2019-11-08 14:00 ` [PATCH 0/2] block: two fixes on avoiding bio splitting Jens Axboe
  2 siblings, 1 reply; 5+ messages in thread
From: Ming Lei @ 2019-11-08 10:15 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Ming Lei, Christoph Hellwig

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] block: two fixes on avoiding bio splitting
  2019-11-08 10:15 [PATCH 0/2] block: two fixes on avoiding bio splitting Ming Lei
  2019-11-08 10:15 ` [PATCH 1/2] block: still try to split bio if the bvec crosses pages Ming Lei
  2019-11-08 10:15 ` [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K Ming Lei
@ 2019-11-08 14:00 ` Jens Axboe
  2 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2019-11-08 14:00 UTC (permalink / raw)
  To: Ming Lei; +Cc: linux-block, Christoph Hellwig

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K
  2019-11-08 10:15 ` [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K Ming Lei
@ 2019-11-11 10:21   ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2019-11-11 10:21 UTC (permalink / raw)
  To: Ming Lei; +Cc: Jens Axboe, linux-block, 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 10:15 [PATCH 0/2] block: two fixes on avoiding bio splitting Ming Lei
2019-11-08 10:15 ` [PATCH 1/2] block: still try to split bio if the bvec crosses pages Ming Lei
2019-11-08 10:15 ` [RFC PATCH 2/2] block: split bio if the only bvec's length is > SZ_4K Ming Lei
2019-11-11 10:21   ` Christoph Hellwig
2019-11-08 14:00 ` [PATCH 0/2] block: two fixes on avoiding bio splitting Jens Axboe

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git