Re: [PATCH 2/7] blk-mq: reflow blk_insert_flush

From: Damien Le Moal <dlemoal@kernel.org>
To: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>
Cc: Bart Van Assche <bvanassche@acm.org>, linux-block@vger.kernel.org
Subject: Re: [PATCH 2/7] blk-mq: reflow blk_insert_flush
Date: Mon, 17 Apr 2023 15:29:47 +0900	[thread overview]
Message-ID: <f6b64c7e-7d39-bf63-b1df-1f2b4a337a1e@kernel.org> (raw)
In-Reply-To: <20230416200930.29542-3-hch@lst.de>

On 4/17/23 05:09, Christoph Hellwig wrote:
> Use a switch statement to decide on the disposition of a flush request
> instead of multiple if statements, out of which one does checks that are
> more complex than required.  Also warn on a malformed request early
> on instead of doing a BUG_ON later.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

One nit below, but looks OK overall.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> ---
>  block/blk-flush.c | 53 +++++++++++++++++++++++------------------------
>  1 file changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/block/blk-flush.c b/block/blk-flush.c
> index 422a6d5446d1c5..9fcfee7394f27d 100644
> --- a/block/blk-flush.c
> +++ b/block/blk-flush.c
> @@ -402,6 +402,9 @@ void blk_insert_flush(struct request *rq)
>  	struct blk_flush_queue *fq = blk_get_flush_queue(q, rq->mq_ctx);
>  	struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
>  
> +	/* FLUSH/FUA request must never be merged */
> +	WARN_ON_ONCE(rq->bio != rq->biotail);

This could be:

	if (WARN_ON_ONCE(rq->bio != rq->biotail)) {
		blk_mq_end_request(rq, BLK_STS_IOERR);
 		return;
	}

To avoid an oops on the malformed request later on ?