On Fri, Jan 22, 2021 at 05:46:43PM +0100, Christoph Hellwig wrote: > If the inode is not pinned by the time fsync is called we don't need the > ilock to protect against concurrent clearing of ili_fsync_fields as the > inode won't need a log flush or clearing of these fields. Not taking > the iolock allows for full concurrency of fsync and thus O_DSYNC > completions with io_uring/aio write submissions. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> Code looks good, so Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> But it makes me wonder... That is, we already elide the call to generic_write_sync() in direct IO in the case that the device supports FUA and it's a pure overwrite with no dirty metadata on the inode. Hence for a lot of storage and AIO/io_uring+DIO w/ O_DSYNC workloads we're already eliding this fsync-based lock cycle. In the case where we can't do a REQ_FUA IO because it is not supported by the device, then don't we really only need a cache flush at IO completion rather than the full generic_write_sync() call path? That would provide this optimisation to all the filesystems using iomap_dio_rw(), not just XFS.... In fact, I wonder if we need to do anything other than just use REQ_FUA unconditionally in iomap for this situation, as the block layer will translate REQ_FUA to a write+post-flush if the device doesn't support FUA writes directly. You're thoughts on that, Christoph? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
