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. 

> 
>                                                               Honza
> -- 
> Jan Kara <j...@suse.com>
> SUSE Labs, CR
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

Reply via email to