Re: [PATCH v6 1/7] block: add a sanity check for non-write flush/fua bios

Niklas Cassel
2022-12-30T07:21:27Z

On Tue, Nov 08, 2022 at 02:55:38PM +0900, Damien Le Moal wrote:
> From: Christoph Hellwig <hch@infradead.org>
>
> 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@opensource.wdc.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
> 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 17667159482e..d3446d38ba77 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -730,12 +730,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;

Considering that e.g. zonefs does:

bio = bio_alloc(bdev, nr_pages,
REQ_OP_ZONE_APPEND | REQ_SYNC | REQ_IDLE, GFP_NOFS);
...
if (iocb_is_dsync(iocb))
bio->bi_opf |= REQ_FUA;

It seems that we need to either allow REQ_OP_ZONE_APPEND explicitly here,
or perhaps make use of something like op_is_write().

Kind regards,
Niklas