On 6/19/24 12:34, 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.
>
> 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@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Cc: Ye Bin <yebin10@xxxxxxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> block/blk-mq.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 3b4df8e5ac9e..7bb50b6b9567 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2914,6 +2914,21 @@ 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,
> + const struct request_queue *q)
> +{
> + unsigned int bs = queue_logical_block_size(q);
> +
> + if (bio->bi_iter.bi_size & (bs - 1))
> + return true;
> +
> + if (bio->bi_iter.bi_size &&
> + ((bio->bi_iter.bi_sector << SECTOR_SHIFT) & (bs - 1)))
Hmmm... Some BIO operations have a 0 size but do specify a sector (e.g. zone
management operations). So this seems incorrect to me...
> + return true;
> +
> + return false;
> +}
> +
> /**
> * blk_mq_submit_bio - Create and send a request to block device.
> * @bio: Bio pointer.
> @@ -2966,6 +2981,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)
--
Damien Le Moal
Western Digital Research
