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

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 24, 2018 at 10:34:44AM -0700, Christoph Hellwig wrote:
> On Sat, Apr 21, 2018 at 02:54:05PM +0200, Jan Kara wrote:
> > > -		if (iocb->ki_flags & IOCB_DSYNC)
> > > +		if (iocb->ki_flags & IOCB_DSYNC) {
> > >  			dio->flags |= IOMAP_DIO_NEED_SYNC;
> > > +			/*
> > > +			 * 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.
> > > +			 */
> > > +			dio->flags |= IOMAP_DIO_WRITE_FUA;
> > > +		}
> > 
> > So I don't think this is quite correct. IOCB_DSYNC gets set also for O_SYNC
> > writes (in that case we also set IOCB_SYNC). And for those we cannot use
> > the FUA optimization AFAICT (definitely IOMAP_F_DIRTY isn't a safe
> > indicator of a need of full fsync for O_SYNC). Other than that the patch
> > looks good to me.
> 
> Oops, good catch. I think the above if should just be
> 
> 		if (iocb->ki_flags & (IOCB_DSYNC | IOCB_SYNC) == IOCB_DSYNC)) {
> 
> and we are fine. 

Ah, not exactly. IOMAP_DIO_NEED_SYNC needs to be set for either
DYSNC or SYNC writes, while IOMAP_DIO_WRITE_FUA should only be set
for DSYNC.

I'll fix this up appropriately.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com