Re: [PATCH] xfs: fix write failures in software-provided atomic writes

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

On Thu, Oct 30, 2025 at 07:38:43PM +0000, John Garry wrote:
> On 30/10/2025 16:35, John Garry wrote:
> > > > 
> > > That's a good breadcrumb for me to follow;
> > 
> > I hope that it is ...
> > 
> > > I will turn on the rmap
> > > tracepoints to see if they give me a better idea of what's going on.
> > > I mentioned earlier that I think the problem could be that iomap treats
> > > srcmap::type == IOMAP_HOLE as if the srcmap isn't there, and so it'll
> > > read from the cow fork blocks even though that's not right.
> > 
> > Something else I notice for my failing test is that we do the regular
> > write, it ends in a sub-fs block write on a hole. But that fs block
> > (which was part of a hole) ends up being filled with all the same data
> > pattern (when I would expect the unwritten region to be 0s when read
> > back) - and this is what the compare fails on.
> 
> This makes the problem go away for me:
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index e1da06b157cf..e04af830d196 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1113,6 +1113,7 @@ xfs_atomic_write_cow_iomap_begin(
>  	unsigned int		dblocks = 0, rblocks = 0;
>  	int			error;
>  	u64			seq;
> +	xfs_filblks_t		count_fsb_orig = count_fsb;
> 
>  	ASSERT(flags & IOMAP_WRITE);
>  	ASSERT(flags & IOMAP_DIRECT);
> @@ -1202,7 +1203,7 @@ xfs_atomic_write_cow_iomap_begin(
>  found:
>  	if (cmap.br_state != XFS_EXT_NORM) {
>  		error = xfs_reflink_convert_cow_locked(ip, offset_fsb,
> -				count_fsb);
> +				count_fsb_orig);
>  		if (error)
>  			goto out_unlock;
>  		cmap.br_state = XFS_EXT_NORM;
> @@ -1215,6 +1216,7 @@ xfs_atomic_write_cow_iomap_begin(
>  	return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
> 
> I think that the problem may be that we were converting an inappropriate
> number of blocks from unwritten to real allocations (but never writing to
> the excess blocks). Does it look ok?

That looks like a good correction to me; I'll run that on my test fleet
overnight and we'll see what happens.  Thanks for putting this together!

--D


> thanks
>