On Tue, Nov 20, 2018 at 08:17:38AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> When we write into an unwritten extent via direct IO, we dirty
> metadata on IO completion to convert the unwritten extent to
> written. However, when we do the FUA optimisation checks, the inode
> may be clean and so we issue a FUA write into the unwritten extent.
> This means we then bypass the generic_write_sync() call after
> unwritten extent conversion has ben done and we don't force the
> modified metadata to stable storage.
>
> This violates O_DSYNC semantics. The window of exposure is a single
> IO, as the next DIO write will see the inode has dirty metadata and
> hence will not use the FUA optimisation. Calling
> generic_write_sync() after completion of the second IO will also
> sync the first write and it's metadata.
>
> Fix this by avoiding the FUA optimisation when writing to unwritten
> extents.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/iomap.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 64ce240217a1..72f3864a2e6b 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1596,12 +1596,13 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
>
> if (iomap->flags & IOMAP_F_NEW) {
> need_zeroout = true;
> - } else {
> + } else if (iomap->type == IOMAP_MAPPED) {
> /*
> - * Use a FUA write if we need datasync semantics, this
> - * is a pure data IO that doesn't require any metadata
> - * updates and the underlying device supports FUA. This
> - * allows us to avoid cache flushes on IO completion.
> + * Use a FUA write if we need datasync semantics, this is a pure
> + * data IO that doesn't require any metadata updates (including
> + * after IO completion such as unwritten extent conversion) and
> + * the underlying device supports FUA. This allows us to avoid
> + * cache flushes on IO completion.
> */
> if (!(iomap->flags & (IOMAP_F_SHARED|IOMAP_F_DIRTY)) &&
> (dio->flags & IOMAP_DIO_WRITE_FUA) &&
> --
> 2.19.1
>
