dde0c2e79848 "fs: add IOCB_SYNC and IOCB_DSYNC" had
done this
+ if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))
+ res |= IOCB_DSYNC;
in iocb_flags(). The first half is not a problem. However,
if you look at the second one, you'll see fetches from
file->f_mapping->host->i_flags and, worse yet,
file->f_mapping->host->i_sb->s_flags. Now, ->f_mapping might
be within the cacheline we'd already fetched from. However,
->f_mapping->host and ->f_mapping->host->i_flags are in
different cachelines (i_data.host and i_flags in some struct inode
instance). ->host->i_sb is in the same cacheline as ->host->i_flags,
but ->host->i_sb->s_flags is certain to bring yet another cacheline
into the picture.
IOW, it's not going to be cheap, no matter what we do. Moreover,
that thing used to live in generic_write_sync() and used only by
the ->write_iter() instances that knew they would care about it.
Now it's used by all ->write_iter() and ->read_iter() callers.
How about the following:
* new flag: IOCB_DSYNC_KNOWN
* in iocb_flags(), O_DSYNC => IOCB_SYNC_KNOWN | IOCB_DSYNC
* ditto for places explicitly setting IOCB_DSYNC
* places checking IOCB_DSYNC use
static inline bool iocb_is_dsync(struct kiocb *iocb)
{
if (likely(iocb->ki_flags & IOCB_DSYNC_KNOWN))
return iocb->ki_flags & IOCB_DSYNC;
if (unlikely(IS_SYNC(iocb->ki_filp->f_mapping->host))
iocb->ki_flags |= IOCB_DSYNC_KNOWN | IOCB_DSYNC;
return true;
}
iocb->ki_flags |= IOCB_DSYNC_KNOWN;
return false;
}
instead
That way init_sync_kiocb() becomes much lighter, especially
if we cache its value in struct file - there are very few places where
it can change. Comments?
