On Tue, Jan 03, 2023 at 02:19:18PM +0900, Damien Le Moal wrote:
> From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
>
> Check that the PREFUSH and FUA flags are only set on write bios,
> given that the flush state machine expects that.
>
> [Damien] The check is also extended to REQ_OP_ZONE_APPEND operations as
> these are data write operations used by btrfs and zonefs and may also
> have the REQ_FUA bit set.
>
> Reported-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> block/blk-core.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 9321767470dc..c644aac498ef 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -744,12 +744,16 @@ 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 &&
> + bio_op(bio) != REQ_OP_ZONE_APPEND))
> 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;
> + }
> }
> }
Hello Damien,
In a previous email I wrote:
> It seems that you can have flag WC set, without having flag FUA set.
>
> So should perhaps the line:
>
>> + if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {
>
> instead be:
>
> if (!test_bit(QUEUE_FLAG_FUA, &q->queue_flags)) {
You replied with:
"Need both. If there is no write cache or write cache is off, FUA is
implied and is useless."
Did you change your mind since then?
Kind regards,
Niklas
