On 04/24/18 19:34, 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.
The above line just gives parenthesis salad errors, so why not compromise
on:
if ((iocb->ki_flags & (IOCB_DSYNC | IOCB_SYNC)) == IOCB_DSYNC) {
Unless my bit twiddling has completely left me I think this is what was
intended, and it actually compiles too.
cheers
Holger
