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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 08, 2022 at 02:55:38PM +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.
>
> Reported-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
> Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
> ---
>  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;
> +		if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) {

Considering that blk_queue_write_cache() looks like this:

void blk_queue_write_cache(struct request_queue *q, bool wc, bool fua)
{
	if (wc)
		blk_queue_flag_set(QUEUE_FLAG_WC, q);
	else
		blk_queue_flag_clear(QUEUE_FLAG_WC, q);
	if (fua)
		blk_queue_flag_set(QUEUE_FLAG_FUA, q);
	else
		blk_queue_flag_clear(QUEUE_FLAG_FUA, q);

	wbt_set_write_cache(q, test_bit(QUEUE_FLAG_WC, &q->queue_flags));
}

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)) {


Kind regards,
Niklas



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux