Re: [PATCH 3/3] iomap: Use FUA for pure data O_DSYNC DIO writes

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

 



>  		if (iomap->flags & IOMAP_F_NEW)
>  			need_zeroout = true;
> +		else {

Curly braces on both sides of the else, please.

>  		if (dio->flags & IOMAP_DIO_WRITE) {
> -			bio_set_op_attrs(bio, REQ_OP_WRITE, REQ_SYNC | REQ_IDLE);
> +			int op_flags = REQ_SYNC | REQ_IDLE;
> +
> +			if (use_fua)
> +				op_flags |= REQ_FUA;
> +			else
> +				dio->flags &= ~IOMAP_DIO_WRITE_FUA;
> +			bio_set_op_attrs(bio, REQ_OP_WRITE, op_flags);

Please kill the bio_set_op_attrs flags while you are at it and assign
directly to bio->bi_opf:

			bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_IDLE;
			if (use_fua)
				bio->bi_opf |= REQ_FUA;
			else
				dio->flags &= ~IOMAP_DIO_WRITE_FUA;

>  		dio->flags |= IOMAP_DIO_WRITE;
> -		if (iocb->ki_flags & IOCB_DSYNC)
> +		if (iocb->ki_flags & IOCB_DSYNC) {
>  			dio->flags |= IOMAP_DIO_WRITE_SYNC;
> +			/*
> +			 * Any non-FUA write will clear this flag, hence we know
> +			 * before completion whether a cache flush is necessary.
> +			 */
> +			dio->flags |= IOMAP_DIO_WRITE_FUA;

I'd mention that we optimistically try fua first in the comment, and then
go on with what you wrote.

> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ed63f3b69c12..e853f1748a4b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -800,6 +800,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
>  #define blk_queue_quiesced(q)	test_bit(QUEUE_FLAG_QUIESCED, &(q)->queue_flags)
>  #define blk_queue_preempt_only(q)				\
>  	test_bit(QUEUE_FLAG_PREEMPT_ONLY, &(q)->queue_flags)
> +#define blk_queue_fua(q)	test_bit(QUEUE_FLAG_FUA, &(q)->queue_flags)

Separate patch, please.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux