Re: [PATCH] iomap: Fix broken data integrity guarantees for O_SYNC writes

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jul 30, 2025 at 12:28:41PM +0200, Jan Kara wrote:
> Commit d279c80e0bac ("iomap: inline iomap_dio_bio_opflags()") has broken
> the logic in iomap_dio_bio_iter() in a way that when the device does
> support FUA (or has no writeback cache) and the direct IO happens to
> freshly allocated or unwritten extents, we will *not* issue fsync after
> completing direct IO O_SYNC / O_DSYNC write because the
> IOMAP_DIO_WRITE_THROUGH flag stays mistakenly set. Fix the problem by
> clearing IOMAP_DIO_WRITE_THROUGH whenever we do not perform FUA write as
> it was originally intended.
> 
> CC: John Garry <john.g.garry@oracle.com>
> CC: "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
> Fixes: d279c80e0bac ("iomap: inline iomap_dio_bio_opflags()")
> CC: stable@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Aha, I wonder if that's why fuse2fs+iomap occasionally fails the fstest
that tries to replay all the FUA writes, finds none, and complains.

(and I bet nobody noticed this on xfs because the log sends FUA, whereas
fuse2fs merely sends fsync to the block device)

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> BTW, I've spotted this because some performance tests got suspiciously fast
> on recent kernels :) Sadly no easy improvement to cherry-pick for me to fix
> customer issue I'm chasing...
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 6f25d4cfea9f..b84f6af2eb4c 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -363,14 +363,14 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  		if (iomap->flags & IOMAP_F_SHARED)
>  			dio->flags |= IOMAP_DIO_COW;
>  
> -		if (iomap->flags & IOMAP_F_NEW) {
> +		if (iomap->flags & IOMAP_F_NEW)
>  			need_zeroout = true;
> -		} else if (iomap->type == IOMAP_MAPPED) {
> -			if (iomap_dio_can_use_fua(iomap, dio))
> -				bio_opf |= REQ_FUA;
> -			else
> -				dio->flags &= ~IOMAP_DIO_WRITE_THROUGH;
> -		}
> +		else if (iomap->type == IOMAP_MAPPED &&
> +			 iomap_dio_can_use_fua(iomap, dio))
> +			bio_opf |= REQ_FUA;
> +
> +		if (!(bio_opf & REQ_FUA))
> +			dio->flags &= ~IOMAP_DIO_WRITE_THROUGH;
>  
>  		/*
>  		 * We can only do deferred completion for pure overwrites that
> -- 
> 2.43.0
> 
>