On Wed, Nov 02, 2022 at 07:05:35AM +0900, Damien Le Moal wrote:
> >> + if (!op_is_write(rq->cmd_flags) && (rq->cmd_flags & REQ_FUA)) {
> >> + blk_mq_end_request(rq, BLK_STS_NOTSUPP);
> >
> > How could this even happen? If we want a debug check, I think it
> > should be in submit_bio and a WARN_ON_ONCE.
>
> I have not found any code that issues a FUA read. So I do not think this
> can happen at all currently. The check is about making sure that it
> *never* happens.
>
> I thought of having the check higher up in the submit path but I wanted to
> avoid adding yet another check in the very hot path. But if you are OK
> with that, I will move it.
I'd do something like this:
---
>From 96847cce848938d1ee368e609ccb28a19854fba3 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@xxxxxx>
Date: Wed, 2 Nov 2022 08:05:41 +0100
Subject: block: add a sanity check for non-write flush/fua bios
Check that the PREFUSH and FUA flags are only set on write bios,
given that the flush state machine expects that.
Reported-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
block/blk-core.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index e9e2bf15cd909..4e2b01a53c6ab 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -720,12 +720,15 @@ void submit_bio_noacct(struct bio *bio)
* Filter flush bio's early so that bio based drivers without flush
* support don't have to worry about them.
*/
- if (op_is_flush(bio->bi_opf) &&
- !test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
- bio->bi_opf &= ~(REQ_PREFLUSH | REQ_FUA);
- if (!bio_sectors(bio)) {
- status = BLK_STS_OK;
+ if (op_is_flush(bio->bi_opf)) {
+ if (WARN_ON_ONCE(bio_op(bio) != REQ_OP_WRITE))
goto end_io;
+ if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
+ bio->bi_opf &= ~(REQ_PREFLUSH | REQ_FUA);
+ if (!bio_sectors(bio)) {
+ status = BLK_STS_OK;
+ goto end_io;
+ }
}
}
--
2.30.2
