Re: [PATCH] xfs: add missing cmap->br_state = XFS_EXT_NORM update

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

On Thu, Feb 17, 2022 at 05:55:42PM +0800, Gao Xiang wrote:
> COW extents are already converted into written real extents after
> xfs_reflink_convert_cow_locked(), therefore cmap->br_state should
> reflect it.
> 
> Otherwise, there is another necessary unwritten convertion
> triggered in xfs_dio_write_end_io() for direct I/O cases.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

I /think/ this looks ok.  Does it test ok too?  AFAICT nothing in the
iomap/writeback machinery cares the incorrect state because we always
set IOMAP_F_SHARED (which triggers COW) so I think this is simply a fix
for directio, like you said?

--D

> ---
> From the logic itself and runtime tracing, IMO, it seems true.
> Kindly correct me here if my understanding is wrong.
> 
>  fs/xfs/xfs_reflink.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 75bd2e03cd5b..5f0a364739a5 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -424,7 +424,10 @@ xfs_reflink_allocate_cow(
>  	if (!convert_now || cmap->br_state == XFS_EXT_NORM)
>  		return 0;
>  	trace_xfs_reflink_convert_cow(ip, cmap);
> -	return xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
> +	error = xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
> +	if (!error)
> +		cmap->br_state = XFS_EXT_NORM;
> +	return error;
>  
>  out_trans_cancel:
>  	xfs_trans_cancel(tp);
> -- 
> 2.24.4
>