[PATCH V2 1/1] block: check bio alignment in blk_mq_submit_bio

[PATCH V2 1/1] block: check bio alignment in blk_mq_submit_bio

[Ming Lei]

IO logical block size is one fundamental queue limit, and every IO has
to be aligned with logical block size because our bio split can't deal
with unaligned bio.

The check has to be done with queue usage counter grabbed because device
reconfiguration may change logical block size, and we can prevent the
reconfiguration from happening by holding queue usage counter.

logical_block_size stays in the 1st cache line of queue_limits, and this
cache line is always fetched in fast path via bio_may_exceed_limits(),
so IO perf won't be affected by this check.

Cc: Yi Zhang <yi.zhang@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ye Bin <yebin10@huawei.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
V2:
	- cover any zero sized bio which .bi_sector needs to be initialized too

 block/blk-mq.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3b4df8e5ac9e..d161682ecd20 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2914,6 +2914,17 @@ static void blk_mq_use_cached_rq(struct request *rq, struct blk_plug *plug,
 	INIT_LIST_HEAD(&rq->queuelist);
 }
 
+static bool bio_unaligned(const struct bio *bio, struct request_queue *q)
+{
+	unsigned int bs_mask = queue_logical_block_size(q) - 1;
+
+	/* .bi_sector of any zero sized bio need to be initialized */
+	if ((bio->bi_iter.bi_size & bs_mask) ||
+	    ((bio->bi_iter.bi_sector << SECTOR_SHIFT) & bs_mask))
+		return true;
+	return false;
+}
+
 /**
  * blk_mq_submit_bio - Create and send a request to block device.
  * @bio: Bio pointer.
@@ -2966,6 +2977,15 @@ void blk_mq_submit_bio(struct bio *bio)
 			return;
 	}
 
+	/*
+	 * Device reconfiguration may change logical block size, so alignment
+	 * check has to be done with queue usage counter held
+	 */
+	if (unlikely(bio_unaligned(bio, q))) {
+		bio_io_error(bio);
+		goto queue_exit;
+	}
+
 	if (unlikely(bio_may_exceed_limits(bio, &q->limits))) {
 		bio = __bio_split_to_limits(bio, &q->limits, &nr_segs);
 		if (!bio)
-- 
2.44.0

Re: [PATCH V2 1/1] block: check bio alignment in blk_mq_submit_bio

[Christoph Hellwig]

Module the q argument mess I'll just fix up when I get to it this
looks fine.

Re: [PATCH V2 1/1] block: check bio alignment in blk_mq_submit_bio

[Christoph Hellwig]

So make this a:

Reviewed-by: Christoph Hellwig <hch@lst.de>

so that we can get it included and the ball rolling.

On Wed, Jun 19, 2024 at 09:22:09PM -0700, Christoph Hellwig wrote:
> Module the q argument mess I'll just fix up when I get to it this
> looks fine.
> 
> 
---end quoted text---

Re: [PATCH V2 1/1] block: check bio alignment in blk_mq_submit_bio

[Jens Axboe]

On Thu, 20 Jun 2024 11:06:31 +0800, Ming Lei wrote:
> IO logical block size is one fundamental queue limit, and every IO has
> to be aligned with logical block size because our bio split can't deal
> with unaligned bio.
> 
> The check has to be done with queue usage counter grabbed because device
> reconfiguration may change logical block size, and we can prevent the
> reconfiguration from happening by holding queue usage counter.
> 
> [...]

Applied, thanks!

[1/1] block: check bio alignment in blk_mq_submit_bio
      commit: 0676c434a99be42f3bacca4adfd27df65edbf903

Best regards,
-- 
Jens Axboe