Re: [PATCH 02/10] teach iomap_dio_rw() to suppress dsync

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 07, 2022 at 11:31:35PM +0000, Al Viro wrote:
> New flag, equivalent to removal of IOCB_DSYNC from iocb flags.
> This mimics what btrfs is doing (and that's what btrfs will
> switch to).  However, I'm not at all sure that we want to
> suppress REQ_FUA for those - all btrfs hack really cares about
> is suppression of generic_write_sync().  For now let's keep
> the existing behaviour, but I really want to hear more detailed
> arguments pro or contra.
> 
> [folded brain fix from willy]
> 
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c  | 20 +++++++++++---------
>  include/linux/iomap.h |  6 ++++++
>  2 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 370c3241618a..c10c69e2de24 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -548,17 +548,19 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		}
>  
>  		/* for data sync or sync, we need sync completion processing */
> -		if (iocb->ki_flags & IOCB_DSYNC)
> +		if (iocb->ki_flags & IOCB_DSYNC &&
> +		    !(dio_flags & IOMAP_DIO_NOSYNC)) {
>  			dio->flags |= IOMAP_DIO_NEED_SYNC;
>  
> -		/*
> -		 * For datasync only writes, we optimistically try using FUA for
> -		 * this IO.  Any non-FUA write that occurs will clear this flag,
> -		 * hence we know before completion whether a cache flush is
> -		 * necessary.
> -		 */
> -		if ((iocb->ki_flags & (IOCB_DSYNC | IOCB_SYNC)) == IOCB_DSYNC)
> -			dio->flags |= IOMAP_DIO_WRITE_FUA;
> +		       /*
> +			* For datasync only writes, we optimistically try
> +			* using FUA for this IO.  Any non-FUA write that
> +			* occurs will clear this flag, hence we know before
> +			* completion whether a cache flush is necessary.
> +			*/
> +			if (!(iocb->ki_flags & IOCB_SYNC))
> +				dio->flags |= IOMAP_DIO_WRITE_FUA;
> +		}
>  	}
>  
>  	if (dio_flags & IOMAP_DIO_OVERWRITE_ONLY) {
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index e552097c67e0..c8622d8f064e 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -353,6 +353,12 @@ struct iomap_dio_ops {
>   */
>  #define IOMAP_DIO_PARTIAL		(1 << 2)
>  
> +/*
> + * The caller will sync the write if needed; do not sync it within
> + * iomap_dio_rw.  Overrides IOMAP_DIO_FORCE_WAIT.
> + */
> +#define IOMAP_DIO_NOSYNC		(1 << 3)
> +
>  ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
>  		unsigned int dio_flags, void *private, size_t done_before);
> -- 
> 2.30.2
>